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

AUTHORS

@@ -86,3 +86,4 @@ LeanCode <contribution@leancode.pl>
 Piotr Denert <pdenert09@gmail.com>
 Marcin Chudy <marcinchudy94@gmail.com>
 Paweł Jakubowski <pawel.jakubowski@leancode.pl>
+Yashas H Majmudar <yashashm.dev@gmail.com>

packages/go_router/AUTHORS

@@ -6,3 +6,4 @@
 Google Inc.
 csells@sellsbrothers.com
 Hashir Shoaib <hashirshoaeb@gmail.com>
+Yashas H Majmudar <yashashm.dev@gmail.com>

packages/go_router/CHANGELOG.md

@@ -1,3 +1,11 @@
+## 18.0.1
+
+- Fixes Hero flight animations not playing for routes nested inside a
+  `ShellRoute` or `StatefulShellRoute` when the app uses Flutter's `MaterialApp`
+  or `CupertinoApp`. App-type detection is now package-agnostic, matching both
+  Flutter's `MaterialApp`/`CupertinoApp` and the `material_ui`/`cupertino_ui`
+  variants.
+
 ## 18.0.0
 
 - Migrates to material_ui and cupertino_ui.

packages/go_router/lib/src/pages/cupertino.dart

@@ -4,11 +4,21 @@
 
 // ignore_for_file: diagnostic_describe_all_properties
 
+// Unprefixed [CupertinoApp] is cupertino_ui's; the framework's is aliased.
 import 'package:cupertino_ui/cupertino_ui.dart';
+import 'package:flutter/cupertino.dart' as flutter_cupertino;
+
 import '../misc/extensions.dart';
 
 /// Checks for CupertinoApp in the widget tree.
+///
+/// During the Material/Cupertino decoupling (flutter/flutter#184093) an app may
+/// use the framework's or cupertino_ui's [CupertinoApp] — distinct types, and
+/// [findAncestorWidgetOfExactType] matches only one — so both are checked to
+/// install the right HeroController for shell-route Hero flights
+/// (flutter/flutter#192043). Drop the framework check once it is sunset.
 bool isCupertinoApp(BuildContext context) =>
+    context.findAncestorWidgetOfExactType<flutter_cupertino.CupertinoApp>() != null ||
     context.findAncestorWidgetOfExactType<CupertinoApp>() != null;
 
 /// Creates a Cupertino HeroController.

packages/go_router/lib/src/pages/material.dart

@@ -4,12 +4,21 @@
 
 // ignore_for_file: diagnostic_describe_all_properties
 
+// Unprefixed [MaterialApp] is material_ui's; the framework's is aliased.
+import 'package:flutter/material.dart' as flutter_material;
 import 'package:material_ui/material_ui.dart';
 
 import '../misc/extensions.dart';
 
 /// Checks for MaterialApp in the widget tree.
+///
+/// During the Material/Cupertino decoupling (flutter/flutter#184093) an app may
+/// use the framework's or material_ui's [MaterialApp] — distinct types, and
+/// [findAncestorWidgetOfExactType] matches only one — so both are checked to
+/// install the right HeroController for shell-route Hero flights
+/// (flutter/flutter#192043). Drop the framework check once it is sunset.
 bool isMaterialApp(BuildContext context) =>
+    context.findAncestorWidgetOfExactType<flutter_material.MaterialApp>() != null ||
     context.findAncestorWidgetOfExactType<MaterialApp>() != null;
 
 /// Creates a Material HeroController.

packages/go_router/pubspec.yaml

@@ -1,7 +1,7 @@
 name: go_router
 description: A declarative router for Flutter based on Navigation 2 supporting
   deep linking, data-driven routes and more
-version: 18.0.0
+version: 18.0.1
 repository: https://github.com/flutter/packages/tree/main/packages/go_router
 issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+go_router%22
 

packages/go_router/test/cupertino_test.dart

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import 'package:cupertino_ui/cupertino_ui.dart';
+import 'package:flutter/cupertino.dart' as flutter_cupertino;
 import 'package:flutter_test/flutter_test.dart';
 import 'package:go_router/src/pages/cupertino.dart';
 import 'package:material_ui/material_ui.dart';
@@ -18,6 +19,18 @@ void main() {
       expect(isCupertino, true);
     });
 
+    testWidgets("returns [true] when Flutter's CupertinoApp is present", (
+      WidgetTester tester,
+    ) async {
+      // Regression test for https://github.com/flutter/flutter/issues/192043.
+      // A standard Flutter app uses package:flutter/cupertino.dart's
+      // CupertinoApp, which is a distinct type from cupertino_ui's CupertinoApp.
+      final key = GlobalKey<_DummyStatefulWidgetState>();
+      await tester.pumpWidget(flutter_cupertino.CupertinoApp(home: DummyStatefulWidget(key: key)));
+      final bool isCupertino = isCupertinoApp(key.currentContext! as Element);
+      expect(isCupertino, true);
+    });
+
     testWidgets('returns [false] when MaterialApp is present', (WidgetTester tester) async {
       final key = GlobalKey<_DummyStatefulWidgetState>();
       await tester.pumpWidget(MaterialApp(home: DummyStatefulWidget(key: key)));

packages/go_router/test/hero_controller_test.dart

@@ -0,0 +1,114 @@
+// Copyright 2013 The Flutter Authors
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+// Regression tests for https://github.com/flutter/flutter/issues/192043.
+//
+// go_router migrated its material/cupertino helpers to the material_ui /
+// cupertino_ui packages. Those packages declare their own MaterialApp /
+// CupertinoApp types, distinct from the ones in package:flutter. A standard
+// Flutter app is built with package:flutter's MaterialApp, so app-type
+// detection based on findAncestorWidgetOfExactType stopped matching and the
+// navigators go_router builds (including the nested navigators of a ShellRoute /
+// StatefulShellRoute) fell back to a plain HeroController. That broke Hero
+// flight animations for routes nested inside a shell when the surrounding app is
+// Flutter's MaterialApp.
+//
+// These tests build a router under a real Flutter MaterialApp and assert that
+// go_router installs its Material HeroController for every navigator it creates
+// (the root navigator plus the shell's nested navigator), rather than a plain
+// one. go_router's Material controller is identified by the material_ui
+// MaterialRectArcTween it produces, which is distinct from the arc tween used by
+// Flutter's own MaterialApp.
+
+import 'package:flutter/material.dart' as flutter_material;
+import 'package:flutter/widgets.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:go_router/go_router.dart';
+import 'package:leak_tracker_flutter_testing/leak_tracker_flutter_testing.dart';
+import 'package:material_ui/material_ui.dart' show MaterialRectArcTween;
+
+// Flutter's own MaterialApp / CupertinoApp (as opposed to the leak-clean
+// material_ui / cupertino_ui variants) report framework-owned objects to the
+// leak tracker that are outside go_router's control, so leak tracking is
+// disabled for these regression tests.
+final LeakTesting _ignoreLeaks = LeakTesting.settings.withIgnoredAll();
+
+/// Counts the [HeroControllerScope]s in the tree whose controller is the one
+/// created by go_router's [createMaterialHeroController], identified by the
+/// [material_ui] [MaterialRectArcTween] it produces.
+int _goRouterMaterialControllerCount(WidgetTester tester) {
+  var count = 0;
+  for (final Element element in find.byType(HeroControllerScope).evaluate()) {
+    final HeroController? controller = (element.widget as HeroControllerScope).controller;
+    final CreateRectTween? createRectTween = controller?.createRectTween;
+    if (createRectTween != null && createRectTween(Rect.zero, Rect.zero) is MaterialRectArcTween) {
+      count++;
+    }
+  }
+  return count;
+}
+
+void main() {
+  testWidgets('ShellRoute navigators get the Material HeroController under a Flutter MaterialApp', (
+    WidgetTester tester,
+  ) async {
+    final router = GoRouter(
+      initialLocation: '/a',
+      routes: <RouteBase>[
+        ShellRoute(
+          builder: (BuildContext context, GoRouterState state, Widget child) => child,
+          routes: <RouteBase>[
+            GoRoute(
+              path: '/a',
+              builder: (BuildContext context, GoRouterState state) => const SizedBox(),
+            ),
+          ],
+        ),
+      ],
+    );
+    addTearDown(router.dispose);
+
+    await tester.pumpWidget(flutter_material.MaterialApp.router(routerConfig: router));
+    await tester.pumpAndSettle();
+
+    // Both the root navigator and the shell's nested navigator must use the
+    // Material HeroController. Before the fix, app-type detection failed and
+    // these fell back to a plain HeroController, so the count would be 0.
+    expect(_goRouterMaterialControllerCount(tester), 2);
+  }, experimentalLeakTesting: _ignoreLeaks);
+
+  testWidgets(
+    'StatefulShellRoute navigators get the Material HeroController under a Flutter MaterialApp',
+    (WidgetTester tester) async {
+      final router = GoRouter(
+        initialLocation: '/a',
+        routes: <RouteBase>[
+          StatefulShellRoute.indexedStack(
+            builder: (BuildContext context, GoRouterState state, StatefulNavigationShell shell) =>
+                shell,
+            branches: <StatefulShellBranch>[
+              StatefulShellBranch(
+                routes: <RouteBase>[
+                  GoRoute(
+                    path: '/a',
+                    builder: (BuildContext context, GoRouterState state) => const SizedBox(),
+                  ),
+                ],
+              ),
+            ],
+          ),
+        ],
+      );
+      addTearDown(router.dispose);
+
+      await tester.pumpWidget(flutter_material.MaterialApp.router(routerConfig: router));
+      await tester.pumpAndSettle();
+
+      // The root navigator and the branch's nested navigator must both use the
+      // Material HeroController. Before the fix, the count would be 0.
+      expect(_goRouterMaterialControllerCount(tester), 2);
+    },
+    experimentalLeakTesting: _ignoreLeaks,
+  );
+}

packages/go_router/test/material_test.dart

@@ -3,6 +3,7 @@
 // found in the LICENSE file.
 
 import 'package:cupertino_ui/cupertino_ui.dart';
+import 'package:flutter/material.dart' as flutter_material;
 import 'package:flutter_test/flutter_test.dart';
 import 'package:go_router/src/pages/material.dart';
 import 'package:material_ui/material_ui.dart';
@@ -11,13 +12,30 @@ import 'helpers/error_screen_helpers.dart';
 
 void main() {
   group('isMaterialApp', () {
-    testWidgets('returns [true] when MaterialApp is present', (WidgetTester tester) async {
+    testWidgets('returns [true] when the material_ui MaterialApp is present', (
+      WidgetTester tester,
+    ) async {
       final key = GlobalKey<_DummyStatefulWidgetState>();
       await tester.pumpWidget(MaterialApp(home: DummyStatefulWidget(key: key)));
       final bool isMaterial = isMaterialApp(key.currentContext! as Element);
       expect(isMaterial, true);
     });
 
+    testWidgets('returns [true] when the framework MaterialApp is present', (
+      WidgetTester tester,
+    ) async {
+      // Regression test for the Material/Cupertino decoupling
+      // (https://github.com/flutter/flutter/issues/184093): a standard Flutter
+      // app uses package:flutter/material.dart's MaterialApp, which is a
+      // distinct type from material_ui's MaterialApp. Detection must recognize
+      // both so shell-route Hero flights keep working
+      // (https://github.com/flutter/flutter/issues/192043).
+      final key = GlobalKey<_DummyStatefulWidgetState>();
+      await tester.pumpWidget(flutter_material.MaterialApp(home: DummyStatefulWidget(key: key)));
+      final bool isMaterial = isMaterialApp(key.currentContext! as Element);
+      expect(isMaterial, true);
+    });
+
     testWidgets('returns [false] when CupertinoApp is present', (WidgetTester tester) async {
       final key = GlobalKey<_DummyStatefulWidgetState>();
       await tester.pumpWidget(CupertinoApp(home: DummyStatefulWidget(key: key)));