Skip to content

Commit 63ee971

Browse files
davidmiglozdavidmigloz
authored
[go_router] Fix ShellRoute chrome dropped from semantics tree by ModalBarrier (#12353)
Shell chrome painted before a `ShellRoute` or `StatefulShellRoute` navigator (a side rail, or an app bar in a `Row`/`Column` based shell) disappears from the semantics tree. Screen readers cannot reach it at all: the nodes are not merely unnamed, they do not exist. The mechanism: every `ModalRoute` builds a `ModalBarrier` wrapped in `BlockSemantics`, which drops the semantics of everything painted before it up to the nearest semantics boundary. The nested `Navigator` that go_router builds for shell routes does not establish such a boundary, so the block escapes the shell's navigator and prunes the shell's own chrome. Bottom-nav shells are unaffected only because `Scaffold` happens to paint its body before its bars. This PR wraps the navigator built for `ShellRoute`/`StatefulShellRoute` branches in `Semantics(container: true)`, which contains the block. The root navigator is left unwrapped, since it has no earlier-painted siblings by construction. This is the workaround a framework team member confirmed on the linked issue; applying it inside go_router fixes it for every shell consumer without app-side patches. Semantics tree of a minimal repro (a `Row` shell: 220px sidebar with three nav buttons, routed content on the right), before and after, captured with `debugDumpSemanticsTree`: <details> <summary>Before: 6 nodes, the entire sidebar subtree is missing</summary> ``` SemanticsNode#0 │ Rect.fromLTRB(0.0, 0.0, 2400.0, 1800.0) │ └─SemanticsNode#1 │ Rect.fromLTRB(0.0, 0.0, 800.0, 600.0) scaled by 3.0x │ textDirection: ltr │ sortKey: OrdinalSortKey#39327(order: 0.0) │ └─SemanticsNode#2 │ Rect.fromLTRB(0.0, 0.0, 800.0, 600.0) │ flags: scopesRoute │ └─SemanticsNode#3 │ Rect.fromLTRB(221.0, 0.0, 800.0, 600.0) │ sortKey: OrdinalSortKey#39327(order: 0.0) │ └─SemanticsNode#4 │ Rect.fromLTRB(0.0, 0.0, 579.0, 600.0) │ flags: scopesRoute │ └─SemanticsNode#5 Rect.fromLTRB(85.5, 284.0, 493.5, 316.0) label: "Dashboard content" textDirection: ltr ``` Node `#3` starts at `x=221`, right of the 220px sidebar plus a 1px divider. There is no node anywhere for the sidebar: no title, no navigation container, no buttons. The sidebar is painted before the `/dashboard` route (`#4`, `scopesRoute`) inside the same enclosing semantics scope, which is exactly what that route's `BlockSemantics` drops. </details> <details> <summary>After: sidebar fully present, routed content unchanged (two nodes for the repro's own toggle switch omitted for brevity)</summary> ``` SemanticsNode#0 │ Rect.fromLTRB(0.0, 0.0, 2400.0, 1800.0) │ └─SemanticsNode#1 │ Rect.fromLTRB(0.0, 0.0, 800.0, 600.0) scaled by 3.0x │ textDirection: ltr │ sortKey: OrdinalSortKey#39327(order: 0.0) │ └─SemanticsNode#2 │ Rect.fromLTRB(0.0, 0.0, 800.0, 600.0) │ flags: scopesRoute │ ├─SemanticsNode#3 │ Rect.fromLTRB(16.0, 16.0, 204.0, 76.0) │ label: "Nested Navigator Semantics" │ textDirection: ltr │ ├─SemanticsNode#4 │ │ Rect.fromLTRB(0.0, 92.0, 220.0, 260.0) │ │ label: "Main navigation" │ │ textDirection: ltr │ │ │ ├─SemanticsNode#5 │ │ Rect.fromLTRB(0.0, 0.0, 220.0, 56.0) │ │ actions: focus, tap │ │ flags: isSelected, isButton, hasEnabledState, isEnabled, │ │ isFocusable, hasSelectedState │ │ label: "Dashboard" │ │ textDirection: ltr │ │ │ ├─SemanticsNode#6 │ │ Rect.fromLTRB(0.0, 56.0, 220.0, 112.0) │ │ actions: focus, tap │ │ flags: isButton, hasEnabledState, isEnabled, isFocusable, │ │ hasSelectedState │ │ label: "Settings" │ │ textDirection: ltr │ │ │ └─SemanticsNode#7 │ Rect.fromLTRB(0.0, 112.0, 220.0, 168.0) │ actions: focus, tap │ flags: isButton, hasEnabledState, isEnabled, isFocusable, │ hasSelectedState │ label: "Reports" │ textDirection: ltr │ └─SemanticsNode#10 │ Rect.fromLTRB(221.0, 0.0, 800.0, 600.0) │ sortKey: OrdinalSortKey#39327(order: 0.0) │ └─SemanticsNode#11 │ Rect.fromLTRB(0.0, 0.0, 579.0, 600.0) │ flags: scopesRoute │ └─SemanticsNode#12 Rect.fromLTRB(85.5, 284.0, 493.5, 316.0) label: "Dashboard content" textDirection: ltr ``` The routed content node ("Dashboard content") is byte-identical in both dumps. The fix does not change the routed content's semantics, only whether the chrome painted before the shell navigator survives alongside it. </details> Notes for review: - Tests: the fix commit adds a `Shell navigator semantics boundary` group to `builder_test.dart` (chrome survives, structural wrap present, root navigator not wrapped), and a second commit adds a `StatefulShellRoute.indexedStack` regression test covering branch switching. Removing the wrap makes the chrome tests fail with `Found 0 widgets with a semantics label`. - Version/CHANGELOG: go_router uses batch release, so this PR adds a file under `pending_changelogs/` (`version: patch`) instead of touching `pubspec.yaml` or `CHANGELOG.md`. - Interaction with flutter/flutter#181519 (replacing `BlockSemantics` in modal routes with `AccessibilityFocusBlockType.blockSubtree`): the fix establishes a semantics container boundary at the shell navigator, which is where a nested navigator should scope its routes' blocking regardless of the blocking mechanism. If that migration later makes the containment unnecessary, the wrap stays harmless. Fixes flutter/flutter#135656 Related: flutter/flutter#55758, flutter/flutter#150978 ## Pre-Review Checklist [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
1 parent 0dfacae commit 63ee971

