Skip to content

Keep the caret after text pasted into a note - #5992

Merged
raineorshine merged 10 commits into
mainfrom
copilot/fix-caret-placement-in-note
Oct 27, 2026
Merged

raineorshine merged 10 commits into
mainfrom
copilot/fix-caret-placement-in-note

Conversation

Copilot AI commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor

Pasting into the middle of a note on iOS added a line break and moved the caret to the end of the note. The issue also reports that Android drops the caret and keyboard; that did not reproduce in emulated Android Chrome, so this PR has only been checked against the iOS failure.

Cause: WebKit's paste inserts the text wrapped in a styled span plus a \n:

Sed <span style="caret-color: …">Ipsum</span>
tortor nunc

Note's onChange then stripped that markup. The saved value no longer matched the DOM, so react-contenteditable rewrote innerHTML, which keeps the line break and drops the caret position.

Changes

  • src/components/Note.tsx: onPaste now handles the paste itself.
    • It reads text/plain (or text/uri-list), calls preventDefault(), and inserts the text with document.execCommand('insertText', false, text). The caret lands after the pasted text and the DOM matches the saved value.
    • If the clipboard has no text, it falls back to the browser's paste and the justPasted strip in onChange, as before ([iOS PWA] Cannot import url #286).
    • Unlike thought paste (useOnPaste), a note does not run importData. A multi-line paste stays literal text in the note, as it did before.
  • src/e2e/iOS/helpers/tapEditMenuItem.ts (new): taps a named item (Copy, Paste, …) in the native iOS edit menu. This is the native-context logic that used to live in pasteFromEditMenu, which now calls it.
  • src/e2e/iOS/__tests__/caret.ts: new regression test. It copies "Ipsum" from the thought through the native menu, then pastes it between "Sed " and "tortor" in the note. It expects the note to read Sed Ipsumtortor nunc with the caret at offset 9.
    • It passed on a BrowserStack iPhone in the Capacitor app. In CI's mobile Safari it fails at the Copy step ("no native edit menu appeared") and never reaches the paste.
    • The copy step needs another way to raise the menu, or to fill the clipboard, before the iOS check can go green.
  • docs/cursor-and-caret.md: notes how a note handles paste.

Co-Authored-By: GitHub Copilot Coding Agent unknown (unknown) <198982749+Copilot@users.noreply.github.com>

Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com>
@github-actions

github-actions Bot commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor
Preview Deployment · Oct 27, 2026, 11:05 PM UTC · a360992

Preview deployment

Insert the clipboard's plain text with execCommand('insertText') instead of
letting the browser paste HTML, which WebKit wraps in a styled span and a
trailing newline. Stripping that in onChange left a value that no longer
matched the DOM, so the note re-rendered and the caret jumped to the end.

Co-Authored-By: GitHub Copilot Coding Agent unknown (unknown) <198982749+Copilot@users.noreply.github.com>

Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix caret placement issue when pasting in notes Keep the caret after text pasted into a note Oct 15, 2026
Copilot AI requested a review from BayuAri October 15, 2026 03:37
@BayuAri

BayuAri commented Oct 15, 2026

Copy link
Copy Markdown
Collaborator

@copilot
Please address the failing Check

Copilot AI and others added 2 commits October 15, 2026 04:30
…crub test

Under Appium, navigator.webdriver is false in mobile Safari, so the cursor
scroll after the paste is smooth and moves the note for ~350ms even with
transitions disabled. A tap aimed mid-scroll landed on the parent thought.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com>
showEditMenu tapped the selection at its viewport coordinates, which in
mobile Safari lands 60px above it, under the browser chrome, so no edit
menu appeared and the note-paste test failed at its Copy step. Add
getScreenOffsetY as tap does, and wait for the note to come to rest before
tapping it.

Co-Authored-By: GitHub Copilot Coding Agent claude-opus-5.5 (high) <198982749+Copilot@users.noreply.github.com>

Co-authored-by: BayuAri <8419585+BayuAri@users.noreply.github.com>

Copilot AI commented Oct 15, 2026

Copy link
Copy Markdown
Contributor Author

@copilot
Please address the failing Check

Fixed the failing BrowserStack (iOS) check in 9749cf2, and it's green on that commit along with Lint, Test, Puppeteer and BrowserStack Android. TDD was still running when I posted this.

Both tests passed in the iOS run.

@github-actions
github-actions Bot marked this pull request as ready for review October 15, 2026 05:11

