Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Timob 6017 Mobile web animation. #847

Merged
merged 28 commits into from Dec 8, 2011
Merged

Timob 6017 Mobile web animation. #847

merged 28 commits into from Dec 8, 2011

Conversation

nebrius
Copy link
Contributor

@nebrius nebrius commented Dec 6, 2011

No description provided.

… supported in mobileweb), autoreverse, and repeat.
…'t (and couldn't) work properly and replaced it with a separate rotation entry in animations.
…. It was ignoring all passed-in parameters. Created an alternate method to avoid potentially breaking things.
…t anything in the constructor, not just the official API properties of the object.
…emoved some edge-case detection that was too brittle.
duration = val[prop]
} else {
props.push(prop);
obj._setPrefixedCSSRule = function(rule,value) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is AWESOME! My only concern is eventually rules like "transition" will no longer require a vendor prefix and the rule should be "transition" and not "Transition". In other words, you should auto-capitalize the rule, but leave the non-vendor prefixed version whatever case is passed in. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I was rather proud of that little bit of code. You brought up a good point and I totally agree. Fixed.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

obj._setPrefixedCSSRule looks identical to obj._getPrefixedCSSRule except for the return or set.

Suggest you have a single function to build the ruleset and then use that in get and set. It should save quite a bit of space and not have duplicate code around.

@cb1kenobi
Copy link
Contributor

Reviewed code, tested using provided example and Kitchen Sink. Pretty solid. Request accepted.

(animation.opaque === false || animation.visible === false) && (_style.opacity = 0.0);

// Set the position and size properties
isDefined(animation.top) && (_style.top = animation.top + "px");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI values coming into Titanium can have units. px, dp, sp, in, mm, cm, %

@donthorp
Copy link
Contributor

donthorp commented Dec 7, 2011

Code re-reviewed. Request accepted

@cb1kenobi
Copy link
Contributor

Code re-reviewed and re-tested. Looks good! Request accepted.

cb1kenobi added a commit that referenced this pull request Dec 8, 2011
Timob 6017 Mobile web animation.
@cb1kenobi cb1kenobi merged commit b6d2b78 into tidev:master Dec 8, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants