Repository navigation
Put the caret where a withheld tap landed when iOS 27 reverts it #5983
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 14 commits
4d499ac
f654c14
bcc800d
2b5e478
c85e1fe
2149e0a
5772bff
c154e10
db23870
3dfff66
323a589
5ef881b
35ed3aa
463095f
10a4e72
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| import { useEffect } from 'react' | ||
| import { isSafari, isTouch } from '../../browser' | ||
| import getCaretOffset from '../../device/getCaretOffset' | ||
| import * as selection from '../../device/selection' | ||
| import touchStore from '../../stores/touchStore' | ||
|
|
||
| /** | ||
| * On WebKit 27, iOS can move the caret for a tap whose touchend it withholds (#5660), then move it back to where it was | ||
| * before the touch (#5981). Puts the caret where the tap landed instead. The touch is recorded in touchStore's | ||
| * revertWatch by initEvents, which also ends the watch on any other input. | ||
| */ | ||
| const useRevertCorrection = ({ | ||
| editableRef, | ||
| enabled, | ||
| onCorrect, | ||
| }: { | ||
| editableRef: React.RefObject<HTMLElement | null> | ||
| /** Correct only while this editable is the one being edited, i.e. the cursor thought or its focused note. */ | ||
| enabled: boolean | ||
| /** Places the caret at the corrected offset. Defaults to setting the selection alone. Must be stable. */ | ||
| onCorrect?: (offset: number) => void | ||
| }) => { | ||
| useEffect(() => { | ||
| const editable = editableRef.current | ||
| if (!editable || !enabled || !isTouch || !isSafari()) return | ||
|
|
||
| /** Notes a move away from the offset before the touch, and corrects a return to it after one. */ | ||
| const onSelectionChange = () => { | ||
| // the selection belongs to another editable | ||
| if (document.activeElement !== editable) return | ||
| const watch = touchStore.getState().revertWatch | ||
| // no touch on this editable is being watched | ||
| if (watch?.editable !== editable) return | ||
| // a range is not a caret that iOS placed or reverted, e.g. a word selected by a double tap | ||
| if (!selection.isCollapsed()) return | ||
| const offset = selection.offsetFromNode(editable) | ||
| // the caret is not inside this editable | ||
| if (offset === null) return | ||
|
|
||
| if (!watch.moved && offset !== watch.before) touchStore.update({ revertWatch: { ...watch, moved: true } }) | ||
| // not a revert: the caret has not left its offset yet, or has moved somewhere other than back to it | ||
| if (!watch.moved || offset !== watch.before) return | ||
|
|
||
| touchStore.update({ revertWatch: null }) | ||
| const tapped = getCaretOffset(editable, watch).offset | ||
| // nowhere to put the caret, or the tap was on the caret, so there is nothing to undo | ||
| if (tapped === null || tapped === watch.before) return | ||
| // iOS is still handling the tap when it fires this selectionchange. | ||
| requestAnimationFrame(() => (onCorrect ? onCorrect(tapped) : selection.set(editable, { offset: tapped }))) | ||
| } | ||
|
|
||
| document.addEventListener('selectionchange', onSelectionChange) | ||
|
|
||
| return () => { | ||
| if (touchStore.getState().revertWatch?.editable === editable) touchStore.update({ revertWatch: null }) | ||
| document.removeEventListener('selectionchange', onSelectionChange) | ||
| } | ||
| }, [editableRef, enabled, onCorrect]) | ||
| } | ||
|
|
||
| export default useRevertCorrection | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -12,6 +12,49 @@ import tap from '../helpers/tap' | |
| import waitForEditable from '../helpers/waitForEditable' | ||
| import waitUntil from '../helpers/waitUntil' | ||
|
|
||
| /** | ||
| * Records the caret's focus offset on every selectionchange from the next touchstart on, in the page, so that a change | ||
| * between two WebDriver calls is not missed, and one still pending from before the touch is not counted. | ||
| */ | ||
| const recordCaretFromNextTouch = async () => { | ||
| await browser.execute(() => { | ||
| const w = window as unknown as { __caretOffsets: number[] } | ||
| w.__caretOffsets = [] | ||
| document.addEventListener( | ||
| 'touchstart', | ||
| () => | ||
| document.addEventListener('selectionchange', () => { | ||
| const offset = document.getSelection()?.focusOffset | ||
| if (offset != null) w.__caretOffsets.push(offset) | ||
| }), | ||
| { capture: true, once: true }, | ||
| ) | ||
| }) | ||
|
|
||
| return { | ||
| /** | ||
| * Waits until the caret has left the offset `before`, returned to it, and left it again, i.e. a tap that iOS | ||
| * reverted has been corrected. Also passes if `timeout` expires without the caret being reverted. | ||
| */ | ||
| waitToSettle: async ({ before, timeout }: { before: number; timeout: number }): Promise<void> => { | ||
| const start = Date.now() | ||
| /** True once the offsets show the caret leaving before, returning to it, and leaving it again. */ | ||
| const corrected = (offsets: number[]) => { | ||
| const left = offsets.findIndex(offset => offset !== before) | ||
| const returned = offsets.findIndex((offset, i) => i > left && offset === before) | ||
| return left !== -1 && returned !== -1 && offsets.slice(returned + 1).some(offset => offset !== before) | ||
| } | ||
| // Safari's driver does not wait for a promise returned from execute, so poll, with the deadline in the condition | ||
| await browser.waitUntil( | ||
| async () => | ||
| Date.now() - start >= timeout || | ||
| corrected(await browser.execute(() => (window as unknown as { __caretOffsets: number[] }).__caretOffsets)), | ||
| { timeout: timeout * 5, interval: 50 }, | ||
| ) | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| describe('Double tap', () => { | ||
| // https://github.com/cybersemics/em/issues/5660 | ||
| it('a tap after a double tap on the caret does not activate drag and drop', async () => { | ||
|
|
@@ -64,4 +107,31 @@ describe('Double tap', () => { | |
| expect(alertText).not.toBe('Drag and drop to move thought') | ||
| expect(await isKeyboardShown()).toBe(true) | ||
| }) | ||
|
|
||
| // https://github.com/cybersemics/em/issues/5981 | ||
| it('a tap within the thought being edited after a double tap is not undone', async () => { | ||
| await newThought('Existentialism eternally') | ||
| await hideKeyboardByTappingDone() | ||
| const editable = await waitForEditable('Existentialism eternally') | ||
| await tap(editable, { pointerType: 'touch' }) | ||
| await waitUntil(isKeyboardShown) | ||
|
|
||
| await tap(editable, { offset: 'Existentialism'.length, pointerType: 'touch' }) | ||
| await waitUntil( | ||
| async () => (await browser.execute(() => document.getSelection()?.focusOffset)) === 'Existentialism'.length, | ||
| ) | ||
| await tap(editable, { offset: 'Existentialism'.length, pointerType: 'touch', count: 2 }) | ||
| await waitUntil(async () => (await getSelection().toString()) === 'Existentialism') | ||
| await setSelection('Existentialism'.length, 'Existentialism'.length) | ||
|
|
||
| const caret = await recordCaretFromNextTouch() | ||
| // the middle of "eternally" | ||
| await tap(editable, { offset: 'Existentialism eter'.length, pointerType: 'touch' }) | ||
| // iOS can move the caret back within about 300ms of the tap, after which em puts it back again | ||
| await caret.waitToSettle({ before: 'Existentialism'.length, timeout: 1000 }) | ||
|
|
||
| const offset = await browser.execute(() => document.getSelection()?.focusOffset ?? null) | ||
| // anywhere in "eternally", which runs to the end of the text | ||
| expect(offset).toBeGreaterThanOrEqual('Existentialism '.length) | ||
|
Collaborator
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. Is there a reason to use
Collaborator
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. Copilot says:
I would support that argument mainly because our word snapping works a little differently from Safari's, and I foresee a future in which we fix that because it can be annoying to have the caret bouncing around. |
||
| }) | ||
| }) | ||
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.
Too many return statements in this function, but they all have a purpose. Would it be better to move them into helper functions?
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.
The return statements are reasonable IMO, but it might be helpful to include a comment for each explaining what that purpose is.