Skip to content

Android Q transition by default #82670

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.

Sign up for GitHub

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

Jump to bottom
Merged
fluttergithubbot merged 13 commits into from
Aug 17, 2021
Merged

Android Q transition by default #82670

fluttergithubbot merged 13 commits into from
Aug 17, 2021

Conversation

@AlexV525
Copy link
Member

@AlexV525 AlexV525 commented May 17, 2021 •
edited
Loading

Continued from #51538 , resolves #43277 .

According to Style guide , we might need to keep the same transition behavior with the latest Android.

It should be a breaking change since it's an update with the fundamental part of Flutter.

Before After
Before After

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 feature I am adding, or Hixie said the PR is test-exempt.
  • All existing and new tests are passing.

@flutter-dashboard flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels May 17, 2021
@google-cla google-cla bot added the cla: yes label May 17, 2021
AlexV525
AlexV525 commented May 17, 2021
View reviewed changes
@AlexV525 AlexV525 mentioned this pull request May 18, 2021
Closed
8 tasks
@AlexV525
Copy link
Member Author

AlexV525 commented May 19, 2021

By replacing with the new transition, the "Hero pop transition interrupted by a push" test is no longer needed, or should be only tested with FadeUpwardsPageTransitionsBuilder. So I update this to force using the fade transition builder. cc @HansMuller who wrote this test.

flutter/packages/flutter/test/widgets/heroes_test.dart

Line 729 in 7b0fb83

testWidgets('Hero pop transition interrupted by a push', (WidgetTester tester) async {

Android.Emulator.-.Pixel_4_XL_API_30_5554.2021-05-19.22-01-16.mp4

This was referenced May 19, 2021
Merged
Merged
@AlexV525 AlexV525 marked this pull request as ready for review May 20, 2021 09:11
@AlexV525 AlexV525 requested review from goderbauer and shihaohong May 20, 2021 09:12
@AlexV525
Copy link
Member Author

AlexV525 commented May 20, 2021

Regarding internal tests that broken previously, I'm assuming they are all affected by the finder's scope, which require to migrate finders to descendants. It won't be too hard to migrate them. :)

@AlexV525 AlexV525 requested review from chunhtai and darrenaustin May 29, 2021 05:49
@AlexV525
Copy link
Member Author

AlexV525 commented Jun 3, 2021

cc @goderbauer

@AlexV525 AlexV525 requested review from HansMuller and removed request for darrenaustin June 8, 2021 01:38
@AlexV525
Copy link
Member Author

AlexV525 commented Jun 12, 2021

/cc @HansMuller

@HansMuller
Copy link
Contributor

HansMuller commented Aug 13, 2021

@AlexV525 - sorry it's taken so long to get to this. I will review it tomorrow and it looks like you'll need to update the PR because of the conflict.

@AlexV525 AlexV525 force-pushed the replace-with-zoom-transition branch 2 times, most recently from 368da62 to 45d9853 Compare August 13, 2021 03:53
@AlexV525 AlexV525 force-pushed the replace-with-zoom-transition branch from 45d9853 to 2a3a8dd Compare August 13, 2021 03:57
HansMuller
HansMuller reviewed Aug 13, 2021
View reviewed changes
Copy link
Contributor

@HansMuller HansMuller left a comment

Choose a reason for hiding this comment

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

This looks good. I'm going to try and land it on Tuesday 8/17 since I'm not around until then and we'll need to keep an eye on it; it may break existing internal tests.

@AlexV525
Copy link
Member Author

AlexV525 commented Aug 13, 2021 •
edited
Loading

This looks good. I'm going to try and land it on Tuesday 8/17 since I'm not around until then and we'll need to keep an eye on it; it may break existing internal tests.

Sure, I'll write a breaking change note once the website's infrastructure is stable.

HansMuller
HansMuller approved these changes Aug 17, 2021
View reviewed changes
Copy link
Contributor

@HansMuller HansMuller left a comment

Choose a reason for hiding this comment

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

LGTM

@fluttergithubbot fluttergithubbot merged commit a1ae4fe into flutter:master Aug 17, 2021
@AlexV525
Copy link
Member Author

AlexV525 commented Aug 18, 2021

We used to have a BOT running internal tests, right?

@jmagman jmagman mentioned this pull request Aug 18, 2021
Closed
jmagman added a commit that referenced this pull request Aug 18, 2021
@jmagman
Revert "Android Q transition by default (#82670)"
09cf167 
This reverts commit a1ae4fe.
This was referenced Aug 18, 2021
Merged
Open
jmagman added a commit that referenced this pull request Aug 18, 2021
@jmagman
Revert "Android Q transition by default (#82670)" (#88394)
4053b4b 
This reverts commit a1ae4fe.
AlexV525 added a commit to AlexV525/flutter that referenced this pull request Aug 18, 2021
@AlexV525
Reland "Android Q transition by default (flutter#82670)"
3da7cdc 
This reverts commit 4053b4b.
This was referenced Aug 18, 2021
Closed
Merged
HansMuller pushed a commit that referenced this pull request Aug 18, 2021
@AlexV525
Reland "Android Q transition by default (#82670)" (#88409)
df399f9 
* Reland "Android Q transition by default (#82670)"

This reverts commit 4053b4b.

* Fix `overall_experience_test.dart` for flutter_tools
HansMuller pushed a commit that referenced this pull request Aug 18, 2021
Revert "Reland "Android Q transition by default (#82670)" (#88409)"
aa1dd36 
This reverts commit df399f9.
@AlexV525 AlexV525 mentioned this pull request Aug 19, 2021
Closed
fluttergithubbot pushed a commit that referenced this pull request Aug 19, 2021
@HansMuller
Revert "Reland "Android Q transition by default (#82670)" (#88409)" (#…
afe58fb 
…88482)
blasten pushed a commit to blasten/flutter that referenced this pull request Aug 19, 2021
@AlexV525
Android Q transition by default (flutter#82670)
bcd8984
blasten pushed a commit to blasten/flutter that referenced this pull request Aug 19, 2021
@jmagman
Revert "Android Q transition by default (flutter#82670)" (flutter#88394)
a3a9046 
This reverts commit a1ae4fe.
blasten pushed a commit to blasten/flutter that referenced this pull request Aug 19, 2021
@AlexV525
Reland "Android Q transition by default (flutter#82670)" (flutter#88409)
b62730d 
* Reland "Android Q transition by default (flutter#82670)"

This reverts commit 4053b4b.

* Fix `overall_experience_test.dart` for flutter_tools
blasten pushed a commit to blasten/flutter that referenced this pull request Aug 19, 2021
@HansMuller
Revert "Reland "Android Q transition by default (flutter#82670)" (flu…
f69d9fe 
…tter#88409)" (flutter#88482)
This was referenced Jan 10, 2022
Merged
Closed
@AlexV525 AlexV525 deleted the replace-with-zoom-transition branch June 16, 2022 11:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@shihaohong shihaohong Awaiting requested review from shihaohong

@goderbauer goderbauer Awaiting requested review from goderbauer

@chunhtai chunhtai Awaiting requested review from chunhtai

1 more reviewer

@HansMuller HansMuller HansMuller approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Make Android 10 Activity Animation the default animation

3 participants

@AlexV525 @HansMuller @fluttergithubbot