-
Notifications
You must be signed in to change notification settings - Fork 29.7k
fix a bug when android uses CupertinoPageTransitionsBuilder... #114303
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
fix a bug when android uses CupertinoPageTransitionsBuilder... #114303
Conversation
|
This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
e73f462 to
1dcda2c
Compare
|
@ipcjs Looks like this change is breaking some tests. Can you please take a look and update the PR? |
|
It looks like the tests are failing... |
1dcda2c to
e351120
Compare
|
All tests have passed! |
2d690f0 to
2d032a1
Compare
|
@Hixie Yes |
packages/flutter/test/material/page_transitions_theme_test.dart
Outdated
Show resolved
Hide resolved
packages/flutter/test/material/page_transitions_theme_test.dart
Outdated
Show resolved
Hide resolved
2d032a1 to
53d847a
Compare
chunhtai
left a comment
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.
Except for request for another test, the code LGTM
chunhtai
left a comment
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
packages/flutter/test/material/page_transitions_theme_test.dart
Outdated
Show resolved
Hide resolved
packages/flutter/test/material/page_transitions_theme_test.dart
Outdated
Show resolved
Hide resolved
c7a5d5d to
f50b6e2
Compare
|
I've rebase to latest master branch, |
|
Looks like all checks have passed! |
| Animation<double> animation, | ||
| Animation<double> secondaryAnimation, | ||
| Widget child, | ||
| TargetPlatform platform, |
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.
This is effectively a breaking change for both callers and subclassers (it does break existing internal Google apps). On the upside, it does not breaking any existing Flutter repo or customer tests (flutter/tests). A brief migration will be needed per https://docs.flutter.dev/resources/compatibility. One of the reviewers can update the internal Google apps.
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.
Should I create a migration guide in the website repository look like flutter/website#8976?
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.
flutter/website#8976 looks like it covers the changes. Will land this PR and make the internal updates.
|
auto label is removed for flutter/flutter, pr: 114303, due to This PR has not met approval requirements for merging. You are not a member of flutter-hackers and need 1 more review(s) in order to merge this PR.
|
HansMuller
left a comment
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
flutter/flutter@35085c3...bc49cd1 2023-07-06 [email protected] Allow long-press gestures to continue even if buttons change. (flutter/flutter#127877) 2023-07-06 [email protected] Enable unreachable_from_main lint - it is stable now!!1 (flutter/flutter#129854) 2023-07-06 [email protected] Update labeler to new label names (flutter/flutter#130040) 2023-07-05 [email protected] MergeableMaterial: Fix adding a slice and separating it (flutter/flutter#128804) 2023-07-05 [email protected] Update infrastructure issue template for new priority scheme (flutter/flutter#129741) 2023-07-05 [email protected] Fix typo in canvas example (flutter/flutter#129879) 2023-07-05 [email protected] Reland Fix AnimatedList & AnimatedGrid doesn't apply MediaQuery padding #129556 (flutter/flutter#129860) 2023-07-05 [email protected] Change from "created via performance template" to "from: performance template" (flutter/flutter#130035) 2023-07-05 [email protected] Removes deprecated APIs from v2.6 in `binding.dart` and `widget_tester.dart` (flutter/flutter#129663) 2023-07-05 [email protected] Add new hot reload case string (flutter/flutter#130008) 2023-07-05 [email protected] Manual roll Flutter Engine from 987b621eac4e to bd2e42b203e1 (32 revisions) (flutter/flutter#130023) 2023-07-05 [email protected] Add simple unit tests for annotations.dart file (flutter/flutter#128902) 2023-07-05 [email protected] fix a bug when android uses CupertinoPageTransitionsBuilder... (flutter/flutter#114303) 2023-07-05 [email protected] Add .env file support for option `--dart-define-from-file` (flutter/flutter#128668) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
|
Unfortunately, preliminary testing didn't turn up the failures that were caused by the breaking API change here. We've had to revert this PR: #130144. |
…." (flutter#130144) Reverts flutter#114303 The breaking API change in flutter#114303 broke internal tests/apps (Google internal link b/290154304) as well as external dependents: flutter#130062. Fixes flutter#130062
When android uses iOS style
PageTransitionsBuilderand iOS uses android stylePageTransitionsBuilder, on android, swipe from the left edge of the screen doesn't work. This PR solves that problem.#99919 introduced a breaking change, the pr re-implemented it
without introducing a breaking change.**Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.