Skip to content

Remove the FocusScopeNode in the navigator - #109702

Merged
auto-submit[bot] merged 4 commits into
flutter:masterfrom
gspencergoog:remove_navigator_scope
Aug 22, 2022
Merged

auto-submit[bot] merged 4 commits into
flutter:masterfrom
gspencergoog:remove_navigator_scope

Conversation

@gspencergoog

@gspencergoog gspencergoog commented Aug 17, 2022 •

Copy link
Copy Markdown
Contributor

Description

There have been a number of issues over the years having to do with the FocusScopeNode that the Navigator introduces.
This PR attempts to remove that node and replace it with a focus node, and instead add one around the topmost Navigator in WidgetsApp.

Related Issues

Tests

  • Modified some existing tests to match the new reality.

@flutter-dashboard flutter-dashboard Bot added f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels. labels Aug 17, 2022
@gspencergoog
gspencergoog force-pushed the remove_navigator_scope branch 7 times, most recently from 113bd70 to 0f645e9 Compare August 17, 2022 17:12
@gspencergoog
gspencergoog force-pushed the remove_navigator_scope branch 2 times, most recently from 91cbd4a to c7042a9 Compare August 17, 2022 19:06

@gspencergoog gspencergoog Aug 17, 2022 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

GitHub did an amazingly poor job of diffing this. It's basically removing an empty node at the end, and an empty layer in the tree.

Screen Shot 2022-08-17 at 3 56 17 PM

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I recommend reviewing this with githubs "Hide whitespace" option. Makes it a lot better!

@gspencergoog
gspencergoog marked this pull request as ready for review August 17, 2022 22:58
@flutter-dashboard flutter-dashboard Bot added the p: material_ui material_ui package in flutter/packages label Aug 17, 2022
@gspencergoog

Copy link
Copy Markdown
Contributor Author

The huge downside (upside?) of this PR is that it will invalidate many, many, golden tests that use the Semantics Debugger.

@goderbauer goderbauer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/cc @chunhtai for Navigator

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wondering if this could now have any unintended side effects in the nested navigator case where the enclosingScope could be a distant ancestor of the Navigator?

Just requesting focus on the navigator.focusNode is not enough here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No, because if a nested navigator pops a route and comes back, then requesting focus on the enclosing scope will refocus the previously focused item in the scope.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I recommend reviewing this with githubs "Hide whitespace" option. Makes it a lot better!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Line 239 above probably also needs updating?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@gspencergoog

Copy link
Copy Markdown
Contributor Author

From what I can tell, the only tests failing in the Google tests are golden tests which use the semantics debugger. So either traversal isn't tested well enough (and it probably isn't), or this change doesn't break things.

@goderbauer

Copy link
Copy Markdown
Member

Let's go with this approach then and make a note about refactoring FocusScope at some point. We discussed adding a flag to FocusTraversalGroup to keep focus inside that group and introducing a new FocusHistory widget that manage, well, focus history. Both would replace FocusScope, which we could then deprecate.

@goderbauer goderbauer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM after that one remaining doc comment is addressed.

@gspencergoog
gspencergoog force-pushed the remove_navigator_scope branch from c7042a9 to b40f2d9 Compare August 20, 2022 00:38
@gspencergoog

Copy link
Copy Markdown
Contributor Author

Here's the issue I filed to address the refactor. #109896

