Added proper focus handling when pushing and popping routes

[gspencergoog]

Description

The proposed change will change focus handling when pushing and popping routes so that the FocusScopeNode for the route receives focus when pushed, and that the FocusScopeNode in the navigator receives focus when the route is popped.

This means that the last setFirstFocus call on the scope is used to determine which control actually receives focus. When the focus scope receives focus, it traverses its children, trying to find a non-scope node that is the "first focus" of itself or a child node.

This is a breaking change, because the focus behavior has changed. If you push a route after this change, and had a 'first focus' set on a widget via FocusScopeNode.setFirstFocus, it won't currently receive focus immediately, but after this change it will. Similarly, if you pop a route after this change, the focus will go back to where it was before the route was pushed, which is correct, but different from what happens now.

Related Issues

Addresses #13264

Tests

• Added tests for route pushing/popping.
• Adjusted other tests to account for new behavior.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I signed the CLA.
• I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
• I updated/added relevant documentation (doc comments with ///).
• All existing and new tests are passing.
• The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
• I am willing to follow-up on review comments in a timely manner.
• I checked all the boxes!

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

• Yes, this is a breaking change. (Breaking change announcement)

[codecov]

Codecov Report

Merging #40166 into master will decrease coverage by 1.95%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #40166      +/-   ##
==========================================
- Coverage   60.07%   58.11%   -1.96%     
==========================================
  Files         193      192       -1     
  Lines       18751    18443     -308     
==========================================
- Hits        11265    10719     -546     
- Misses       7486     7724     +238

Flag	Coverage Δ
#flutter_tool	58.11% <0%> (-1.96%)	⬇️

Impacted Files	Coverage Δ
packages/flutter_tools/lib/src/devfs.dart	10.83% <0%> (-57.98%)	⬇️
...ckages/flutter_tools/lib/src/ios/code_signing.dart	61.76% <0%> (-36.79%)	⬇️
...flutter_tools/lib/src/android/android_builder.dart	12.5% <0%> (-30.36%)	⬇️
...ackages/flutter_tools/lib/src/commands/attach.dart	46.8% <0%> (-26.88%)	⬇️
packages/flutter_tools/lib/src/linux/makefile.dart	80% <0%> (-20%)	⬇️
...s/flutter_tools/lib/src/windows/visual_studio.dart	82.6% <0%> (-14.46%)	⬇️
packages/flutter_tools/lib/src/base/build.dart	61.34% <0%> (-13.66%)	⬇️
...utter_tools/lib/src/reporting/crash_reporting.dart	80.95% <0%> (-13.5%)	⬇️
packages/flutter_tools/lib/src/android/gradle.dart	62.39% <0%> (-13.42%)	⬇️
packages/flutter_tools/lib/src/base/terminal.dart	69.89% <0%> (-11.59%)	⬇️
... and 128 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 406b449...8f8368f. Read the comment docs.

[HansMuller]

I can't say that I understand all of the implications of this (sensible) change. LGTM though.

[ThinkDigitalSoftware]

@gspencergoog This causes an issue when the route that's pushed is not a modal route, like a dropdown button or a popupmenu. It goes against the standard android behavior.

[gspencergoog]

@ThinkDigitalSoftware Can you elaborate? This change should only change focus when pushing a route if setFirstFocus has been called on the FocusScope (or autofocus has been set on a Focus widget). It doesn't focus the focus nodes directly, only their scopes. It's necessary to be able to do this in order for keyboard focus traversal to have a starting scope for focus.

[ThinkDigitalSoftware]

Well in my case, I haven't set any of those properties. I have a barebones example where when you tap a textfield and then a Dropdown, once the Dropdown is dismissed, the textfield is activated again and the keyboard always comes up, whether it was up previously or not. It makes sense for modal routes, because you're returning to where you are. But if you're filling out a form, this causes the screen to scroll back to the textfield and activate it, regardless of how far it is from the Dropdown /popup

[ThinkDigitalSoftware]

This pr was meant to address some of the issues, but not all. Also, I'm not sure if your PR caused all of the issues I'm having. I probably shouldn't insinuate that, but someone linked here and mentioned that this is when the behavior changed. #41574

[ThinkDigitalSoftware]

[gspencergoog]

@ThinkDigitalSoftware, I see the issue. I've also seen something else that is related just now, so I'm filing a bug and will look at it tomorrow: #41881