-
Notifications
You must be signed in to change notification settings - Fork 28.5k
Update CupertinoPageRoute
transition animation curves
#122275
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
Conversation
ae4fd64
to
16a4e22
Compare
369fd59
to
a1b2077
Compare
f9c8820
to
57e5d61
Compare
@ivirtex thank you for putting this together. I think the failing test is from issues outside this PR that have been addressed, but still looks a little flaky. I would try rebasing again. For the actual code, it LGTM, except I'm wondering if the curve should be given a generic name and put in the Curves class? I could see it being specific to this case, but there is history of putting very specific curves in there. |
57e5d61
to
5e65f2e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@MitchellGoodwin I've put this curve in the Curves class, but I'm not sure how to generate animation previews as other Curves have in their doc comments e.g.: curve_fast_linear_to_slow_ease_in.mp4 |
5cf2103
to
a4dcf5a
Compare
I believe that's done through this repo: https://github.com/flutter/assets-for-api-docs |
Thanks! I've just created a PR on this repo: flutter/assets-for-api-docs#211 and added link to relevant diagram in the doc comments on this side. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. The other PR for the diagram also looks good to me as well. I think this one can be merged as is first, and the other will have to wait until this code is live in stable. Feel free to ping me if I don't get back to this to merge or comment on this by next week.
9b77a92
to
381cc6f
Compare
auto label is removed for flutter/flutter, pr: 122275, due to This PR has not met approval requirements for merging. You have project association CONTRIBUTOR and need 0 more review(s) in order to merge this PR.
|
auto label is removed for flutter/flutter, pr: 122275, due to Validations Fail. |
Update `CupertinoPageRoute` transition animation curves
This PR adds diagram of `Curves.fastEaseInToSlowEaseOut` as part of flutter/flutter#122275 https://user-images.githubusercontent.com/57679865/226763570-1f1e21b7-f615-4827-be31-67831156c15e.mp4 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read the [Flutter Style Guide] _recently_, and have followed its advice. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] All existing and new tests are passing.
This PR updates transition animation curves and transition duration used by
CupertinoPageRoute
.Docs part: flutter/assets-for-api-docs#211
Fixes #122247
RPReplay_Final1678315602.mov
RPReplay_Final1678221817.mov
Pre-launch Checklist
///
).