Opened 13 years ago
Closed 13 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 13 years ago by
Priority: | minor → blocker |
---|---|
Status: | new → open |
comment:2 Changed 13 years ago by
comment:4 Changed 13 years ago by
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 13 years ago by
Blocking: | 7069 added |
---|
comment:6 Changed 13 years ago by
Owner: | set to Corey Frang |
---|---|
Status: | open → assigned |
comment:7 Changed 13 years ago by
Blocking: | 6022 added |
---|
comment:8 Changed 13 years ago by
Blocking: | 6781 added |
---|
comment:9 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Merged in gnarf's branch.
comment:10 Changed 13 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
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 13 years ago by
Blocking: | 6022 removed |
---|
comment:12 Changed 13 years ago by
Resolution: | → fixed |
---|---|
Status: | reopened → closed |
Reclosing at scotts request... see http://bugs.jqueryui.com/ticket/7103 for the other parts...
comment:13 Changed 12 years ago by
Blocking: | 6781 removed |
---|
comment:14 Changed 11 years ago by
Blocking: | 7069 removed |
---|
https://github.com/jquery/jquery-ui/pull/141