Adding semantics traversal order support #4540
Conversation
goderbauer
left a comment
There was a problem hiding this comment.
Do we have a plan for how this can be mirrored on iOS?
| result.setText(object.getValueLabelHint()); | ||
|
|
||
| // Accessibility Focus | ||
| result.setTraversalAfter(mOwner, object.previous); |
There was a problem hiding this comment.
Should this be under the heading "a11y focus"?
| for (SemanticsObject current : virtualIdOrder) { | ||
| if (current.previous != previousId) { | ||
| current.previous = previousId; | ||
| updated.add(current.id); |
There was a problem hiding this comment.
Do we have to add these to updated and send a TYPE_WINDOW_CONTENT_CHANGED even if only their ordering changed?
There was a problem hiding this comment.
Hmm. Maybe not. I'll see if I can skip that.
|
|
||
| // Now that the tree is pruned, sort all the entries and set their | ||
| // previous ids. | ||
| it = mObjects.entrySet().iterator(); |
There was a problem hiding this comment.
So, on any semantics update we have to re-sort the entire tree? I am a little concerned about performance...
There was a problem hiding this comment.
Yeah, me too. I think we need to have some benchmarks though to know really how bad this is.
| // Now that the tree is pruned, sort all the entries and set their | ||
| // previous ids. | ||
| it = mObjects.entrySet().iterator(); | ||
| ArrayList<SemanticsObject> virtualIdOrder = new ArrayList<SemanticsObject>(mObjects.size()); |
There was a problem hiding this comment.
Could this just be:
ArrayList<SemanticsObject> virtualIdOrder = new ArrayList<SemanticsObject>(mObjects.values());
.. and then remove the while loop below?
There was a problem hiding this comment.
Good suggestion. Fixed.
f981c42 to
6b334e6
Compare
| Collections.sort(childrenInTraversalOrder, new Comparator<SemanticsObject>() { | ||
| public int compare(SemanticsObject a, SemanticsObject b) { | ||
| final int top = Integer.compare(a.globalRect.top, b.globalRect.top); | ||
| // TODO(goderbauer): sort right-to-left in rtl environments. |
There was a problem hiding this comment.
Should we keep this TODO since the algorithm still has a LTR bias for the equal case?
There was a problem hiding this comment.
Don't we defer to tree order for the equal case?
There was a problem hiding this comment.
Yes, we will. I need to move this geometric comparison from here to the framework side for the geometric comparison to happen within the LTR/RTL context, as we discussed, and then if they are geometrically equal, or there is no Directionality in force, then we will defer to tree order.
dc5bfda to
bbec8cf
Compare
|
|
||
| result.setSelected(object.hasFlag(Flag.IS_SELECTED)); | ||
| result.setText(object.getValueLabelHint()); | ||
| result.setTraversalBefore(mOwner, object.nextNodeId); |
There was a problem hiding this comment.
Should we do this only if nextNodeId != -1 (or if that is guaranteed to be true, should there be an assert somewhere for that?)
There was a problem hiding this comment.
It so happens that View.NO_ID is what you put when there isn't a next node, and that is also -1. I added an inline conditional, however, to check for -1 and convert it to View.NO_ID, in case that value ever changes.
|
|
253841f to
fcc6dd7
Compare
|
re: iOS: no, I don't know what needs to happen there. I need to speak to someone who knows iOS development better than I do. |
|
Definitely feel free reach out to me and/or @chinmaygarde for the iOS side of this. Happy to provide feedback/review. |
fcc6dd7 to
f71ad18
Compare
|
I think that the iOS side needs something like this stackoverflow example. |
|
I'm going to commit this: the iOS side just will continue to sort by the default geometric method until we implement it there. |
This adds an API for defining the semantic node traversal order. It adds a sortOrder argument to the Semantics widget, which is a class that can define a list of sort keys to sort on. The keys are sorted globally so that an order that doesn't have to do with the current widget hierarchy may be defined. It also adds a shortcut sortKey argument to the Semantics widget that simply sets the sortOrder to just contain that key. The platform side (flutter/engine#4540) gets an additional member in the SemanticsData object that is an integer describing where in the overall order each semantics node belongs. There is an associated engine-side change that takes this integer and uses it to order widgets for the platform's accessibility services.
This adds an API for defining the semantic node traversal order. It adds a sortOrder argument to the Semantics widget, which is a class that can define a list of sort keys to sort on. The keys are sorted globally so that an order that doesn't have to do with the current widget hierarchy may be defined. It also adds a shortcut sortKey argument to the Semantics widget that simply sets the sortOrder to just contain that key. The platform side (flutter/engine#4540) gets an additional member in the SemanticsData object that is an integer describing where in the overall order each semantics node belongs. There is an associated engine-side change that takes this integer and uses it to order widgets for the platform's accessibility services.
This adds support for semantics traversal ordering.
It is a companion to flutter/flutter#14060, adding support for a sortIndex in the semantics data passed to the engine.
Addresses flutter/flutter#12187