-
Notifications
You must be signed in to change notification settings - Fork 6k
Call focus on input after detecting a tap #37863
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -288,24 +288,24 @@ class TextField extends RoleManager { | |||||||||||||||
| _initializeForBlink(); | ||||||||||||||||
| return; | ||||||||||||||||
| } | ||||||||||||||||
| num? lastTouchStartOffsetX; | ||||||||||||||||
| num? lastTouchStartOffsetY; | ||||||||||||||||
| num? lastPointerDownOffsetX; | ||||||||||||||||
| num? lastPointerDownOffsetY; | ||||||||||||||||
|
|
||||||||||||||||
| editableElement.addEventListener('touchstart', | ||||||||||||||||
| editableElement.addEventListener('pointerdown', | ||||||||||||||||
| allowInterop((DomEvent event) { | ||||||||||||||||
| final DomTouchEvent touchEvent = event as DomTouchEvent; | ||||||||||||||||
| lastTouchStartOffsetX = touchEvent.changedTouches!.last.clientX; | ||||||||||||||||
| lastTouchStartOffsetY = touchEvent.changedTouches!.last.clientY; | ||||||||||||||||
| final DomPointerEvent pointerEvent = event as DomPointerEvent; | ||||||||||||||||
| lastPointerDownOffsetX = pointerEvent.clientX; | ||||||||||||||||
| lastPointerDownOffsetY = pointerEvent.clientY; | ||||||||||||||||
| }), true); | ||||||||||||||||
|
|
||||||||||||||||
| editableElement.addEventListener( | ||||||||||||||||
| 'touchend', allowInterop((DomEvent event) { | ||||||||||||||||
| final DomTouchEvent touchEvent = event as DomTouchEvent; | ||||||||||||||||
| 'pointerup', allowInterop((DomEvent event) { | ||||||||||||||||
| final DomPointerEvent pointerEvent = event as DomPointerEvent; | ||||||||||||||||
|
|
||||||||||||||||
| if (lastTouchStartOffsetX != null) { | ||||||||||||||||
| assert(lastTouchStartOffsetY != null); | ||||||||||||||||
| final num offsetX = touchEvent.changedTouches!.last.clientX; | ||||||||||||||||
| final num offsetY = touchEvent.changedTouches!.last.clientY; | ||||||||||||||||
| if (lastPointerDownOffsetX != null) { | ||||||||||||||||
| assert(lastPointerDownOffsetY != null); | ||||||||||||||||
| final num offsetX = pointerEvent.clientX - lastPointerDownOffsetX!; | ||||||||||||||||
| final num offsetY = pointerEvent.clientY - lastPointerDownOffsetY!; | ||||||||||||||||
|
|
||||||||||||||||
| // This should match the similar constant defined in: | ||||||||||||||||
| // | ||||||||||||||||
|
|
@@ -318,13 +318,25 @@ class TextField extends RoleManager { | |||||||||||||||
| // Recognize it as a tap that requires a keyboard. | ||||||||||||||||
| EnginePlatformDispatcher.instance.invokeOnSemanticsAction( | ||||||||||||||||
| semanticsObject.id, ui.SemanticsAction.tap, null); | ||||||||||||||||
|
|
||||||||||||||||
| // We need to call focus for the following scenario: | ||||||||||||||||
| // 1. The virtial keyboard in iOS gets dismissed by the 'Done' button | ||||||||||||||||
| // located at the top right of the keyboard. | ||||||||||||||||
| // 2. The user tries to focus on the input field again, either by | ||||||||||||||||
| // VoiceOver or manually, but the keyboard does not show up. | ||||||||||||||||
| // In this scenario, the Flutter framework does not send a semantic update, | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. new line to make it more readable:
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done! |
||||||||||||||||
| // so we need to call focus after detecting a tap to make sure that the | ||||||||||||||||
| // the virtual keyboard will show. | ||||||||||||||||
|
||||||||||||||||
| // so we need to call focus after detecting a tap to make sure that the | |
| // the virtual keyboard will show. | |
| // so we need to call focus after detecting a tap to make sure that the | |
| // virtual keyboard will show. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch!
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -63,6 +63,29 @@ void testMain() { | |
| semantics().semanticsEnabled = false; | ||
| }); | ||
|
|
||
| test('tap detection works', () async { | ||
| debugBrowserEngineOverride = BrowserEngine.webkit; | ||
| debugOperatingSystemOverride = OperatingSystem.iOs; | ||
|
Comment on lines
+67
to
+68
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These need to be set back to
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done!
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: resetting using addTearDown would ensure we reset them even when the test fails. This way, this global change won't carry over to other tests. |
||
|
|
||
| final SemanticsActionLogger logger = SemanticsActionLogger(); | ||
| semantics() | ||
| ..debugOverrideTimestampFunction(() => _testTime) | ||
| ..semanticsEnabled = true; | ||
|
|
||
| createTextFieldSemantics(value: 'hello'); | ||
|
|
||
| final DomElement textField = appHostNode | ||
| .querySelector('input[data-semantics-role="text-field"]')!; | ||
|
|
||
| textField.dispatchEvent(createDomPointerEvent('pointerdown')); | ||
| textField.dispatchEvent(createDomPointerEvent('pointerup')); | ||
|
||
|
|
||
| expect(await logger.idLog.first, 0); | ||
| expect(await logger.actionLog.first, ui.SemanticsAction.tap); | ||
|
|
||
| semantics().semanticsEnabled = false; | ||
| }); | ||
|
|
||
| // TODO(yjbanov): this test will need to be adjusted for Safari when we add | ||
| // Safari testing. | ||
| test('sends a tap action when browser requests focus', () async { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's rename these to
deltaXanddeltaYto avoid confusion with offsets.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!