Repository navigation
fix(diffs): keep the native selection in sync with keyboard selections - #1245
lucasmeijer wants to merge 1 commit into
Conversation
|
@lucasmeijer is attempting to deploy a commit to the Pierre Computer Company Team on Vercel. A member of the Team first needs to authorize it. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e36acc649
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (echo !== undefined && isSameRange(echo, composedRange)) { | ||
| return; |
There was a problem hiding this comment.
Do not swallow a Shift-click on the selection anchor
When a ranged selection is active and the user Shift-clicks its anchor, the pointerdown path in #listenContentElement first calls #setWindowSelection with a collapsed range at that anchor. The browser's final range is the same collapsed range, so this branch classifies the only resulting selectionchange as an echo and returns before the #isContentMouseDown && #shiftKeyPressed / extendSelection path can collapse the editor selection; the old range therefore stays selected. Do not consume this echo during the Shift-click gesture (the existing extendSelection tests explicitly expect anchor clicks to collapse).
Useful? React with 👍 / 👎.
Hi, human Lucas here. I'm a very happy user of diffs!
Please consider this pull request a bug report that contains:
description of the problem i have as a user
Everything above here is me talking, everything below here is AI generated description. the patch is ai generated, the videos are screen recordings my agent made. I hope you find this a useful shape of a bug fix / contribution. Thanks for Diffs! Bye, Lucas.
Description
Two related bugs with keyboard selections in the editor. Both come from the editor collapsing the window (native) selection to the anchor while it shows a range.
#focus(anchor)writes a collapsed window selection at the anchor. The resultingselectionchangeis ignored only for two animation frames. Chromium sometimes delivers it later, and the handler then reads it as the user placing the caret at the anchor. While Shift is held,#selectionStartis set, so that path accepts it. On currentmainthis happens in most runs. In our headless-shell runs it collapsed in 36 of 48 tries.selectionchangefires.Before (current
main):selection-before.mp4
After (this PR):
selection-after.mp4
Fix, in
editor.ts:#focus()takes an optionalselection, and the publicfocus()and#scrollToPrimaryCaret()pass the primary selection. The window selection now mirrors the whole range instead of a collapsed caret at the anchor. That fixes (2), and it also means a late echo carries the same bounds as the editor's selection.#setWindowSelection()remembers the boundary points it applied (#windowSelectionEcho). Theselectionchangehandler ignores one event reporting exactly those points, however late it arrives. Any pointer press clears the remembered range, so real clicks always count. That fixes (1).getComposedWindowRange()helper. Behavior is unchanged, including theShadowRoot.getSelection()fallback.Motivation & Context
In the AgentsInTheCloud file editor, selecting text with Shift+Arrows looked random: sometimes it worked, sometimes it snapped back to where the selection started.
Type of changes
Checklist
moon run root:lint)moon run root:format)moonx diffs:test)Tests added (
code-view-selection.pw.ts+ fixturecode-view-selection.html):selectionchange. This simulates the late delivery deterministically. It fails onmain, where the selection collapses to the anchor, and passes with the fix.mainand passes with the fix.diffs:testhas 3 failures inCodeView item collapsed statein my environment. They fail the same way onmainwithout this change. In the e2e suite,code-view-annotations(3 tests) andedit.pw.ts › programmatic refocus…also fail identically onmainlocally.How was AI used in generating this PR
An AI coding agent (Claude) reproduced and root-caused the issue and wrote the change, the tests, the fixture and this description. It also recorded the before/after videos and wrote their captions. The videos are real screen recordings of the fixture in headless Chromium. I reviewed the change and the reasoning above.
Related issues
None that I could find.