Expose the currently available semantic scroll actions #11286
Conversation
There was a problem hiding this comment.
Since this makes the earlier work pointless, maybe put all the earlier work into the else branch of this if block?
There was a problem hiding this comment.
It does not make the previous work pointless. We should only expose an action if there is a handler that can handle it (previous work) and if it is valid (the work in the if).
If there - for whatever reason - is no handler, we will blow up when executing the gesture....
|
This is great. Test? |
|
|
||
| // If a set of validActions has been provided only expose those. | ||
| if (validActions != null) | ||
| actions = actions.where((SemanticsAction action) => validActions.contains(action)).toList(); |
There was a problem hiding this comment.
this is O(N^2). Since validActions is only used for this, maybe it should be a Set instead of a List?
Or maybe it should just be a pair of booleans ("can scroll backwards" and "can scroll forwards")? Then you could just check the booleans when adding the actions?
There was a problem hiding this comment.
I wasn't too worried about that as validActions is guaranteed to be very small. Having said that, I refactored it to a Set, which gives us O(1) for contains. Happy to optimize further if this turns out to be an actual performance bottleneck.
|
|
goderbauer
added
the
a: accessibility
3 participants
Fixes #1675.