Repository navigation
Put the caret where a tap landed when iOS reverts it after the touchend - #6002
ethan-james wants to merge 4 commits into
Conversation
Preview Deployment · Oct 28, 2026, 6:43 PM UTC · 3c08c7d |
trevinhofmann
left a comment
There was a problem hiding this comment.
LGTM as an extension of #5983, thank you!
|
|
||
| // place the caret before "the" | ||
| await tap(editable, { offset: 26, pointerType: 'touch' }) | ||
| await waitForCaretOffset(26) |
There was a problem hiding this comment.
Nit: Might be helpful to use something such as 'Hello beautiful people in '.length rather than magic numbers in the test, but I think this is fine.
There was a problem hiding this comment.
Makes sense! I wasn't sure how we felt about those string-based offset, but it should work fine.
|
Is this intended to stack on #5983? |
I think so. Last time I stacked something I had to undo it, so I haven't quite got my head around those. This one is finalized, though, so it makes sense. |
A tap a few characters from the caret gets no mouse events, so onMouseDown never places the caret, and after the touchend iOS moves the caret to the tapped word and then back to where it was (#4220). useRevertCorrection, mounted by useEditMode on the thought being edited, records the caret offset and the touch point at the touchend, takes a selectionchange back to that offset after a move away as the revert, and places the caret at getCaretOffset of the touch point, dispatching setCursor so the Redux offset follows. The watch ends on the next touchstart, mousedown, keydown or beforeinput. This replaces #5983's revert correction. The case it was built for, a revert before a touchend that iOS 27 withholds (#5981), is waiting on a Safari fix, so the watch no longer spans the touch: it needs no touchStore state, no listeners in initEvents and no TOUCH_SLOP check for drags. Co-Authored-By: Claude claude-opus-5-5 (medium) <noreply@anthropic.com>
e51362c to
8435ba0
Compare
raineorshine
left a comment
There was a problem hiding this comment.
Thank you, this one looks pretty manageable. useRevertCorrection is self contained, so at least the complexity is not spread over many files.
|
|
||
| // iOS is still handling the tap when it fires this selectionchange. | ||
| requestAnimationFrame(() => { | ||
| selection.set(editable, { offset: tapped }) |
There was a problem hiding this comment.
This deviates from the declarative design of useEditMode where we attempt to centralize selection.set and avoid competing selection logic. Many conditions feed into one write.
Is there something fundamentally different about useRevertCorrection that it can’t follow the existing pattern?
Would it be possible to integrate into useEditMode?
It may be cleaner the way you have it, but since it does seem to be a departure from our earlier architecture, along with useCaretRestore, so I want to make sure that we have a unified vision rather than patching on a new paradigm. Refactoring may be necessary if there is a better way to layer multiple effects and touch handlers.
There was a problem hiding this comment.
The detection can't fold into the declarative effect, because it reacts to iOS changing the selection rather than to app state, so a selectionchange listener has to live somewhere (as with useCaretRestore). The write could, in principle: dispatch setCursor with the tapped offset and let setSelectionToCursorOffset place the caret, as the retarget path in onTouchEnd does.
The blocker is that the selection effect deliberately doesn't re-run when only cursorOffset changes, so that it doesn't set the selection while typing. The retarget path gets a re-run because the cursor moves to a different thought. Here the thought is already the cursor, and the only existing way to force a re-run is editableNonce. That's global: every ContentEditable re-checks its HTML and every Note drops its draft. On the thought being edited, HTML in Redux that lags the DOM would also be written over the text. A dedicated trigger would avoid that, but it would be new state for a single caller.
So I'm looking at keeping the detection in the hook and routing the write through useEditMode's setCaretOffset, which onMouseDown already uses, so both tap paths share one placement.
Generated by Claude Code
There was a problem hiding this comment.
It seems like a new caretNonce dependency would be a pretty good solution for getting the declarative effect to run without messing up the editableNonce and risking losing typed changes. I think that this would be best handled in #4470, which I've de-prioritized but can re-prioritize now that the path forward seems clearer.
Anything decided here should be extended to cover useCaretRestore and, ideally, setCaretOffset as well. It would be great to keep state.cursorOffset in better sync by always calling setCursor rather than simply selection.set. However, there are some open questions about whether waiting for the Redux sync is fast enough for some of these caret adjustment operations to proceed without noticeable flicker. If I remember correctly, I was exploring replacing useEffect with useLayoutEffect, which is also a substantial change that needs a lot of testing.
For now, I think I'll drop the setCursor dispatch because it doesn't cover any new cases. If cursorOffset was up to date before the caret was reverted, then it will still be up to date after the correction. If it was stale due to lack of mouse events, then it can stay stale.
| if (tapped === null || tapped === before) return | ||
|
|
||
| // iOS is still handling the tap when it fires this selectionchange. | ||
| requestAnimationFrame(() => { |
There was a problem hiding this comment.
Why requestAnimationFrame? I’m not sure there is any guarantee that the browser selection lines up with the frame rate.
Is there something more salient than the next frame that you could wait for? Or is it a magic number that might be sensitive to cpu load?
The correction no longer waits a frame. iOS does not move the caret again after the revert, so setting it in the revert's selectionchange holds: the caret.ts test for #4220 passed 9 of 9 times on BrowserStack on iOS 17, 26 and 27 without the frame, and on an iPhone on iOS 27 all 14 corrected taps in a debug log stayed where they were put. Co-Authored-By: Claude claude-opus-5-5 (medium) <noreply@anthropic.com>
The revert correction no longer dispatches setCursor. Nothing writes the cursor offset in Redux during a tap that gets no mouse events, so a tap that iOS places itself leaves it where it was, and the correction now does the same: it only sets the selection. The hook no longer needs the thought's path. Co-Authored-By: Claude claude-opus-5-5 (medium) <noreply@anthropic.com>


Fixes #4220. Replaces #5655 and #5983.
A tap a few characters from the caret gets no mouse events, so
useEditMode'sonMouseDownnever places the caret. After thetouchend, iOS moves the caret to the tapped word and then, within about 70ms, back to where it was. On BrowserStack every such tap reverted on iOS 17, 26 and 27, with noSelectioncall from em.A new
useRevertCorrectionhook, mounted byuseEditModebesideuseCaretRestoreon the thought being edited, records the caret offset and the touch point at the tap'stouchend. Aselectionchangethat returns the caret to that offset after it moved away is taken as the revert, and the caret is placed atgetCaretOffsetof the touch point in the same handler. iOS does not move it again after the revert: without a frame's delay, all 14 corrected taps in a debug log from an iPhone on iOS 27 stayed where they were put. It only sets the selection. The cursor offset in Redux is left alone, as it is for a tap that iOS places itself, since nothing writes it during a tap without mouse events. The watch ends on the nexttouchstart,mousedown,keydownorbeforeinput. Themousedownmatters most, since it means the tap is oneonMouseDownplaces itself. Unlike #5655's fixed 100ms timer, the hook reacts to the revert itself, so it does not depend on how long iOS takes.The caret goes to em's own offset for the tap point, which can differ from the word edge iOS first chose (#5982).
Scope
This used to be based on #5983, which also corrected a revert that arrives before a
touchendthat iOS 27 withholds (#5981). #5981 is on hold until Safari fixes the withheld touch (#5660), so that case is out of scope. Without it, the watch no longer has to span the touch, which removes most of #5983:revertWatchstate intouchStore; the watch is a local variable in the hook's effectinitEvents; the hook's listeners exist only while the thought is being editedtouchmove/TOUCH_SLOPcheck: a drag or long press has already moved the caret by itstouchend, and nothing moves it backsetCaretOffsethoist,store.getState()orsetCursordispatch, anduseEditModegains one lineAdjacent behaviour
mousedownends the watch, soonMouseDownkeeps ownership.document.activeElement.touchend.keydownorbeforeinput, which ends the watch.Tests
caret.ts: "Tap moves the caret to a word a few characters away", Steps to Reproduce A from #4220, waiting with a newwaitForCaretOffsethelper that requires the offset on two samples 250ms apart. Without the hook mounted, it fails on BrowserStack on iOS 17 and 26 with "Expected the caret to come to rest at offset 23, but it was at 26".Not yet checked on a physical device.
🤖 Generated with Claude Code