Do not visit offstage subtrees when hero transition#44890
Do not visit offstage subtrees when hero transition#44890najeira wants to merge 1 commit intoflutter:masterfrom
Conversation
|
I'm going to fix format of test file. |
532bae4 to
74e4efa
Compare
|
fixed and force pushed |
| inviteHero(hero, tag); | ||
| } | ||
| } | ||
| } else if (element.widget is Offstage) { |
There was a problem hiding this comment.
Special-casing the Offstage widget is not a good idea as there may be other widgets that will make a Hero "unreachable". We should see if we can fix this problem in a more generic way.
There was a problem hiding this comment.
Agree, It's not a good design that Hero knows Offstage. However, the current implementation throws exceptions, I think it is a reasonable first aid.
There was a problem hiding this comment.
I agree with @goderbauer. This is actually a violation of the Flutter style guide: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes.
Furthermore, addressing the root issue is preferable to a 'first aid' or temporary solution. What is the root issue here that is causing the exception?
There was a problem hiding this comment.
Root issue: When there are multiple Navigators such as CupertinoPageScaffold, Hero searches for an inactive Navigators that is not related to the Hero-animating being performed. If the same Heroes exists in different Navigators, the active Navigator's Hero should be selected.
There was a problem hiding this comment.
Should each Navigators have a widget like HeroStage that indicates whether or not to participate in a Hero animation and should be switched according to active / inactive? The mechanism is similar to Offstage, but it is a way to explicitly add Hero-related widgets.
|
Having an explicit HeroVisibility widget that allows you to turn off Heroes in a certain subtree seems like a good approach. When the _TabSwitchingViewState marks something as offstage it could then also turn off Heroes in that subtree. |
|
I am going to close this PR. Feel free to open a new one with the other approach. |
Description
When hero transition, no need to visit offstage subtrees.
An error occurs in the following step:
Related Issues
#29069
#28042
Tests
I added the following tests:
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?