#366 ✓ staged
Scott Downe

0.1 demo effects

Reported by Scott Downe | February 24th, 2011 @ 03:33 PM | in 0.7

Once ticket #275 lands, we are set up to bring back the red flash, inactive div, and top left icons in the original demo.

Along with creating an effects folder, and putting the three effects plugins in that folder.

Currently, I want to do this in one ticket, but I may decide to break it up.

Blocked on #275

Comments and changes to this ticket

  • Scott Downe

    Scott Downe February 24th, 2011 @ 05:20 PM

    • Milestone changed from 0.4 to 0.5
    • Milestone order changed from “33” to “0”
  • Scott Downe

    Scott Downe March 24th, 2011 @ 05:59 PM

    • Milestone changed from 0.5 to 0.6
    • Milestone order changed from “9” to “0”
  • Scott Downe

    Scott Downe April 11th, 2011 @ 11:27 AM

    This ticket is now blocked on ticket #446.

  • annasob

    annasob May 10th, 2011 @ 05:24 PM

    • Milestone changed from 0.6 to 0.7
    • Milestone order changed from “2” to “0”
  • Scott Downe

    Scott Downe June 2nd, 2011 @ 05:21 PM

    • State changed from “blocked” to “assigned”
  • Rick

    Rick June 18th, 2011 @ 11:39 AM

    • Milestone changed from 0.7 to 0.8
    • Milestone order changed from “5” to “0”
  • Scott Downe

    Scott Downe June 20th, 2011 @ 07:57 AM

    • Milestone changed from 0.8 to 0.7
    • Milestone order changed from “10” to “0”

    So, patch here: https://github.com/ScottDowne/popcorn-js/commits/t366

    I'm sitting on this, mainly because I'm having trouble settling on a word, something to call it. I started with effect because I wanted to get started, knowing that I was probably going to change it.

    Two places we need to reference this feature.

    One is registering a new effect:

    Popcorn.effect( "effectName", {
      _setup: function( options ) {}
    });
    

    Second is applying a registered effect to an instance of a plugin:

    p.footnote({
      start:
      end:
      text:
      effects: "effectName anotherEffectName"
    });
    

    Effect is too limiting in describing the power. It is capable of doing more than just visuals, effects or UI stuff. It can do anything a plugin can do, cept without the timing data. It does sound simple, and won't get confused with our use for the word plugin.

    Extend: describes complete level of capabilities, but might not be technically correct, and is a bit of a buzz word. Now, add that with the current use of the word plugin, and we get into a whole mess of technical definitions only to confuse our users. Plugin vs addon vs extension. I don't want to go there. As far as I am concerned, and from a general user's point of view, these all describe our plugins.

    Inherit: Same complaints as extend.

    Append: Rick suggested this, and I'm liking it. It's my current choice. It provides a simple, yet not implying a limited capabilities, description.

    I'm looking for new ideas, or new perspectives of current ideas.

  • Rick

    Rick June 20th, 2011 @ 08:57 AM

    To clarify, I think I was misunderstood. I used the "append" name as an example of a method that is just a facade to a polymorphic function that might handle many tasks but can be named whatever. I don't think that append makes sense here. Additionally, "extend" is in use by the Popcorn.extend.

    Scott, let's chat on IRC a bit more about this today

  • Rick

    Rick June 20th, 2011 @ 09:02 AM

    Suggested:

    The abstract:

    
    Popcorn.compose( "composablename", {
    
      _setup: function( options ) {
    
      }
    
    });
    

    The specific:

    
    Popcorn.plugin.effect( "effectname", {
    
      _setup: function( options ) {
    
      }
    
    });
    

    And is implemented:

    
    Popcorn.plugin.effect = function( name, definitionObject ) {
    
      Popcorn.compose( name, definitionObject );
    
    };
    
  • Scott Downe

    Scott Downe June 21st, 2011 @ 07:20 PM

    • State changed from “assigned” to “peer-review-requested”
    • Assigned user cleared.

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

    So, in total what I've done is:

    • created an effect and compose interface.
    • created core tests for that interface.
    • created an applyclass effect plugin.
    • created a simple applyclass.html demo.
    • applied applyclass to semantic demo.

    Anything regarding expand or flash effect was removed once I figured out the excellence of applayclass, so any reference to them should be removed in a later commit, if I missed any references to them, let me know and I will remove it.

    I will clean up my commit history if it is not readable. Just ask.

  • Rick

    Rick June 21st, 2011 @ 07:53 PM

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

    Rick June 21st, 2011 @ 08:16 PM

    I've been cheering you on right along and of course you know I think the approach is sound, in fact - excellent.

    First thing that stands out to me is this:

    
    definition._setup = definition._setup || Popcorn.nop;
    definition._teardown = definition._teardown || Popcorn.nop;
    definition.start = definition.start || Popcorn.nop;
    definition.end = definition.end || Popcorn.nop;
    
    
    // ... 
    
    natives._setup = combineFn( natives._setup, compose._setup );
    natives._teardown = combineFn( natives._teardown, compose._teardown );
    natives.start = combineFn( natives.start, compose.start );
    natives.end = combineFn( natives.end, compose.end );
    

    Which can be reduced significantly by doing something like this:

    
    var methods = [ "_setup", "_teardown", "start", "end" ];
    
    //...
    
    // apply safe, and empty default functions
    methods.forEach(function( method ) {
      definition[ method ] = definition[ method ] || Popcorn.nop;      
    });
    
    
    // ...
    
    // extends previous functions with compose function
    methods.forEach(function( method ) {
      natives[ method ] = combineFn( natives[ method ], compose[ method ] );
    });
    
  • Rick

    Rick June 21st, 2011 @ 08:26 PM

    In popcorn.applyclass.js...

    I would recommend against assuming that "$" is always synonymous with jQuery and you can minimize the code for checking...

    
    if ( typeof $ !== "undefined" ) {
    
    // to...
    
    if ( !window.jQuery ) {
    

    I would just go ahead make all calls to $() => jQuery(), it'll be safer that way. For the CDN url, link to this:
    http://ajax.googleapis.com/ajax/libs/jquery/1/jquery.min.js ... which ensures you always get the latest stable release. (We just rc'ed 1.6.2, so 1.6.1 is already on the way out)

    I think we might be able to do this stuff without jQuery, using qSA and classList... let's work together on that tomorrow

    Can you clarify this:

    
    
    effect: "applyclass",
    applyclass: ".overlay: applyoverlay, parent: hover"
    

    It sort of seems like "applyclass" is doing a double duty?

  • Rick

    Rick June 21st, 2011 @ 08:28 PM

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

    Also... This is really great work :)

  • Scott Downe

    Scott Downe June 22nd, 2011 @ 09:45 AM

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

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

    I added the forEach for methods.

    I a cleaned up the jquery stuff.

    I want to keep jQuery for this, mainly to leverage the selectors, ( #target .class ) or all the other neat things selectors allows.

    effect: "applyclass",
    applyclass: ".overlay: applyoverlay, parent: hover"
    

    effect: "applyclass" is the ticket to load in an effect or composition. This can be any effect that may or may not expose something to the track event, which also exposes whatever it loads to the manifest of that event instance.

    I think we need both, because these can be combined in multiple ways, can also be read form the manifest, and applied via defaults.

  • Scott Downe

    Scott Downe June 22nd, 2011 @ 09:50 AM

    • State changed from “peer-review-requested” to “assigned”
    • Assigned user changed from “Rick” to “Scott Downe”

    Ah, I see now. qSA = querySelectorAll.

  • Rick

    Rick June 22nd, 2011 @ 10:01 AM

    Re: effect: "applyclass"

    Ok, thanks for the clarification on that.

    We can use any valid selectors with qSA as well. Like I said, let's work together to see if we can remove the dependency

  • Rick

    Rick June 22nd, 2011 @ 12:13 PM

    This is really tremendous, great work on switching to classList + qSA

    A couple things...

    https://github.com/ScottDowne/popcorn-js/blob/38b739baabbd537d7457e...

    Since you're only referring to this once, we can skip the var assignment...

    
    var element = elements[ idx ];
    
    element.classList.toggle( val );
    

    can be:

    
    elements[ idx ].classList.toggle( val );
    

    Inside of _setup, check for existence of options.applyclass before using it and array is kinda vague :P

    
    var array = options.applyclass.replace( /\s/g, "" ).split( "," ),
    

    Style nitpick, space needed between "" and ) here:
    https://github.com/ScottDowne/popcorn-js/blob/38b739baabbd537d7457e...

    Here, there is an assumption that item will have 2 indices, 0 & 1:
    https://github.com/ScottDowne/popcorn-js/blob/38b739baabbd537d7457e...

  • Scott Downe

    Scott Downe June 22nd, 2011 @ 03:41 PM

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

    3/4 fixed.

    Good call on applyclass.replace when applyclass could be undefined.

    I believe we are fine regarding "Here, there is an assumption that item will have 2 indices, 0 & 1:"

    options.classes[ item[ 0 ] ] = item[ 1 ] || "";
    

    I default to an empty string if index 1 does not exist. Index 0 will only not exist if someone does closes the list with a comma:

    applyclass: "classname1, classname2, "
    

    I'll see what I can do about that, but index 1 should be fine.

  • Scott Downe
  • Rick

    Rick June 22nd, 2011 @ 04:12 PM

    • State changed from “assigned” to “review-looks-good”
    • Assigned user changed from “Scott Downe” to “Jon Buckley”
  • Jon Buckley

    Jon Buckley June 22nd, 2011 @ 04:24 PM

    • State changed from “review-looks-good” to “review-needs-work”

    Semantic demo doesn't fade the red transition away

  • Jon Buckley

    Jon Buckley June 23rd, 2011 @ 10:45 AM

    • Assigned user changed from “Jon Buckley” to “Scott Downe”

    Scott, want to go ahead with landing the effects plugin as-is, and move the semantic demo fixes to another ticket?

  • Scott Downe

    Scott Downe June 23rd, 2011 @ 11:55 AM

    • State changed from “review-needs-work” to “super-review-requested”

    Stage this please, I'm going to do the demo fix in another ticket.

  • Jon Buckley

    Jon Buckley June 23rd, 2011 @ 12:03 PM

    • State changed from “super-review-requested” to “staged”
  • Scott Downe

    Scott Downe June 23rd, 2011 @ 01:17 PM

    Doing the demo fix here #592.

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