Fix ExcludeFocus so it won't refocus a sibling of the focused node.
#61756
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
fluttergithubbot
added
the
framework
gspencergoog
force-pushed
the
fix_exclude_focus
branch
2 times, most recently
from
88e40d3 to
4eb0037
Compare
gspencergoog
changed the title
ExcludeFocus so it won't refocus a sibling of the focused node.
gspencergoog
requested review from
goderbauer and
darrenaustin
and removed request for
goderbauer
darrenaustin
approved these changes
Jul 17, 2020
| @@ -864,19 +867,19 @@ class FocusNode with DiagnosticableTreeMixin, ChangeNotifier { | |||
| while (!scope.canRequestFocus) { | |||
| scope = scope.enclosingScope ?? _manager?.rootScope; | |||
| } | |||
| scope?._doRequestFocus(findFirstFocus: false); | |||
| scope._doRequestFocus(findFirstFocus: false); | |||
There was a problem hiding this comment.
Are these changes here related to the fix, or just cleaning up something? I assume scope can no longer be null when it might have before?
There was a problem hiding this comment.
Actually, I made those changes thinking that these couldn't ever be null, because the rootScope can't be null, but if the _manager is null, it still could be. I reverted these.
gspencergoog
force-pushed
the
fix_exclude_focus
branch
from
4eb0037 to
d632495
Compare
Pragya007
pushed a commit
to Pragya007/flutter
that referenced
this pull request
Aug 11, 2020
…flutter#61756) This changes FocusNode.descendantsAreFocusable so that it doesn't allow the enclosing scope to re-focus a node that is a descendant of the node with descendantsAreFocusable set to false. Because of the order in which the internal state for descendantsAreFocusable was being set, setting it to false was causing a sibling node to be focused when descendantsAreFocusable of the parent was set to false, even though it shouldn't have been focusable, because the enclosing scope would search for a candidate to be focused before the internal state was set to false. Instead of looping over the children and telling them all to unfocus (and select the previously focused node), this unfocuses the node that has descendantsAreFocusable set to false, with the disposition UnfocusDisposition.previouslyFocusedChild, so that its enclosing scope will look for a previously focused child that isn't part of the subtree being excluded. This affects how the ExcludeFocus widget behaves when turning on exclude.
mingwandroid
pushed a commit
to mingwandroid/flutter
that referenced
this pull request
Sep 6, 2020
…flutter#61756) This changes FocusNode.descendantsAreFocusable so that it doesn't allow the enclosing scope to re-focus a node that is a descendant of the node with descendantsAreFocusable set to false. Because of the order in which the internal state for descendantsAreFocusable was being set, setting it to false was causing a sibling node to be focused when descendantsAreFocusable of the parent was set to false, even though it shouldn't have been focusable, because the enclosing scope would search for a candidate to be focused before the internal state was set to false. Instead of looping over the children and telling them all to unfocus (and select the previously focused node), this unfocuses the node that has descendantsAreFocusable set to false, with the disposition UnfocusDisposition.previouslyFocusedChild, so that its enclosing scope will look for a previously focused child that isn't part of the subtree being excluded. This affects how the ExcludeFocus widget behaves when turning on exclude.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This changes
FocusNode.descendantsAreFocusableso that it doesn't allow the enclosing scope to re-focus a node that is a descendant of the node withdescendantsAreFocusableset to false.Because of the order in which the internal state for
descendantsAreFocusablewas being set, setting it to false was causing a sibling node to be focused whendescendantsAreFocusableof the parent was set to false, even though it shouldn't have been focusable, because the enclosing scope would search for a candidate to be focused before the internal state was set to false.Instead of looping over the children and telling them all to unfocus (and select the previously focused node), this unfocuses the node that has
descendantsAreFocusableset to false, with the dispositionUnfocusDisposition.previouslyFocusedChild, so that its enclosing scope will look for a previously focused child that isn't part of the subtree being excluded.This affects how the
ExcludeFocuswidget behaves when turning onexclude.Related Issues
Tests
Breaking Change