Bug Tracker

Ticket #8498 (closed feature: fixed)

Opened 3 years ago

Last modified 2 years ago

Animate Hooks

Reported by: gnarf Owned by: gnarf
Priority: blocker Milestone: 1.7.2
Component: effects Version: 1.5.1
Keywords: Cc: scott.gonzalez
Blocking: Blocked by:

Description (last modified by ajpiano) (diff)

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

comment:2 Changed 3 years ago by scott.gonzalez

  • Cc scott.gonzalez added

comment:3 Changed 3 years ago by rwaldron

  • Priority changed from undecided to low
  • Status changed from new to open
  • Component changed from unfiled to effects

comment:4 Changed 3 years ago by rwaldron

  • Owner set to gnarf
  • Status changed from open to assigned

comment:5 Changed 3 years ago by john

  • Type changed from bug to feature
  • Summary changed from Animating the "margin" "padding" or other similar properties can cause bad "flashes" to Animate Hooks

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

comment:6 Changed 3 years ago by john

  • Keywords 1.7-discuss added

Nominating ticket for 1.7 discussion.

comment:7 Changed 3 years ago by rwaldron

  • Description modified (diff)

+1, In support of, pending #9401

comment:8 Changed 3 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 3 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 3 years ago by gnarf (previous) (diff)

comment:10 Changed 3 years ago by timmywil

  • Description modified (diff)

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

comment:11 Changed 3 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 3 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 3 years ago by addyosmani

+1

comment:14 Changed 3 years ago by ajpiano

  • Description modified (diff)

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

comment:15 Changed 3 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 3 years ago by dmethvin

  • Priority changed from low to blocker
  • Milestone changed from 1.next to 1.7

comment:19 Changed 3 years ago by gnarf

  • Keywords 1.8-discuss added; 1.7-discuss removed
  • Milestone changed from 1.7 to 1.8

comment:18 Changed 3 years ago by Mike Sherov

  • Status changed from assigned to closed
  • Resolution set to fixed

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

Changeset: 8f5f1b2e6c0f7b8b6d66bfc06c7b169a9443c2b8

comment:19 Changed 3 years ago by dmethvin

  • Keywords 1.8-discuss removed
  • Milestone changed from 1.8 to 1.7.2
Note: See TracTickets for help on using tickets.