Add OrderedFocusTraversalPolicy and FocusTraversalGroup to allow focus traversal to follow a predefined order.#49235
Merged
gspencergoog merged 24 commits intoflutter:masterfrom Feb 11, 2020
Conversation
fluttergithubbot
added
f: material design
gspencergoog
force-pushed
the
ordinal_focus
branch
from
0cfc232 to
6a501fd
Compare
domesticmouse
approved these changes
Jan 21, 2020
Contributor
|
PTAL @redbrogdon |
gspencergoog
changed the title
Member
|
nit: the analyzer is mad about some white space. |
gspencergoog
force-pushed
the
ordinal_focus
branch
from
6a501fd to
376deac
Compare
Contributor
Author
|
That analyzer is so picky, picky, picky! :-) (removed whitespace). |
gspencergoog
force-pushed
the
ordinal_focus
branch
from
efa4f69 to
3558ea6
Compare
goderbauer
reviewed
Jan 22, 2020
goderbauer
reviewed
Jan 22, 2020
Member
|
From looking at the code it wasn't immediately clear to me if you could mix and match different policies. Let's say I am using a (third-party) widget that is internally managing FocusOrder via the lexical order and I want to use that widget in my app where I generally manage focus order with numerical order. Can I do that? How? Might be worthwhile to have a test. Same for different non-OrderedFocusTraversalPolicy in different subtrees. |
gspencergoog
force-pushed
the
ordinal_focus
branch
4 times, most recently
from
088c1ca to
096fb6f
Compare
gspencergoog
changed the title
Member
|
The analyzer is complaining about a dependency loop. |
gspencergoog
force-pushed
the
ordinal_focus
branch
from
096fb6f to
04035f3
Compare
gspencergoog
changed the title
gspencergoog
force-pushed
the
ordinal_focus
branch
4 times, most recently
from
e172b4b to
077bc81
Compare
Contributor
Author
|
OK, @goderbauer, I think this is ready for another look. |
gspencergoog
force-pushed
the
ordinal_focus
branch
from
0416d99 to
922447a
Compare
Member
|
I am still a little concerned about the performance implications of this. Ignoring that, this LGTM. |
Member
|
Looks like Cirrus is unhappy now, though. |
gspencergoog
added a commit
to gspencergoog/flutter
that referenced
this pull request
Feb 12, 2020
…l… (flutter#49235)" This reverts commit 8ef5e2f because it breaks some semantics tests.
1 task
gspencergoog
added a commit
that referenced
this pull request
Feb 13, 2020
This re-lands #49235 with the addition of includeSemantics flag on the Focus widget so that the FocusTraversalGroup can create a Focus widget without affecting the semantics tree. The FocusTraversalGroup uses the Focus widget to create a grouping of descendants for traversal, but doesn't actually participate in focus (canRequestFocus is always false), so we don't want it to add a Semantics widget in that case, since that can cause semantics changes. The canRequestFocus attribute can also be used when a widget is disabled, so we do sometimes want to include Semantics even if that is false, but not in the case where it is always false, as for FocusTraversalGroup. - Added a test to make sure that FocusTraversalGroup doesn't add any semantics information.
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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
6 participants
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 change adds a way to provide explicit focus order for a part of the widget tree.
It adds
FocusTraversalPolicyGroup, which in many ways is similar toDefaultFocusTraversal, except that it groups a widget subtree together so that those nodes are traversed as a group.DefaultFocusTraversaldoesn't work as one would expect: If there is more than one DefaultFocusTraversal inside of a focus scope, the policy can change depending on which node was asked to move "next", which can cause unexpected behavior. The new grouping mechanism doesn't have that problem. I deprecateDefaultFocusTraversalin this PR.It adds
OrderedFocusTraversalPolicy, which is a policy that can be supplied toFocusTraversalPolicyGroupto set the policy for a sub-tree. It looks forFocusTraversalOrderinherited widgets, which use aFocusOrderto do the sorting.FocusOrderhas two subclasses:NumericalFocusOrder(which sorts based on a double), andLexicalFocusOrder, which sorts based on aString.As part of doing this, I refactored the way
FocusTraversalPolicyis implemented so that it has more default implementation methods, and exposes a new protected member:sortDescendants, which makes it easier for developers to make their own policy subclasses: they only need to implementsortDescendantsto get a new ordering behavior, but can also still override any of the default implementation behaviors if they need different behavior.I was able to do this without breaking the API (AFAICT).
Related Issues
Tests
*Orderclasses.Breaking Change