@gspencergoog gspencergoog added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 22, 2022
@auto-submit
auto-submit Bot merged commit 77b41ba into flutter:master Aug 22, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 23, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 23, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 23, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 23, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 24, 2022
tarrinneal pushed a commit to flutter/packages that referenced this pull request Aug 24, 2022
* e997ab8 Roll Flutter Engine from 00bb6056cb80 to e70ba6a3a32d (1 revision) (flutter/flutter#110003)

* 6540ae0 Add clipBehavior to Card+InkWell example (flutter/flutter#109872)

* 3dcc6ed Roll Plugins from cd586d7 to 89fe2a2 (4 revisions) (flutter/flutter#110008)

* c43e44b Roll Flutter Engine from e70ba6a3a32d to 0d182ddb0d1d (1 revision) (flutter/flutter#110009)

* 6a1fc9e InputDecorator iconColor/prefixIconColor/suffixIconColor (flutter/flutter#109988)

* d4eaf01 Roll Flutter Engine from 0d182ddb0d1d to 60707621bf9b (1 revision) (flutter/flutter#110011)

* 77b41ba Remove the FocusScopeNode in the navigator (flutter/flutter#109702)

* 4b361cc Roll Flutter Engine from 60707621bf9b to ca7acb34c5bc (2 revisions) (flutter/flutter#110015)

* a8d6649 Roll Flutter Engine from ca7acb34c5bc to 38d2213d99ac (1 revision) (flutter/flutter#110026)

* 717d92e Roll Flutter Engine from 38d2213d99ac to 9513c34aabcd (2 revisions) (flutter/flutter#110033)

* 7e12b37 Deprecate 2018 text theme parameters (flutter/flutter#109817)

* ff803fd 0fd04455f deprecate pushPhysicalLayer (flutter-team-archive/engine#35566) (flutter/flutter#110036)

* 484a884 Bump github/codeql-action from 2.1.19 to 2.1.20 (flutter/flutter#110044)

* aa9b29b Roll Flutter Engine from 0fd04455fd77 to 7252c6406b2c (3 revisions) (flutter/flutter#110042)

* d79a3d7 Remove most benchmarks of SkSL warmup (flutter/flutter#110040)

* 7e33cd5 Fixed leading button size on app bar (flutter/flutter#110043)

* 129fa76 Moving ios benchmark tests to prod. (flutter/flutter#110049)

* 111a7d0 [platform_view]retry launch if it fails to launch for xcuitest (flutter/flutter#110030)

* 2d8067e Roll Flutter Engine from 7252c6406b2c to 258c580a83c6 (3 revisions) (flutter/flutter#110051)

* 328545e Roll Flutter Engine from 258c580a83c6 to ca48808d06f3 (1 revision) (flutter/flutter#110054)

* 720010c Roll Flutter Engine from ca48808d06f3 to 8c29591b89a3 (10 revisions) (flutter/flutter#110078)

* abfba69 Roll Flutter Engine from 8c29591b89a3 to c9d0a012817d (1 revision) (flutter/flutter#110087)

* 6d65551 Roll Plugins from 89fe2a2 to 3a5e6f3 (4 revisions) (flutter/flutter#110092)

* 8027842 Copy artifacts to `applicationBinaryPath` when specified for build+test separation (flutter/flutter#109879)

* c865207 Document tristate value (flutter/flutter#110106)

* c8569b6 Revert "Fixed leading button size on app bar (#110043)" (flutter/flutter#110103)

* 9ef5017 Update open_jdk version `version:11` (flutter/flutter#110110)

* c0c3874 Revert "Reland: Set IconButton.visualDensity default to VisualDensity.standard (#109432)" (flutter/flutter#110119)

* 0f9206c Roll Flutter Engine from c9d0a012817d to 8cc02dc73e95 (11 revisions) (flutter/flutter#110121)

* 34b4066 Roll Flutter Engine from 8cc02dc73e95 to 9dee4b9e4b8f (1 revision) (flutter/flutter#110122)
@gspencergoog
gspencergoog deleted the remove_navigator_scope branch October 7, 2022 22:07
auto-submit Bot pushed a commit that referenced this pull request Dec 5, 2023
Part of #139243

Deprecated in #109702

The replacement for focusScopeNode is focusNode.enclosingScope
creatorpiyush pushed a commit to creatorpiyush/packages that referenced this pull request Jun 10, 2026
* e997ab8 Roll Flutter Engine from 00bb6056cb80 to e70ba6a3a32d (1 revision) (flutter/flutter#110003)

* 6540ae0 Add clipBehavior to Card+InkWell example (flutter/flutter#109872)

* 3dcc6ed Roll Plugins from cd586d7 to 89fe2a2 (4 revisions) (flutter/flutter#110008)

* c43e44b Roll Flutter Engine from e70ba6a3a32d to 0d182ddb0d1d (1 revision) (flutter/flutter#110009)

* 6a1fc9e InputDecorator iconColor/prefixIconColor/suffixIconColor (flutter/flutter#109988)

* d4eaf01 Roll Flutter Engine from 0d182ddb0d1d to 60707621bf9b (1 revision) (flutter/flutter#110011)

* 77b41ba Remove the FocusScopeNode in the navigator (flutter/flutter#109702)

* 4b361cc Roll Flutter Engine from 60707621bf9b to ca7acb34c5bc (2 revisions) (flutter/flutter#110015)

* a8d6649 Roll Flutter Engine from ca7acb34c5bc to 38d2213d99ac (1 revision) (flutter/flutter#110026)

* 717d92e Roll Flutter Engine from 38d2213d99ac to 9513c34aabcd (2 revisions) (flutter/flutter#110033)

* 7e12b37 Deprecate 2018 text theme parameters (flutter/flutter#109817)

* ff803fd 0fd04455f deprecate pushPhysicalLayer (flutter-team-archive/engine#35566) (flutter/flutter#110036)

* 484a884 Bump github/codeql-action from 2.1.19 to 2.1.20 (flutter/flutter#110044)

* aa9b29b Roll Flutter Engine from 0fd04455fd77 to 7252c6406b2c (3 revisions) (flutter/flutter#110042)

* d79a3d7 Remove most benchmarks of SkSL warmup (flutter/flutter#110040)

* 7e33cd5 Fixed leading button size on app bar (flutter/flutter#110043)

* 129fa76 Moving ios benchmark tests to prod. (flutter/flutter#110049)

* 111a7d0 [platform_view]retry launch if it fails to launch for xcuitest (flutter/flutter#110030)

* 2d8067e Roll Flutter Engine from 7252c6406b2c to 258c580a83c6 (3 revisions) (flutter/flutter#110051)

* 328545e Roll Flutter Engine from 258c580a83c6 to ca48808d06f3 (1 revision) (flutter/flutter#110054)

* 720010c Roll Flutter Engine from ca48808d06f3 to 8c29591b89a3 (10 revisions) (flutter/flutter#110078)

* abfba69 Roll Flutter Engine from 8c29591b89a3 to c9d0a012817d (1 revision) (flutter/flutter#110087)

* 6d65551 Roll Plugins from 89fe2a2 to 3a5e6f3 (4 revisions) (flutter/flutter#110092)

* 8027842 Copy artifacts to `applicationBinaryPath` when specified for build+test separation (flutter/flutter#109879)

* c865207 Document tristate value (flutter/flutter#110106)

* c8569b6 Revert "Fixed leading button size on app bar (#110043)" (flutter/flutter#110103)

* 9ef5017 Update open_jdk version `version:11` (flutter/flutter#110110)

* c0c3874 Revert "Reland: Set IconButton.visualDensity default to VisualDensity.standard (#109432)" (flutter/flutter#110119)

* 0f9206c Roll Flutter Engine from c9d0a012817d to 8cc02dc73e95 (11 revisions) (flutter/flutter#110121)

* 34b4066 Roll Flutter Engine from 8cc02dc73e95 to 9dee4b9e4b8f (1 revision) (flutter/flutter#110122)
bisor0627 pushed a commit to bisor0627/packages that referenced this pull request Jun 19, 2026
* e997ab8 Roll Flutter Engine from 00bb6056cb80 to e70ba6a3a32d (1 revision) (flutter/flutter#110003)

* 6540ae0 Add clipBehavior to Card+InkWell example (flutter/flutter#109872)

* 3dcc6ed Roll Plugins from cd586d7 to 89fe2a2 (4 revisions) (flutter/flutter#110008)

* c43e44b Roll Flutter Engine from e70ba6a3a32d to 0d182ddb0d1d (1 revision) (flutter/flutter#110009)

* 6a1fc9e InputDecorator iconColor/prefixIconColor/suffixIconColor (flutter/flutter#109988)

* d4eaf01 Roll Flutter Engine from 0d182ddb0d1d to 60707621bf9b (1 revision) (flutter/flutter#110011)

* 77b41ba Remove the FocusScopeNode in the navigator (flutter/flutter#109702)

* 4b361cc Roll Flutter Engine from 60707621bf9b to ca7acb34c5bc (2 revisions) (flutter/flutter#110015)

* a8d6649 Roll Flutter Engine from ca7acb34c5bc to 38d2213d99ac (1 revision) (flutter/flutter#110026)

* 717d92e Roll Flutter Engine from 38d2213d99ac to 9513c34aabcd (2 revisions) (flutter/flutter#110033)

* 7e12b37 Deprecate 2018 text theme parameters (flutter/flutter#109817)

* ff803fd 0fd04455f deprecate pushPhysicalLayer (flutter-team-archive/engine#35566) (flutter/flutter#110036)

* 484a884 Bump github/codeql-action from 2.1.19 to 2.1.20 (flutter/flutter#110044)

* aa9b29b Roll Flutter Engine from 0fd04455fd77 to 7252c6406b2c (3 revisions) (flutter/flutter#110042)

* d79a3d7 Remove most benchmarks of SkSL warmup (flutter/flutter#110040)

* 7e33cd5 Fixed leading button size on app bar (flutter/flutter#110043)

* 129fa76 Moving ios benchmark tests to prod. (flutter/flutter#110049)

* 111a7d0 [platform_view]retry launch if it fails to launch for xcuitest (flutter/flutter#110030)

* 2d8067e Roll Flutter Engine from 7252c6406b2c to 258c580a83c6 (3 revisions) (flutter/flutter#110051)

* 328545e Roll Flutter Engine from 258c580a83c6 to ca48808d06f3 (1 revision) (flutter/flutter#110054)

* 720010c Roll Flutter Engine from ca48808d06f3 to 8c29591b89a3 (10 revisions) (flutter/flutter#110078)

* abfba69 Roll Flutter Engine from 8c29591b89a3 to c9d0a012817d (1 revision) (flutter/flutter#110087)

* 6d65551 Roll Plugins from 89fe2a2 to 3a5e6f3 (4 revisions) (flutter/flutter#110092)

* 8027842 Copy artifacts to `applicationBinaryPath` when specified for build+test separation (flutter/flutter#109879)

* c865207 Document tristate value (flutter/flutter#110106)

* c8569b6 Revert "Fixed leading button size on app bar (#110043)" (flutter/flutter#110103)

* 9ef5017 Update open_jdk version `version:11` (flutter/flutter#110110)

* c0c3874 Revert "Reland: Set IconButton.visualDensity default to VisualDensity.standard (#109432)" (flutter/flutter#110119)

* 0f9206c Roll Flutter Engine from c9d0a012817d to 8cc02dc73e95 (11 revisions) (flutter/flutter#110121)

* 34b4066 Roll Flutter Engine from 8cc02dc73e95 to 9dee4b9e4b8f (1 revision) (flutter/flutter#110122)
pull Bot pushed a commit to Little-Star888/flutter that referenced this pull request Sep 17, 2026
In flutter#109702, Navigator's internal `FocusScopeNode` was removed and
hoisted into `WidgetsApp`, but only inside the `else if
(_usesNavigator)` branch. As a result, apps using `WidgetsApp.router`
(including `MaterialApp.router` and `CupertinoApp.router` with
`RouterConfig` or `RouterDelegate`) lacked the enclosing
`FocusScope(debugLabel: 'Navigator Scope', autofocus: true)`.

On Flutter Web with semantics enabled (`ensureSemantics()`), when
widgets such as `Autocomplete` show an `OverlayPortal`, the semantics
compiler reparents the route node under a newly synthesized container
node. Without an enclosing `FocusScope`, moving the route's semantics
node detaches it from its previous DOM parent, triggering a native
browser blur event on any focused `<input>`. The text field loses focus
immediately after typing a single character, causing the options overlay
to dismiss.

This change wraps `routing` in `FocusScope` whenever `routing != null`
in `WidgetsApp`, ensuring consistent focus scope hierarchy across
`_usesNavigator`, `_usesRouterWithConfig`, and
`_usesRouterWithDelegates`.

Fixes flutter#187561

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [AI contribution guidelines] and understand my
responsibilities, or I am not using AI tools.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [x] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant in-code documentation (doc comments with
`///`).
- [x] If this PR introduces a new feature or capability, I created and
linked a website documentation issue or PR in [flutter/website] (or
verified none is needed).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[AI contribution guidelines]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[flutter/website]: https://github.com/flutter/website
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App f: routes Navigator, Router, and related APIs. framework flutter/packages/flutter repository. See also f: labels. p: material_ui material_ui package in flutter/packages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants