Add arguments to SemanticsActions; implement extend selection for a11y #13490
Conversation
|
Consider announcing the breaking changes on https://groups.google.com/forum/#!forum/flutter-dev |
Yes, that's the plan (see PR description) :) |
There was a problem hiding this comment.
Can this check be moved in front of the call to getOffsetAfter?
There was a problem hiding this comment.
Fixed this to check if extentOffset is null and now it cannot be moved anymore :)
There was a problem hiding this comment.
Does this mean there's a test waiting to be written? :)
There was a problem hiding this comment.
Can this check be moved up too?
There was a problem hiding this comment.
nit: I'd avoid words "when", "whenever" and "on" when describing a sequence of events. Prefer more precise "before", "after", "immediately after", etc.
There was a problem hiding this comment.
The names of these fields would read better with "has" or "with" as a prefix.
There was a problem hiding this comment.
I'm wondering if making this method generic (i.e. T args) would buy us anything?
There was a problem hiding this comment.
Discussed offline: Doesn't buy us anything.
There was a problem hiding this comment.
What happened to showOnScreen?
There was a problem hiding this comment.
I brought showOnScreen back. Initially, I though I would omit it since it is more of an internal action. However, somebody who wants to go fully custom might want to override the action (just like we do on Android).
There was a problem hiding this comment.
tiny nit: Swapping the two conditionals at this level would make the diffs simpler to read...I can't tell if it would change the algorithm or not though.
There was a problem hiding this comment.
I suspect it would be more performant to extract this closure into a private method
|
Addressed all comments, please take another look. |
goderbauer
force-pushed
the
selection
branch
from
c1fe76c to
db9e2d1
Compare
|
if LGT Travis |
goderbauer
force-pushed
the
selection
branch
from
db9e2d1 to
f51c8a2
Compare
|
Breaking change announcement is at https://groups.google.com/forum/#!topic/flutter-dev/eXx13sdDiP8. |
) Reverts the revert in #4448 with fixes to pass on the bot. This change will require framework changes in flutter/flutter#13490.
goderbauer
force-pushed
the
selection
branch
from
f51c8a2 to
988f199
Compare
goderbauer
force-pushed
the
selection
branch
from
988f199 to
efdfc15
Compare
goderbauer
added
the
a: accessibility
flutter#13490) **This PR contains a breaking API change:** `SemanticsConfiguration.addAction()` has been removed and replaces by action-specific setters (`onTap`, `onLongPress`, etc.) to take care of the fact that some action handlers (those, who take arguments) have different signatures.
5 participants
This requires engine change flutter/engine#4444.
This PR contains a breaking API change:
SemanticsConfiguration.addAction()has been removed and replaces by action-specific setters (onTap,onLongPress, etc.) to take care of the fact that some action handlers (those, who take arguments) have different signatures.I will announce the change on our mailing list well before submitting this change.
Fixes #13391.