Opened 8 years ago

Closed 8 years ago

#7060 closed enhancement (fixed)

Improving internal Effects API

Reported by: Corey Frang Owned by: Corey Frang
Priority: blocker Milestone: 1.9.0
Component: ui.effects.core Version: 1.8.10
Keywords: Cc:
Blocked by: Blocking:

Description

Currently, the effects functions are being passed an object that looks something like this:

{ options: { pieces: 9 }, mode: 'hide', callback: fn, duration: 200 }

It seems that the options here becomes a little redundant internally, causing a need to reference o.options.pieces instead of just o.pieces

We can pretty easily override this function to create an "Effect Options Object" that would look more like this:

{ effect: 'explode', pieces: 9, mode: 'hide', complete: fn, duration: 200 }

There will be a BC break here - Any custom $.effects[] functions that have been written will need to be updated to replace o.options.* -> o.* and o.callback -> o.complete

It would be possible to add a BC duck punch if necessary, however my quick google search shows no results for "jquery ui effects custom".

I've started cleaning up the API a little here: https://github.com/gnarf37/jquery-ui/compare/master...effects-api

Let me know if you think the BC punch is needed - I still want to try and clean up some of the effects.js files before I file a pull...

Change History (13)

comment:1 Changed 8 years ago by Scott González

Priority: minorblocker
Status: newopen

comment:4 Changed 8 years ago by Corey Frang

Adding another internal effects API issue / idea onto this ticket - might be worthy of its own.

In thinking about how to handle the BC shim for this API change, and other ways we might clean up the internal API - I thought we could move the actual effect implementation functions out of the $.effects[] and into $.effects.effect[] . It gives us a discreet namespace where we are sure that the function is an effect API function. Currently, other functions exist out on $.effects[] that aren't effects, allowing you to get interesting results for $("div").toggle("animateClass") - Letting that namespace hold utility functions, while pushing the actual effects into a new namespace could show some minor improvements.

Also - We could potentially leverage $.effects.$ = jQuery.sub() and extend its fn to serve up the setMode, save, restore, createWrapper, removeWrapper, and a few other helper functions that all take el as a first parameter. Each of the effect functions can then use $.effects.$( this ) and have all the helper functions available in a nice chain.

Got most of these changes in and you can look at the branch here: https://github.com/gnarf37/jquery-ui/compare/effects-api...effects-sub -- it seems github is mistakenly showing some of the whitespace changes that were in the effects-api branch on the effects-sub branch, but you should get the idea...

comment:5 Changed 8 years ago by Corey Frang

Blocking: 7069 added

comment:6 Changed 8 years ago by Corey Frang

Owner: set to Corey Frang
Status: openassigned

comment:7 Changed 8 years ago by Corey Frang

Blocking: 6022 added

comment:8 Changed 8 years ago by Corey Frang

Blocking: 6781 added

comment:9 Changed 8 years ago by Scott González

Resolution: fixed
Status: assignedclosed

Merged in gnarf's branch.

comment:10 Changed 8 years ago by Corey Frang

Resolution: fixed
Status: closedreopened

There is still https://github.com/jquery/jquery-ui/pull/146 to look at scott - gonna reopen this until you decide there too :)

comment:11 Changed 8 years ago by Corey Frang

Blocking: 6022 removed

comment:12 Changed 8 years ago by Corey Frang

Resolution: fixed
Status: reopenedclosed

Reclosing at scotts request... see http://bugs.jqueryui.com/ticket/7103 for the other parts...

comment:13 Changed 8 years ago by Corey Frang

Blocking: 6781 removed

comment:14 Changed 7 years ago by mikesherov

Blocking: 7069 removed
Note: See TracTickets for help on using tickets.