Bug Tracker

Modify

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

Version 1, edited 3 years ago by gnarf (previous) (next) (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 2 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 2 years ago by dmethvin

  • Keywords 1.8-discuss removed
  • Milestone changed from 1.8 to 1.7.2

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.