[Re-Land] Implement focus traversal for desktop platforms. #31614
Conversation
This reverts commit 590cc27 (flutter#31461) to re-land the focus rewrite.
HansMuller
left a comment
HansMuller
left a comment
There was a problem hiding this comment.
LGTM so long as route.navigator is guaranteed to be non-null for some reason or we use route?.navigator
| animations.add(widget.route.secondaryAnimation); | ||
| _listenable = Listenable.merge(animations); | ||
| if (widget.route.isCurrent) { | ||
| widget.route.navigator.focusScopeNode.setFirstFocus(focusScopeNode); |
There was a problem hiding this comment.
According to https://docs.flutter.io/flutter/widgets/Route/navigator.html, a route's navigator can be null (here and below).
There was a problem hiding this comment.
It can only be null if it hasn't been pushed yet, or if it has been disposed.
In this case isCurrent checks that the navigator isn't null, and in the case of didPush below, by definition it has been pushed, so it's probably OK as-is. I'm happy to still add the ?'s if you think it would be better to do it defensively against a future change, but they're not needed as-is.
In #31614, I added an unfocus() to FocusNodes to allow giving up of focus, but it only worked on the primary focus. This changes that so that it will unfocus the entire chain, not just the primary focus. Now, if you call unfocus() on a FocusNode or FocusScopeNode, and their hasFocus returns true, then after calling unfocus(), it will return false. Before this change, it would only do that if hasPrimaryFocus was also true. This also fixes a bug in the way setFirstFocus was implemented, making it conform more to the behavior of the previous implementation. It has simplified logic in reparent, and in when it requests focus for scope nodes that have had setFirstFocus called on them.
3 participants
Description
This re-lands the Focus changes in #30040. Correctness changes in routes.dart, and removes the automatic requesting of focus on reparent when there is no current focus, which caused undesirable selections.
Related Issues
Addresses #11344, #1608, #13264, and #1678
Fixes #30084
Fixes #26704