@BayuAri BayuAri left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed fixed.
Text is pasted without adding extra line breaker and caret remain where it is left off accordingly (not moved to the end of the note or even disappear)

@BayuAri
BayuAri requested a review from trevinhofmann October 15, 2026 14:00

@trevinhofmann trevinhofmann left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

Comment thread src/e2e/iOS/__tests__/caret.ts Outdated
Comment on lines +217 to +218
// Disabling transitions does not stop the smooth scroll that brings the cursor into view.
await waitForElementAtRest(note)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this the browser's default behavior of scrolling the focused input to the center of the viewport? Can it be prevented, or does that have to wait for the various autoscroll bugs to be fixed?

It's not ideal to have to wait for a smooth scroll in a test that is about caret placement.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Please answer the question above in this thread. Explanation only: don't change any code.

Pointers:

  • The scroll comes from our own scrollCursorIntoView (src/device/scrollCursorIntoView.ts), which uses behavior: 'smooth' when the cursor is less than a viewport away. It is not the browser's native focus autoscroll, which preventAutoscroll already suppresses for notes and thoughts.
  • scrollCursorIntoView and scrollTo already scroll instantly under automation (navigator.webdriver ? 'instant' : behavior). Appium leaves navigator.webdriver false in mobile Safari, so that guard never fires on iOS.
  • disableTransitions only disables CSS transitions. An explicit behavior option also overrides CSS scroll-behavior.
  • It can be prevented now without waiting on the autoscroll bugs, e.g. by having the helper coerce smooth window.scrollTo/scrollBy calls to instant. That only removes the animation, though. The cursor scroll is still scheduled on the next tick and throttled to 400ms, so the note can still jump after it is found. Keep waitForElementAtRest for now.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not the browser's default. The scroll is em's own scrollCursorIntoView (src/device/scrollCursorIntoView.ts). Safari's native scroll-on-focus is already suppressed for notes and thoughts by preventAutoscroll. After the paste moves the cursor, scrollCursorIntoView scrolls with behavior: 'smooth' whenever the distance is less than one viewport height.

There is already an automation guard, navigator.webdriver ? 'instant' : behavior, in both scrollCursorIntoView and scrollTo. On iOS it never fires, because Appium leaves navigator.webdriver false in mobile Safari. disableTransitions doesn't help either: it only turns off CSS transitions, and an explicit behavior option overrides CSS scroll-behavior anyway.

Can it be prevented? Yes, and it doesn't have to wait for the autoscroll bugs. A test helper could force smooth window.scrollTo/scrollBy calls to scroll instantly.

That only removes the animation, though. scheduleScrollCursorIntoView still runs the scroll on the next tick and throttles it to 400ms. So the note can still move after the test has found it, just in one jump instead of a slide. The test would still have to wait for the scroll to finish, and waitForElementAtRest is what does that. I'd keep it until the cursor scroll gives the test something to wait on, such as a signal that it has finished.

No code changes in this reply, as requested.

@BayuAri
BayuAri removed their request for review October 27, 2026 03:03
The cursor overlay is a child of LayoutTree, so its mount effect ran
scrollCursorIntoView before LayoutTree's passive effect had written
layoutTreeTop. With layoutTreeTop still 0, a cursor in plain view was
measured as behind the toolbar and smooth-scrolled. A layout effect runs
before any passive effect in the commit, so the first check now sees the
tree's real top.

Remove waitForElementAtRest from the note tests, which waited out that
scroll instead of preventing it.

Co-Authored-By: Claude claude-opus-5-5 (medium) <noreply@anthropic.com>
Opening the keyboard with newThought first let Safari scroll the page, so
the cursor scroll after the paste moved the note while the test tapped it.

Co-Authored-By: Claude claude-opus-5-5 (medium) <noreply@anthropic.com>
Co-Authored-By: Claude claude-opus-5-5 (medium) <noreply@anthropic.com>
Co-Authored-By: Claude claude-opus-5-5 (medium) <noreply@anthropic.com>
Safari's WebDriver does not implement getProperty, so the textContent read
failed every attempt.

Co-Authored-By: Claude claude-opus-5-5 (medium) <noreply@anthropic.com>
@raineorshine
raineorshine merged commit b3ea54f into main Oct 27, 2026
17 checks passed
@raineorshine
raineorshine deleted the copilot/fix-caret-placement-in-note branch October 27, 2026 23:51

This branch was successfully deployed

1 active deployment
Preview — a3609926 Deployed Oct 27, 2026 by github-actions[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Mobile] Caret is placed at the end of the Note when user pastes into the middle of the Note

4 participants