-
Notifications
You must be signed in to change notification settings - Fork 2.5k
Fix context menu is obscured when edit message. #7951
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix context menu is obscured when edit message. #7951
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! I've reviewed the changes and found some issues that need attention. While the approach to fix the context menu positioning is valid, there are concerns about potential layout regressions and the robustness of the solution across different viewport sizes.
| "relative flex flex-col gap-1 bg-editor-background outline-none border border-none box-border", | ||
| isEditMode ? "p-2 w-full" : "px-1.5 pb-1 w-[calc(100%-16px)] ml-auto mr-auto", | ||
| "flex flex-col gap-1 bg-editor-background outline-none border border-none box-border", | ||
| isEditMode ? "p-2 w-full" : "relative px-1.5 pb-1 w-[calc(100%-16px)] ml-auto mr-auto", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intentional? Removing relative from the main container and only applying it conditionally to non-edit mode could break the positioning context for child elements in normal mode. This might cause unexpected layout issues.
Could we keep relative on the main container and use a different approach for fixing the edit mode positioning?
| "absolute", | ||
| "bottom-full", | ||
| "left-0", | ||
| isEditMode ? "left-6" : "left-0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded positioning values (left-6 and -mb-3) might not work well across different screen sizes or container widths. Could we consider a more responsive approach?
Or perhaps calculate the position based on the container's dimensions?
| "right-0", | ||
| "z-[1000]", | ||
| "mb-2", | ||
| isEditMode ? "-mb-3" : "mb-2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that PR #7760 attempted to fix this issue using z-index adjustments. Have you tested if combining both approaches (z-index + positioning) would provide a more robust solution? This might handle edge cases better.
daniel-lxs
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @NaccOll!
Related GitHub Issue
Closes: #7759
Roo Code Task Context (Optional)
Description
Adjust the position to avoid ContextMenu being blocked in edit mode, so as to achieve an effect similar to the ChatTextArea at the bottom
Test Procedure
In any conversation, select the user message, click Edit, and enter the @ symbol to test whether the context menu is working well.
Pre-Submission Checklist
Screenshots / Videos
Documentation Updates
Additional Notes
Get in Touch
NaccOll
Important
Adjusts context menu positioning in
ChatTextArea.tsxto ensure visibility during message editing.ChatTextArea.tsxto ensure visibility during message editing.left-6and-mb-3for context menu positioning.relativeclass from the main container in edit mode to prevent layout issues.flexandflex-colclasses.This description was created by
for 578943c. You can customize this summary. It will automatically update as commits are pushed.