search

visionmedia / move.js

CSS3 backed JavaScript animation framework
http://visionmedia.github.com/move.js/
4.72k stars 686 forks source link

added ability to set multiple properties by accepting a JS object as a parameter #74

Closed RaghavRamesh closed 7 years ago

RaghavRamesh commented 8 years ago

In response to the feature request in #61

A commented 8 years ago

@axelpale what do you think about this PR? I'm confused 'cause I didn't use move by this way. hmhm

axelpale commented 8 years ago

@RaghavRamesh @shuvalov-anton Basically this seems ok to me because for example jQuery#css allows similar input. The PR implements the feature cleanly, although it adds one extra if-comparison to each .set call, which, however, is inevitable.

Thinking aloud: Only thing that keeps me thinking is the execution order. For example transformations must be in order to get the correct outcome. Is the same true for css-properties? I think not, css-propertiess are not order dependent. Are the durations applied to each property separately or are they all applied at the same time? Latter is true, implemented in Move#applyProperties, which is good.

Therefore I endorse the merge. Any further thoughts?

BTW, later on we should give API docs a little love.

RaghavRamesh commented 8 years ago

Any updates on merging this? :)

axelpale commented 7 years ago

Thank you for the PR and sorry for the laaaazyyy merge. Generally speaking, accepting a PR is a bit risky business as we do not have unit tests or performance benchmarks.

http://imgur.com/gallery/y7Hm9