[go_router] Detect both framework and material_ui/cupertino_ui apps for Hero controller - #12695
Open
yashas-hm wants to merge 5 commits into
Open
[go_router] Detect both framework and material_ui/cupertino_ui apps for Hero controller#12695yashas-hm wants to merge 5 commits into
yashas-hm wants to merge 5 commits into
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates go_router to version 18.0.1, resolving an issue where Hero flight animations failed to play for nested routes inside shell routes when using Flutter's framework MaterialApp or CupertinoApp. The fix updates isCupertinoApp and isMaterialApp to check for both the framework-specific and package-specific variants of these widgets, and adds corresponding regression tests. The reviewer suggests reordering these checks to prioritize the framework-specific types first, allowing the lookup to short-circuit earlier for most applications and improve efficiency.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
go_router 18.0.0 migrated its Material/Cupertino helpers to the
material_ui/cupertino_uipackages. As part of that migration,isMaterialApp/isCupertinoApp(inlib/src/pages/material.dartandlib/src/pages/cupertino.dart)resolve
MaterialApp/CupertinoAppto thematerial_ui/cupertino_uitypesand detect the app with
findAncestorWidgetOfExactType.Because that is an exact type match, an app built with the framework's own
MaterialApp/CupertinoApp(frompackage:flutter/material.dart/package:flutter/cupertino.dart) is no longer detected.RouteBuilderthen fallsthrough to a bare
HeroController()instead ofcreateMaterialHeroController()/createCupertinoHeroController(), and Hero animations silently stop flying insideShellRoute/StatefulShellRoute. There is no compile error and no warning — theanimation just disappears.
This is a transition-period problem: while Material and Cupertino are being decoupled
from the SDK (flutter/flutter#184093),
package:flutter/material.dartremains fullysupported and is what the vast majority of apps still use.
This PR makes app-type detection recognize both the framework's
MaterialApp/CupertinoAppand thematerial_ui/cupertino_uivariants, so thecorrect
HeroControlleris installed regardless of which Material/Cupertino librarythe app is built with. A code comment notes the framework check can be removed once
package:flutter/material.dartis sunset.Related Issues
Fixes flutter/flutter#192043
Context: flutter/flutter#184093 (Material/Cupertino decoupling)
Tests
createMaterialHeroController()/createCupertinoHeroController()for both aframework
MaterialApp/CupertinoAppand amaterial_ui/cupertino_uiMaterialApp/CupertinoApp.Pre-Review Checklist
[shared_preferences]///).