Improve user message editing behavior by preventing mode switch when text is selected; enhance button styling for better usability#104
Open
FranciscoMoretti wants to merge 1 commit intomainfrom
Conversation
…text is selected; enhance button styling for better usability
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts user message editing so clicking while text is selected no longer switches to edit mode, and tweaks the message content button styling to allow text selection while preserving existing hover behavior. Flow diagram for message content click handler decisionflowchart TD
A[User clicks message content button] --> B[Get current selection using window.getSelection]
B --> C{Does selection exist and contain text?}
C -- Yes --> D[Do nothing<br>Stay in view mode]
C -- No --> E[Call setMode with value edit]
E --> F[Component re-renders in edit mode]
D --> G[User can copy or interact with selected text]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Using
window.getSelection()?.toString()at the page level will block editing whenever any text on the page is selected; consider scoping the check to selections within this message element (e.g., inspectinggetSelection().anchorNode/focusNodeancestry) so other-page selections don’t affect edit behavior. - The inline click handler is getting a bit more complex; extracting it into a small named function (e.g.,
handleMessageClick) would improve readability and make the selection logic easier to extend if needed.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using `window.getSelection()?.toString()` at the page level will block editing whenever *any* text on the page is selected; consider scoping the check to selections within this message element (e.g., inspecting `getSelection().anchorNode`/`focusNode` ancestry) so other-page selections don’t affect edit behavior.
- The inline click handler is getting a bit more complex; extracting it into a small named function (e.g., `handleMessageClick`) would improve readability and make the selection logic easier to extend if needed.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Contributor
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/chat/components/user-message.tsx">
<violation number="1" location="apps/chat/components/user-message.tsx:81">
P2: The edit-mode guard checks global document selection, so text selected anywhere on the page can incorrectly block entering edit mode for this message.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Comment on lines
+81
to
+84
| onClick={() => { | ||
| if (window.getSelection()?.toString()) return; | ||
| setMode("edit"); | ||
| }} |
Contributor
There was a problem hiding this comment.
P2: The edit-mode guard checks global document selection, so text selected anywhere on the page can incorrectly block entering edit mode for this message.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/chat/components/user-message.tsx, line 81:
<comment>The edit-mode guard checks global document selection, so text selected anywhere on the page can incorrectly block entering edit mode for this message.</comment>
<file context>
@@ -76,9 +76,12 @@ const PureUserMessage = ({
+ className="block cursor-pointer select-text text-left transition-opacity hover:opacity-80"
data-testid="message-content"
- onClick={() => setMode("edit")}
+ onClick={() => {
+ if (window.getSelection()?.toString()) return;
+ setMode("edit");
</file context>
Suggested change
| onClick={() => { | |
| if (window.getSelection()?.toString()) return; | |
| setMode("edit"); | |
| }} | |
| onClick={(event) => { | |
| const selection = window.getSelection(); | |
| if ( | |
| selection && | |
| !selection.isCollapsed && | |
| event.currentTarget.contains(selection.anchorNode) | |
| ) | |
| return; | |
| setMode("edit"); | |
| }} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Prevent user messages from switching to edit mode when text is selected and make message content selectable in view mode for improved usability.
Enhancements:
Summary by cubic
Stop switching to edit mode when a user clicks a message with text selected, so selecting and copying works as expected.
Also enable text selection in view mode by adding the select-text class to the message button.
Written for commit 74f09ed. Summary will update on new commits.