Repository navigation
[go_router] Expose Navigator clipBehavior on ShellRoute and StatefulShellBranch - #12646
Conversation
…hellBranch Nested shell Navigators always clipped their contents, so a sub-route could not paint outside the bounds the shell laid out for it (a box shadow or an overflowing menu got cut off). Adds a `clipBehavior` parameter to `ShellRoute` and `StatefulShellBranch` that is forwarded to the `Navigator` each of them builds. It defaults to `Clip.hardEdge`, which is the `Navigator` default, so existing behavior is unchanged. Branches configure this individually because each `StatefulShellBranch` builds its own `Navigator`, matching how `observers` and `restorationScopeId` already work.
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
github-actions
Bot
added
p: go_router
triage-framework
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a clipBehavior property to ShellRoute and StatefulShellBranch in go_router, which is forwarded to the nested Navigator to control whether sub-routes can paint outside the shell's bounds. The feedback suggests specifying the default value Clip.hardEdge directly in the NavigatorBuilder typedef to ensure that custom or mock implementations can statically resolve the default value correctly.
|
@googlebot I signed it! |
stuartmorgan-g
removed
the
triage-framework
This package uses the batched release system, so the changelog and version changes here will need to be updated to use the batch process described in the linked docs. As noted there, we recommend using the package tooling rather than making the change manually. |
|
Thanks! Moved the entry to I marked it If you would rather not take a breaking change for this, the alternative is to leave |
go_router opts into batch release in ci_config.yaml, so CHANGELOG.md and the pubspec version must not be edited directly. Revert both and describe the change in pending_changelogs instead.
bf1227a to
582fabb
Compare
|
Hi @m1roxx, thanks for working on this. Regarding you suggestion:
That would definitely be preferred! Bumping to a major version carries unnecessary friction and ecosystem churn. Thank you! |
|
Yup, absolutely. We also just published a major version so we would also not want to do so again so soon. |
Keep the NavigatorBuilder typedef and ShellRouteContext unchanged, and resolve the clip behavior in builder.dart from the shell route and the Navigator key instead, so the change stays additive. Downgrade the pending changelog to a minor version.
|
Done — reworked to keep this non-breaking. One correction to what I wrote above: the "would not extend to custom |
|
@Piinks @elliette friendly ping — I reworked this on Sep 4 to address the review: |
Piinks
left a comment
Piinks
left a comment
There was a problem hiding this comment.
Could you also add Clip clipBehavior = Clip.hardEdge to ShellRouteData.$route and StatefulShellBranchData.$branch in packages/go_router/lib/src/route_data.dart and forward it to ShellRoute / StatefulShellBranch?
|
@Piinks thanks for the review! Addressed everything in 919818f:
Ready for another look. |
- Share the clipBehavior docs through a doc template - Default _CustomNavigator.clipBehavior to Clip.hardEdge - Include clipBehavior in ShellRoute.debugFillProperties - Forward clipBehavior from ShellRouteData.$route and StatefulShellBranchData.$branch - Shorten the pending changelog entry
…uter-shell-clip-behavior # Conflicts: # packages/go_router/lib/src/builder.dart
1dec4c1 to
0d00d94
Compare
Piinks
left a comment
Piinks
left a comment
There was a problem hiding this comment.
LGTM! Thank you for addressing all the review feedback and adding the $route / $branch forwarding and tests.
elliette
added
the
autosubmit
…er#193641) flutter/packages@0ba9a82...d5ec6db 2026-10-01 faheemabbas766@gmail.com [tool] Enforce README package table order (flutter/packages#12316) 2026-09-30 jessiewong401@gmail.com [various] Allow plugin example apps to build and test on JDK 25 (flutter/packages#13031) 2026-09-30 149176071+m1roxx@users.noreply.github.com [go_router] Expose Navigator clipBehavior on ShellRoute and StatefulShellBranch (flutter/packages#12646) 2026-09-30 stuartmorgan@google.com [google_maps_flutter] Convert unit tests to Kotlin (flutter/packages#13072) 2026-09-30 36861262+QuncCccccc@users.noreply.github.com [material_ui] Migrate M3 ListTile template to use new gen_defaults (flutter/packages#13056) 2026-09-30 15619084+vashworth@users.noreply.github.com Allow tests to use macOS 15.7 or macOS 26.6 (flutter/packages#13007) 2026-09-30 43054281+camsim99@users.noreply.github.com [camera_android_camerax] Fix exposure offset setting error thrown when canceled by a new request (flutter/packages#12582) 2026-09-30 engine-flutter-autoroll@skia.org Roll Flutter from 55b8f88 to d649d2b (27 revisions) (flutter/packages#13080) 2026-09-30 36861262+QuncCccccc@users.noreply.github.com [material_ui] Migrate M3 InputDecorator template to use new gen_defaults (flutter/packages#13024) 2026-09-30 tarrinneal@gmail.com [pigeon] Fix JNI/FFI typed data memory lifetime bugs and update docs (flutter/packages#13061) 2026-09-30 43054281+camsim99@users.noreply.github.com [camera_android_camerax] Correct `pre-push` skill version validation logic (flutter/packages#12371) If this roll has caused a breakage, revert this CL and set the roller to dry run mode using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
4 participants
The
NavigatorthatShellRouteand eachStatefulShellBranchbuild always clipped its contents, with no way to opt out. A sub-route could therefore not paint outside the bounds the shell lays out for it — a box shadow or an overflowing menu gets cut off (see the samples in the issue).This adds a
clipBehaviorparameter toShellRouteandStatefulShellBranchthat is forwarded to theNavigatoreach of them builds, as suggested by @chunhtai in flutter/flutter#131836 (comment).It defaults to
Clip.hardEdge, which isNavigator's own default, so existing behavior is unchanged. As noted in the issue discussion, settingClip.nonealso lets route transition animations paint outside the shell's bounds; that trade-off is documented on the newclipBehaviordoc comment so callers can make an informed choice.StatefulShellBranchcarries the parameter rather thanStatefulShellRoutebecause each branch builds its ownNavigator, which is howobserversandrestorationScopeIdare already configured.Fixes flutter/flutter#131836
Pre-Review Checklist
[shared_preferences]///).Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2