Skip to content

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

Merged
merged 13 commits into from
Mar 30, 2023

Conversation

ivirtex
Copy link
Contributor

@ivirtex ivirtex commented Mar 9, 2023

This PR updates transition animation curves and transition duration used by CupertinoPageRoute.
Docs part: flutter/assets-for-api-docs#211

Fixes #122247

Before After
RPReplay_Final1678315602.mov
RPReplay_Final1678221817.mov

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

Sorry, something went wrong.

@flutter-dashboard flutter-dashboard bot added f: cupertino flutter/packages/flutter/cupertino repository f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels. labels Mar 9, 2023
@ivirtex ivirtex force-pushed the update-ios-transitions branch 3 times, most recently from ae4fd64 to 16a4e22 Compare March 14, 2023 22:25
@ivirtex ivirtex force-pushed the update-ios-transitions branch from 369fd59 to a1b2077 Compare March 16, 2023 22:58
@ivirtex ivirtex marked this pull request as ready for review March 17, 2023 07:13
@flutter-dashboard flutter-dashboard bot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Mar 17, 2023
@ivirtex
Copy link
Contributor Author

ivirtex commented Mar 17, 2023

cc @MitchellGoodwin

@MitchellGoodwin MitchellGoodwin self-requested a review March 17, 2023 17:14
@ivirtex ivirtex force-pushed the update-ios-transitions branch 2 times, most recently from f9c8820 to 57e5d61 Compare March 18, 2023 23:30
@MitchellGoodwin
Copy link
Contributor

@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.

@ivirtex ivirtex force-pushed the update-ios-transitions branch from 57e5d61 to 5e65f2e Compare March 20, 2023 17:39
Copy link
Contributor

@MitchellGoodwin MitchellGoodwin left a comment

Choose a reason for hiding this comment

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

LGTM

@flutter-dashboard flutter-dashboard bot added the a: animation Animation APIs label Mar 21, 2023
@ivirtex
Copy link
Contributor Author

ivirtex commented Mar 21, 2023

@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.:
/// {@animation 464 192 https://flutter.github.io/assets-for-api-docs/assets/animation/curve_fast_linear_to_slow_ease_in.mp4}

curve_fast_linear_to_slow_ease_in.mp4

@ivirtex ivirtex force-pushed the update-ios-transitions branch from 5cf2103 to a4dcf5a Compare March 21, 2023 14:54
@MitchellGoodwin
Copy link
Contributor

@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.: /// {@animation 464 192 https://flutter.github.io/assets-for-api-docs/assets/animation/curve_fast_linear_to_slow_ease_in.mp4}

I believe that's done through this repo: https://github.com/flutter/assets-for-api-docs

@ivirtex
Copy link
Contributor Author

ivirtex commented Mar 21, 2023

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.

@ivirtex ivirtex requested a review from MitchellGoodwin March 22, 2023 18:58
Copy link
Contributor

@MitchellGoodwin MitchellGoodwin left a 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.

@ivirtex ivirtex force-pushed the update-ios-transitions branch 2 times, most recently from 9b77a92 to 381cc6f Compare March 23, 2023 13:45
@MitchellGoodwin MitchellGoodwin added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 27, 2023
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 27, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented Mar 27, 2023

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.

  • Merge guidelines: You need at least one approved review if you are already a MEMBER or two member reviews if you are not a MEMBER before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@auto-submit
Copy link
Contributor

auto-submit bot commented Mar 27, 2023

auto label is removed for flutter/flutter, pr: 122275, due to Validations Fail.

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 30, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 30, 2023
@ivirtex ivirtex deleted the update-ios-transitions branch March 30, 2023 21:52
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 31, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Apr 1, 2023
exaby73 pushed a commit to NevercodeHQ/flutter that referenced this pull request Apr 17, 2023
Update `CupertinoPageRoute` transition animation curves
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
MitchellGoodwin pushed a commit to flutter/assets-for-api-docs that referenced this pull request Jun 12, 2023

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: animation Animation APIs autosubmit Merge PR when tree becomes green via auto submit App c: contributor-productivity Team-specific productivity, code health, technical debt. f: cupertino flutter/packages/flutter/cupertino repository f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transition animation curves used by Cupertino are outdated
3 participants