Skip to content

[go_router] Detect both framework and material_ui/cupertino_ui apps for Hero controller - #12695

Open
yashas-hm wants to merge 5 commits into
flutter:mainfrom
yashas-hm-os:fix-issue-192043
Open

[go_router] Detect both framework and material_ui/cupertino_ui apps for Hero controller#12695
yashas-hm wants to merge 5 commits into
flutter:mainfrom
yashas-hm-os:fix-issue-192043

Conversation

@yashas-hm

Copy link
Copy Markdown
Contributor

Description

go_router 18.0.0 migrated its Material/Cupertino helpers to the material_ui /
cupertino_ui packages. As part of that migration, isMaterialApp /
isCupertinoApp (in lib/src/pages/material.dart and lib/src/pages/cupertino.dart)
resolve MaterialApp / CupertinoApp to the material_ui / cupertino_ui types
and detect the app with findAncestorWidgetOfExactType.

Because that is an exact type match, an app built with the framework's own
MaterialApp / CupertinoApp (from package:flutter/material.dart /
package:flutter/cupertino.dart) is no longer detected. RouteBuilder then falls
through to a bare HeroController() instead of createMaterialHeroController() /
createCupertinoHeroController(), and Hero animations silently stop flying inside
ShellRoute / StatefulShellRoute. There is no compile error and no warning — the
animation just disappears.

This is a transition-period problem: while Material and Cupertino are being decoupled
from the SDK (flutter/flutter#184093), package:flutter/material.dart remains fully
supported and is what the vast majority of apps still use.

This PR makes app-type detection recognize both the framework's
MaterialApp / CupertinoApp and the material_ui / cupertino_ui variants, so the
correct HeroController is installed regardless of which Material/Cupertino library
the app is built with. A code comment notes the framework check can be removed once
package:flutter/material.dart is sunset.

Related Issues

Fixes flutter/flutter#192043
Context: flutter/flutter#184093 (Material/Cupertino decoupling)

Tests

  • Added regression tests asserting that a shell-route app selects
    createMaterialHeroController() / createCupertinoHeroController() for both a
    framework MaterialApp / CupertinoApp and a material_ui / cupertino_ui
    MaterialApp / CupertinoApp.
  • Existing go_router tests continue to pass.

Pre-Review Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools.
  • I read the [Tree Hygiene] page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran [the auto-formatter].
  • I signed the [CLA].
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I [linked to at least one issue that this PR fixes] in the description above.
  • I followed [the version and CHANGELOG instructions], using [semantic versioning] and the [repository CHANGELOG style], or I have commented below to indicate which documented exception this PR falls under[^1].
  • I updated/added any relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or I have commented below to indicate which [test exemption] this PR falls under[^1].
  • All existing and new tests are passing.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Comment thread packages/go_router/lib/src/pages/cupertino.dart Outdated
Comment thread packages/go_router/lib/src/pages/material.dart Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[go_router] Hero animation regression between 17.5.0 and 18.0.0 in ShellRoute / StatefulShellRoute

1 participant