search

mobify / pinny

A mobile-first content fly-in UI plugin
MIT License
23 stars 4 forks source link

Smoother animation towards the end of opening/closing pinny #87

Closed vmarta closed 8 years ago

vmarta commented 9 years ago

Status: open for review Reviewer: @Helen-Mobify

The opening/closing animation of pinny is slowed down by the things we do in openComplete() and closeComplete(). See a screenshot of this slowdown below.

So I added a setTimeout of 0 to delay them and to avoid conflicting with the animation. Tested on iPad 3 running iOS 8.0.

Timeline tab

vmarta commented 9 years ago

This is what the timeline looks like after the fix:

wizardlyhel commented 9 years ago

@vmarta This looks amazing

Pulling a couple people in to look at this together. @fractaltheory @donnielrt

donnielrt commented 9 years ago

Very cool, @vmarta!

donnielrt commented 9 years ago

Shouldn't openComplete be firing after the animation in any case?

mlworks commented 9 years ago

@donnielrt Yea it is weird how Velocity's Complete callback happens BEFORE the animation finishes. Here's a screenshot with a debugger set in the complete callback. Notice the small gap in the left, animation is still not done.

screen shot 2015-06-25 at 3 14 17 pm

donnielrt commented 9 years ago

@mlworks interesting, that makes me think we're maybe dealing with a bug we hadn't caught before. If openComplete is firing before the open animation is complete, that's a major bug!

donnielrt commented 8 years ago

Hey, we’re looking to prune older abandoned PRs. If this PR is still relevant and you would like to see it merged in, please reopen the PR and we’ll add it to our backlog! Thanks!

vmarta commented 8 years ago

Let's reopen this baby! I feel like we got a solid fix to pinny's animation.. we've been using it for a couple of our recent projects.

@donnielrt What do we need to do next to get this PR approved?

donnielrt commented 8 years ago

Thanks @vmarta! We're going to take a quick look at it, and get in touch if we have questions. Otherwise we'll go ahead and prep this for publishing.

haroldtreen commented 8 years ago

Thanks for the PR @vmarta!

LGTM :+1:

haroldtreen commented 8 years ago

Merged this PR into btr-fixes where a bunch of other changes are waiting to do a release. We'll get this in through that PR @vmarta :).

https://github.com/mobify/pinny/commit/412fc34a230a54c1b01cc01267ddd0718870e246

vmarta commented 8 years ago

@haroldtreen sounds good.. thanks Harold 👍🏻