Bug Tracker

Opened 6 years ago

Closed 5 years ago

Last modified 5 years ago

#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 ajpiano)

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 scottgonzalez

Cc: scott.gonzalez added

comment:3 Changed 6 years ago by Rick Waldron

Component: unfiledeffects
Priority: undecidedlow
Status: newopen

comment:4 Changed 6 years ago by Rick Waldron

Owner: set to gnarf
Status: openassigned

comment:5 Changed 6 years ago by john

Summary: Animating the "margin" "padding" or other similar properties can cause bad "flashes"Animate Hooks
Type: bugfeature

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

comment:6 Changed 6 years ago by john

Keywords: 1.7-discuss added

Nominating ticket for 1.7 discussion.

comment:7 Changed 6 years ago by Rick Waldron

Description: modified (diff)

+1, In support of, pending #9401

comment:8 Changed 6 years ago by jaubourg

-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 gnarf

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?

Last edited 6 years ago by gnarf (previous) (diff)

comment:10 Changed 6 years ago by timmywil

Description: modified (diff)

+1, Yes, I think some stuff should be in effects.js

comment:11 Changed 6 years ago by dmethvin

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 john

Description: modified (diff)

+1, Only if we're talking about adding a map to cssHooks (and not necessarily adding in those properties ourselves).

comment:13 Changed 6 years ago by addyosmani

+1

comment:14 Changed 6 years ago by ajpiano

Description: modified (diff)

+1, I like the idea of just using cssHooks from within .animate

comment:15 Changed 6 years ago by timmywil

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 dmethvin

Milestone: 1.next1.7
Priority: lowblocker

comment:19 Changed 6 years ago by gnarf

Keywords: 1.8-discuss added; 1.7-discuss removed
Milestone: 1.71.8

comment:18 Changed 5 years ago by Mike Sherov

Resolution: fixed
Status: assignedclosed

Fix #8498. Add cssHooks[prop].expand for use by animate().

Changeset: 8f5f1b2e6c0f7b8b6d66bfc06c7b169a9443c2b8

comment:19 Changed 5 years ago by dmethvin

Keywords: 1.8-discuss removed
Milestone: 1.81.7.2
Note: See TracTickets for help on using tickets.