3 files changed

Lines changed: 234 additions & 8 deletions

File tree

‎packages/go_router/lib/src/builder.dart‎

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -137,11 +137,25 @@ class _CustomNavigator extends StatefulWidget {
137137
required this.errorBuilder,
138138
required this.errorPageBuilder,
139139
required this.requestFocus,
140+
this.isShellNavigator = false,
140141
});
141142

142143
final GlobalKey<NavigatorState> navigatorKey;
143144
final List<NavigatorObserver> observers;
144145

146+
/// Whether this navigator builds the nested Navigator for a
147+
/// [ShellRoute]/[StatefulShellRoute] branch, as opposed to the root
148+
/// [GoRouter] navigator.
149+
///
150+
/// Shell navigators are wrapped in `Semantics(container: true)` so that
151+
/// each route's [ModalBarrier] (which blocks the semantics of
152+
/// previously-painted siblings up to the nearest semantics boundary)
153+
/// cannot reach past the shell's Navigator and drop shell chrome that
154+
/// paints before it (e.g. a side rail or app bar in a `Row`/`Column`
155+
/// shell). The root navigator has no earlier-painted siblings by
156+
/// construction, so it does not need the same containment.
157+
final bool isShellNavigator;
158+
145159
/// The actual [RouteMatchBase]s to be built.
146160
///
147161
/// This can be different from matches in [matchList] if this widget is used
@@ -315,6 +329,7 @@ class _CustomNavigatorState extends State<_CustomNavigator> {
315329
errorBuilder: widget.errorBuilder,
316330
errorPageBuilder: widget.errorPageBuilder,
317331
requestFocus: widget.requestFocus,
332+
isShellNavigator: true,
318333
),
319334
);
320335
},
@@ -443,18 +458,24 @@ class _CustomNavigatorState extends State<_CustomNavigator> {
443458
_updatePages(context);
444459
}
445460
assert(_pages != null);
461+
final navigator = Navigator(
462+
key: widget.navigatorKey,
463+
requestFocus: widget.requestFocus,
464+
restorationScopeId: widget.navigatorRestorationId,
465+
pages: _pages!,
466+
observers: widget.observers,
467+
onPopPage: _handlePopPage,
468+
);
446469
return GoRouterStateRegistryScope(
447470
registry: _registry,
448471
child: HeroControllerScope(
449472
controller: _controller!,
450-
child: Navigator(
451-
key: widget.navigatorKey,
452-
requestFocus: widget.requestFocus,
453-
restorationScopeId: widget.navigatorRestorationId,
454-
pages: _pages!,
455-
observers: widget.observers,
456-
onPopPage: _handlePopPage,
457-
),
473+
// A Navigator does not establish a semantics boundary, so a route's
474+
// ModalBarrier (wrapped in BlockSemantics) can otherwise drop the
475+
// semantics of shell chrome painted before this navigator (e.g. a
476+
// side rail in a Row-based ShellRoute shell). See
477+
// https://github.com/flutter/flutter/issues/135656.
478+
child: widget.isShellNavigator ? Semantics(container: true, child: navigator) : navigator,
458479
),
459480
);
460481
}

