Add longPress/Tap event to SemanticService #16945
Conversation
jonahwilliams
changed the title
|
tests? cc @goderbauer |
|
|
||
| /// An event which triggers long press semantic feedback. | ||
| /// | ||
| /// Only used on Android. |
There was a problem hiding this comment.
Maybe change to: "currently only honored on Android" and say a word about how android typically reacts to these events?
There was a problem hiding this comment.
Update doc on the semantics event themselves
|
|
||
| /// An event which triggers tap semantic feedback. | ||
| /// | ||
| /// Only used on Android. |
There was a problem hiding this comment.
same here and in the other file below.
There was a problem hiding this comment.
same as above
| /// * [wrapForTap] to trigger platform-specific feedback before executing a | ||
| /// [GestureTapCallback]. | ||
| static Future<Null> forTap(BuildContext context) async { | ||
| SemanticsService.tap(); |
There was a problem hiding this comment.
From the BuildContext you could figure out the renderObject and form there you could probably get to the ID of the semantics node that this tap event is for. If you include that ID in the SemanticsEvent, can we then use that ID on the engine side instead of the id of the currently focused node? The focus might have changed between triggering the tap and processing the tap event. This ID would be stable.
There was a problem hiding this comment.
This only works if the RenderObject is a semantics boundary, otherwise the actual RenderObject with a SemanticsNode may be an arbitrary child or parent of the object in question.
There was a problem hiding this comment.
Semantics information always flow up in the tree (with the exception of maybe traversal order) and they can't really "jump over" a semantics node. So, is it not always the first ancestor renderObject with a semantic node that you're looking for?
There was a problem hiding this comment.
I can try traversing the tree upwards in that case - will update with those results.
There was a problem hiding this comment.
I have a somewhat hacky solution - it requires accessing the private _semantics member of RenderObject though, I don't see a way around this.
|
Also +1 to tests :) |
|
Added some tests |
|
Updated RenderObject with a sendSematicsEvent API. This looks up the render object tree for the first non-null semantics node and sends the semantics event from this node. |
|
@goderbauer friendly ping! |
| /// | ||
| /// See [SemanticsNode.sendEvent] for a full description of the behavior. | ||
| void sendSemanticsEvent(SemanticsEvent semanticsEvent) { | ||
| if (_semantics != null) { |
There was a problem hiding this comment.
Should this return right away if semantics are disabled?
|
|
||
| /// An event which triggers long press semantic feedback. | ||
| /// | ||
| /// Currently only honored on Android. Triggers a long-press specific sound |
There was a problem hiding this comment.
nit: remove extra white-space after .?
|
|
||
| /// An event which triggers tap semantic feedback. | ||
| /// | ||
| /// Currently only honored on Android. Triggers a tap specific sound when |
| 'type': 'tap', | ||
| 'nodeId': object.debugSemantics.id, | ||
| 'data': <String, dynamic>{}, | ||
| }); |
There was a problem hiding this comment.
Bonus points for asserting that the node with id object.debugSemantics.id was actually annotated with the click action for the checkbox in the semantics tree.
Same might be useful for some of the other tests.
There was a problem hiding this comment.
Done for all tests except tooltip
Also reverts attempted fix: * fix names of demos in perf tests (flutter#17257) Also reverts: * Add longPress/Tap event to SemanticService (flutter#16945) This reverts commit 22c12f9. This reverts commit 40ddf01. This reverts commit 50bd39a.
4 participants
Engine implementation: flutter/engine#5081
Partial fix of #14187
Creates a new a11y event sent along with a haptic feedback request on Android when a longPress or inkWell tap event is sent. Also sends the semantic tap event on the toggleable update.