Repository navigation
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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. |
0f2f3ab to
596eeb7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f2f3abc8b
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /* Selectable: the message body exists to be read and copied, and without | ||
| * this it inherits the shell's user-select: none — drag selection and | ||
| * Ctrl+C silently do nothing (issue #739). Scoped to the bubble so | ||
| * buttons, the timeline, and the inspector keep their non-selectable | ||
| * behavior. */ | ||
| user-select: text; |
There was a problem hiding this comment.
Preserve native copy for selected chat text
On Windows/Linux, if a timeline region or clip remains selected when the user drag-selects a message, Ctrl+C is still caught by the editor shortcut in NewEditorShell.tsx:1303-1314; that handler calls preventDefault() and copies the region/clip instead, leaving the OS clipboard without the selected message text. Opening or interacting with chat does not clear those timeline selections, and the new Playwright test only inspects window.getSelection() rather than exercising the copy command. Give a non-collapsed DOM text selection precedence over the editor-level copy shortcut and cover that scenario.
AGENTS.md reference: AGENTS.md:L78-L79
Useful? React with 👍 / 👎.
f83edae to
0e4b3a4
Compare
The global stylesheet disables selection on body and the editor shell re-disables it on .app, so drag selection across AI chat message text silently did nothing — and with no selection, Ctrl+C and Edit → Copy had nothing to copy (issue getopenscreen#739). Opt the message bubble back in with user-select: text. The rule is scoped to .msgBubble, so buttons, the timeline, the inspector, and the panel chrome keep their non-selectable behavior. tests/e2e/v4-shell.spec.ts gains one test in its existing harness: a real-mouse drag across the user turn and across the shim's canned assistant reply must produce a selection, and the session header must still resolve to user-select: none. The drag assertions fail without the CSS change and pass with it.
0e4b3a4 to
8f70a81
Compare
Summary
Fixes getopenscreen#739. AI chat message text could not be selected with the mouse: the global stylesheet disables selection on
bodyand the editor shell re-disables it on.app, and nothing opted the chat bubble back in. With no selection possible, Ctrl+C and Edit → Copy had nothing to act on — which hurt most when the message contained a command or a code snippet and the message's own Copy button was also failing (see getopenscreen#738).The fix
One scoped CSS rule:
.msgBubble { user-select: text; }inNewEditorShell.module.css. The bubble is exactly the readable body of a chat message; buttons, the timeline, the inspector, and the panel chrome keep inheritinguser-select: none, so nothing else becomes selectable.Regression evidence (old FAIL / new PASS)
tests/e2e/chat-message-selection.spec.ts(Playwright, browser-shim editor, follows thev4-shell.spec.tsseeding pattern) sends a chat turn so the shim's canned assistant reply renders, then drags across the bubbles with real mouse events and asserts the selection is real bubble text; it also asserts the session header still computesuser-select: none.editor-smoke,v4-shell) still pass.Real-machine verification (Windows 11, computer-use driven)
main(dev build): a real-mouse drag across the assistant text and the user text created no selection, and Ctrl+C left the clipboard (sentinel read-back) untouched — matching the report.One scoping decision to flag: the expanded "thinking" trace stays non-selectable — it is ephemeral (only exists for the turn that streamed it), it is wrapped in the collapsible toggle button, and the issue's acceptance case covers user/assistant message bodies. Enabling it later is a one-liner if wanted.
Intent
Opened against the fork for review; ready for an owner decision before any upstream submission (per the standing rule, no upstream PR will be created without explicit approval).