‎packages/go_router/pending_changelogs/shellroute_navigator_semantics_boundary.yaml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
changelog: |
2+
- Fixes `ShellRoute`/`StatefulShellRoute` shell chrome (e.g. a side rail or app bar painted before the routed child) being dropped from the semantics tree by the active route's `ModalBarrier`.
3+
version: patch

‎packages/go_router/test/builder_test.dart‎

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -380,6 +380,208 @@ void main() {
380380
final Navigator navigator = tester.widget<Navigator>(find.byType(Navigator));
381381
expect(navigator.requestFocus, isFalse);
382382
});
383+
384+
group('Shell navigator semantics boundary', () {
385+
testWidgets('Chrome painted before a ShellRoute Navigator stays in the '
386+
'semantics tree alongside routed content', (WidgetTester tester) async {
387+
final SemanticsHandle semantics = tester.ensureSemantics();
388+
389+
final router = GoRouter(
390+
initialLocation: '/a',
391+
routes: <RouteBase>[
392+
ShellRoute(
393+
builder: (BuildContext context, GoRouterState state, Widget child) {
394+
return Row(
395+
children: <Widget>[
396+
Semantics(
397+
container: true,
398+
label: 'chrome',
399+
child: const SizedBox(width: 40, height: 40),
400+
),
401+
Expanded(child: child),
402+
],
403+
);
404+
},
405+
routes: <RouteBase>[
406+
GoRoute(
407+
path: '/a',
408+
builder: (BuildContext context, GoRouterState state) {
409+
return const Text('content-a');
410+
},
411+
),
412+
GoRoute(
413+
path: '/b',
414+
builder: (BuildContext context, GoRouterState state) {
415+
return const Text('content-b');
416+
},
417+
),
418+
],
419+
),
420+
],
421+
);
422+
addTearDown(router.dispose);
423+
424+
await tester.pumpWidget(MaterialApp.router(routerConfig: router));
425+
await tester.pumpAndSettle();
426+
427+
// Without the Semantics(container: true) wrap around the shell's
428+
// Navigator, the route's ModalBarrier (BlockSemantics) drops the
429+
// chrome painted before it, because a Navigator does not
430+
// establish a semantics boundary. See
431+
// https://github.com/flutter/flutter/issues/135656.
432+
expect(find.bySemanticsLabel('chrome'), findsOneWidget);
433+
expect(find.bySemanticsLabel('content-a'), findsOneWidget);
434+
435+
// Navigation between two routes within the same shell keeps
436+
// working, and the chrome remains exposed after the rebuild.
437+
router.go('/b');
438+
await tester.pumpAndSettle();
439+
440+
expect(find.bySemanticsLabel('chrome'), findsOneWidget);
441+
expect(find.bySemanticsLabel('content-b'), findsOneWidget);
442+
expect(find.bySemanticsLabel('content-a'), findsNothing);
443+
444+
semantics.dispose();
445+
});
446+
447+
testWidgets('ShellRoute Navigator is wrapped in a Semantics(container: true) '
448+
'boundary', (WidgetTester tester) async {
449+
final shellNavigatorKey = GlobalKey<NavigatorState>();
450+
final router = GoRouter(
451+
initialLocation: '/',
452+
routes: <RouteBase>[
453+
ShellRoute(
454+
navigatorKey: shellNavigatorKey,
455+
builder: (BuildContext context, GoRouterState state, Widget child) {
456+
return child;
457+
},
458+
routes: <RouteBase>[
459+
GoRoute(
460+
path: '/',
461+
builder: (BuildContext context, GoRouterState state) {
462+
return const Text('content');
463+
},
464+
),
465+
],
466+
),
467+
],
468+
);
469+
addTearDown(router.dispose);
470+
471+
await tester.pumpWidget(MaterialApp.router(routerConfig: router));
472+
await tester.pumpAndSettle();
473+
474+
final Iterable<Semantics> ancestorSemantics = tester.widgetList<Semantics>(
475+
find.ancestor(of: find.byKey(shellNavigatorKey), matching: find.byType(Semantics)),
476+
);
477+
expect(
478+
ancestorSemantics.first.container,
479+
isTrue,
480+
reason:
481+
'The nearest Semantics ancestor of the shell Navigator '
482+
'should be the container boundary added by go_router.',
483+
);
484+
});
485+
486+
testWidgets('Root GoRouter Navigator is not wrapped in an extra Semantics '
487+
'container boundary', (WidgetTester tester) async {
488+
final rootNavigatorKey = GlobalKey<NavigatorState>();
489+
final router = GoRouter(
490+
navigatorKey: rootNavigatorKey,
491+
initialLocation: '/',
492+
routes: <RouteBase>[
493+
GoRoute(
494+
path: '/',
495+
builder: (BuildContext context, GoRouterState state) {
496+
return const Text('content');
497+
},
498+
),
499+
],
500+
);
501+
addTearDown(router.dispose);
502+
503+
await tester.pumpWidget(MaterialApp.router(routerConfig: router));
504+
await tester.pumpAndSettle();
505+
506+
// The root navigator has no earlier-painted siblings by
507+
// construction, so go_router must not add the shell-only
508+
// Semantics(container: true) wrap around it.
509+
final Finder wrapper = find.ancestor(
510+
of: find.byKey(rootNavigatorKey),
511+
matching: find.byWidgetPredicate(
512+
(Widget widget) => widget is Semantics && widget.container,
513+
),
514+
);
515+
expect(wrapper, findsNothing);
516+
});
517+
518+
testWidgets('Chrome painted before a StatefulShellRoute Navigator stays in the '
519+
'semantics tree alongside routed content', (WidgetTester tester) async {
520+
final SemanticsHandle semantics = tester.ensureSemantics();
521+
StatefulNavigationShell? navigationShell;
522+
523+
final router = GoRouter(
524+
initialLocation: '/a',
525+
routes: <RouteBase>[
526+
StatefulShellRoute.indexedStack(
527+
builder: (BuildContext context, GoRouterState state, StatefulNavigationShell shell) {
528+
navigationShell = shell;
529+
return Column(
530+
children: <Widget>[
531+
Semantics(container: true, label: 'chrome', child: const SizedBox(height: 10)),
532+
Expanded(child: shell),
533+
],
534+
);
535+
},
536+
branches: <StatefulShellBranch>[
537+
StatefulShellBranch(
538+
routes: <RouteBase>[
539+
GoRoute(
540+
path: '/a',
541+
builder: (BuildContext context, GoRouterState state) {
542+
return const Text('content-a');
543+
},
544+
),
545+
],
546+
),
547+
StatefulShellBranch(
548+
routes: <RouteBase>[
549+
GoRoute(
550+
path: '/b',
551+
builder: (BuildContext context, GoRouterState state) {
552+
return const Text('content-b');
553+
},
554+
),
555+
],
556+
),
557+
],
558+
),
559+
],
560+
);
561+
addTearDown(router.dispose);
562+
563+
await tester.pumpWidget(MaterialApp.router(routerConfig: router));
564+
await tester.pumpAndSettle();
565+
566+
// Without the Semantics(container: true) wrap around the shell
567+
// branch's Navigator, the route's ModalBarrier (BlockSemantics)
568+
// drops the chrome painted before it, because a Navigator does not
569+
// establish a semantics boundary. See
570+
// https://github.com/flutter/flutter/issues/135656.
571+
expect(find.bySemanticsLabel('chrome'), findsOneWidget);
572+
expect(find.bySemanticsLabel('content-a'), findsOneWidget);
573+
574+
// Switching branches keeps the chrome exposed after the rebuild.
575+
navigationShell!.goBranch(1);
576+
await tester.pumpAndSettle();
577+
578+
expect(find.bySemanticsLabel('chrome'), findsOneWidget);
579+
expect(find.bySemanticsLabel('content-b'), findsOneWidget);
580+
expect(find.bySemanticsLabel('content-a'), findsNothing);
581+
582+
semantics.dispose();
583+
});
584+
});
383585
});
384586
}
385587

0 commit comments

Comments
 (0)