#8498 closed feature (fixed)
Animate Hooks
| Reported by: | gnarf | Owned by: | gnarf |
|---|---|---|---|
| Priority: | blocker | Milestone: | 1.7.2 |
| Component: | effects | Version: | 1.5.1 |
| Keywords: | Cc: | scottgonzalez | |
| Blocked by: | Blocking: |
Description (last modified by )
A quick fiddle to demonstrate: http://jsfiddle.net/gnarf/9muvX/
I propose that we should add a pre-filter to the properties animated in .animate() that would allow us to convert
a call like
.animate({margin: 40})into.animate({marginLeft: 40, marginRight: 40, marginBottom: 40, marginTop: 40})automatically.
Change History (19)
comment:2 Changed 6 years ago by
| Cc: | scott.gonzalez added |
|---|
comment:3 Changed 6 years ago by
| Component: | unfiled → effects |
|---|---|
| Priority: | undecided → low |
| Status: | new → open |
comment:4 Changed 6 years ago by
| Owner: | set to gnarf |
|---|---|
| Status: | open → assigned |
comment:5 Changed 6 years ago by
| Summary: | Animating the "margin" "padding" or other similar properties can cause bad "flashes" → Animate Hooks |
|---|---|
| Type: | bug → feature |
comment:8 Changed 6 years ago by
-1, sounds reasonable to me... but wouldn't making cssHooks more "open" (that is being able to return the redefinition if needed by a third-party like effects.js) solve the issue? I hate the idea of css related stuff hidden into effects.js.
comment:9 Changed 6 years ago by
| Description: | modified (diff) |
|---|
well Julian - Yeah, I kinda agree that the animateHooks might not be the best implementation -- likely we could make it like:
$.cssHooks.margin = {
map: function( value ) {
return {
marginLeft: value,
marginRight: value,
marginTop: value,
marginBottom: value
};
}
};
And then soup up .css and .animate to read from the hooks looking for a "map" when they are doing their property mangling stuff i.e. camelCase.
My reasoning for taking the approach I did is that when you do .css("margin", 10); you could actually skip "mapping" the values, because the browser already does it for you, there isn't a ton of reason to have the "map" unless you are doing a per-property based animation....
Does the other approach get a more favorable vote in your eyes?
comment:10 Changed 6 years ago by
| Description: | modified (diff) |
|---|
+1, Yes, I think some stuff should be in effects.js
comment:11 Changed 6 years ago by
| Description: | modified (diff) |
|---|
-1, I have concerns about size if all these strings are going into the source -- is there some other solution? Maybe a plugin would be best for non-trivial animation enhancements.
comment:12 Changed 6 years ago by
| Description: | modified (diff) |
|---|
+1, Only if we're talking about adding a map to cssHooks (and not necessarily adding in those properties ourselves).
comment:14 Changed 6 years ago by
| Description: | modified (diff) |
|---|
+1, I like the idea of just using cssHooks from within .animate
comment:15 Changed 6 years ago by
If we are looking towards supporting animations like margin: 30px 40px, I've worked out a regex for it for fun:
/(-?\d+)(?:px)?(?:\s+(-?\d+)(?:px)?)?(?:\s+(-?\d+)(?:px)?)?(?:\s+(-?\d+)(?:px)?)?/
xoxo
comment:17 Changed 6 years ago by
| Milestone: | 1.next → 1.7 |
|---|---|
| Priority: | low → blocker |
comment:19 Changed 6 years ago by
| Keywords: | 1.8-discuss added; 1.7-discuss removed |
|---|---|
| Milestone: | 1.7 → 1.8 |
comment:18 Changed 6 years ago by
| Resolution: | → fixed |
|---|---|
| Status: | assigned → closed |
Fix #8498. Add cssHooks[prop].expand for use by animate().
Changeset: 8f5f1b2e6c0f7b8b6d66bfc06c7b169a9443c2b8
comment:19 Changed 6 years ago by
| Keywords: | 1.8-discuss removed |
|---|---|
| Milestone: | 1.8 → 1.7.2 |

I'm mutating this bug into a new feature request - where we add in a set of hooks for doing animations.