#521 ✓ staged
Scott Downe

some style cleanup of core

Reported by Scott Downe | May 16th, 2011 @ 09:02 PM | in 0.6

I probably didn't get it all, but a win is a win.

https://github.com/ScottDowne/popcorn-js/commits/style

Comments and changes to this ticket

  • Scott Downe

    Scott Downe May 16th, 2011 @ 09:06 PM

    • Title changed from “some whitespace cleanup of core” to “some style cleanup of core”
  • Rick

    Rick May 16th, 2011 @ 09:29 PM

    • State changed from “peer-review-requested” to “under-review”
    • Assigned user set to “Rick”

    weeeeeiiiirdddd. I've been doing the same for the last hour or so, or whenever i finished your last review.

    reviewing!

  • Scott Downe

    Scott Downe May 16th, 2011 @ 09:35 PM

    I started and finished before I made the ticket, so I didn't have a ticket number yet. My branch is "style"

  • Scott Downe

    Scott Downe May 16th, 2011 @ 09:36 PM

    We should merge what we both got, as I am sure I missed some, and also I didn't focus on vertical whitespace.

  • Rick

    Rick May 16th, 2011 @ 09:46 PM

    • State changed from “under-review” to “review-needs-work”
    • Assigned user changed from “Rick” to “Scott Downe”

    Nice work dude. Alot of this is the clean up I've been meaning to get a code freeze for. A few more bits:

    
    sizeOf: function ( obj ) => sizeOf: function( obj ) 
    
    Popcorn.extend(Popcorn.p, (function () { => 
      Popcorn.extend(Popcorn.p, (function() {
    
    
    Popcorn.forEach( self.data.events[ type ], function ( obj, key ) { =>
      Popcorn.forEach( self.data.events[ type ], function( obj, key ) {
    
    
    Popcorn.forEach( setup, function ( callback, type ) { =>
      Popcorn.forEach( setup, function( callback, type ) {
    
    parseFn = function ( filename, callback ) { =>
      parseFn = function( filename, callback ) {
    
    
    var clientRect = elem.getBoundingClientRect(),
        bounds     = {},
        doc        = elem.ownerDocument,
        docElem    = document.documentElement,
        body       = document.body,
        clientTop, clientLeft, scrollTop, scrollLeft, top, left;
    
    =>
    
    var clientRect = elem.getBoundingClientRect(),
        bounds = {},
        doc = elem.ownerDocument,
        docElem = document.documentElement,
        body = document.body,
        clientTop, clientLeft, scrollTop, scrollLeft, top, left;
    
    // While you're at it... fix all of the `name[space]=[space]value" in that function :)
    
  • Rick

    Rick May 16th, 2011 @ 09:47 PM

    In addition to all the horizontal whitespace I was studying the minified code, tweaking and re-minifying, so alot might be in conflict.

  • Scott Downe

    Scott Downe May 16th, 2011 @ 09:57 PM

    You mentioned you were doing this too, I wonder if yours can completely trump mine?

  • Rick

    Rick May 16th, 2011 @ 10:06 PM

    I was focusing more on getting min file size down. Check this out...

    (Same portion of code)

    
    // NEW 504 bytes
    
    (function(p,o){var s=Array.prototype,t=Object.prototype,u=s.forEach,r=t.hasOwnProperty,w=s.slice,x=t.toString,
    y=/^(#([\w\-\_\.]+))$/,q=[],v=false,b=function(a,c){return new b.p.init(a,c||null)};b.instances=instances=[];
    b.instanceIds=instanceIds={};b.removeInstance=function(a){var c=c,d=d;
    if(c.length){c.splice(d[a.id],1);delete d[a.id];return c}};b.addInstance=function(a){var c=c,d=d,f=c.length,e=a.media.id&&a.media.id;a.id=!(e in d)&&e||"__popcorn"+f;d[a.id]=f;c.push(a);return c};b.getInstanceById=
    
    
    // OLD 590 bytes
    
    (function(p,o){var s=Array.prototype.forEach,r=Object.prototype.hasOwnProperty,u=Array.prototype.slice,v=Object.prototype.toString,
    w=/^(#([\w\-\_\.]+))$/,q=[],t=false,b=function(a,c){return new b.p.init(a,c||null)};b.instances=[];
    b.instanceIds={};b.removeInstance=function(a){if(b.instances.length){b.instances.splice(b.instanceIds[a.id],1);delete b.instanceIds[a.id];return b.instances}};
    b.addInstance=function(a){var c=b.instances.length,d=a.media.id&&a.media.id;a.id=!(d in b.instanceIds)&&d||"__popcorn"+c;b.instanceIds[a.id]=c;b.instances.push(a);return b.instances};b.getInstanceById=
    
  • Rick

    Rick May 16th, 2011 @ 10:10 PM

    So, I'm just going to reapply my compression stuff to your branch locally

  • Scott Downe

    Scott Downe May 16th, 2011 @ 10:13 PM

    Yeah, I think we should do this on the same branch as to avoid conflicts. Teamwork!

    I'll be playing with the CSS vendor prefix for a bit now anyway.

  • Rick

    Rick May 16th, 2011 @ 10:30 PM

    • State changed from “review-needs-work” to “super-review-requested”
    • Assigned user changed from “Scott Downe” to “annasob”

    I made a mistake in my approach, so for now lets just move ahead with your work, its good and should land for 0.6

    Tested

    • FF 3.6, 4.x

    • Chrome 11, 12, 13

  • Scott Downe

    Scott Downe May 16th, 2011 @ 11:01 PM

    OK, sounds good.

    I did go ahead and fix the few I missed that you found in your initial review: https://github.com/ScottDowne/popcorn-js/commits/style

    Let's get them while we're here :)

  • annasob

    annasob May 17th, 2011 @ 12:08 PM

    • State changed from “super-review-requested” to “review-looks-good”

    this looks good SR+

  • annasob

    annasob May 17th, 2011 @ 12:10 PM

    • State changed from “review-looks-good” to “staged”

    Staged in annasob/popcorn-js commit

  • Rick

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

Popcorn.js is an HTML5 video framework that lets you bring elements of the web into your videos.

Popcorn.js is a project of Web Made Movies, Mozilla's Open Video Lab.

Shared Ticket Bins

Referenced by

Pages