-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix: resolve context menu z-index issue by keeping text area in fixed position #7831
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
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 the solution to keep the text area in a fixed position is creative and effectively solves the z-index issue.
Suggestions for Improvement
Accessibility Concerns
- The blur effect (
blur-[1px] opacity-70) on line 1923 of ChatView.tsx might make it difficult for users with visual impairments to see the context. Consider using a more subtle visual indicator or making this configurable. - The editing overlay (line 1947) should have proper ARIA attributes for screen readers (e.g.,
role="status"andaria-live="polite").
Code Quality
- Potential memory leak: In ChatRow.tsx (line 132), the message event listener for image selection during edit mode should be cleaned up when
isEditingchanges to false, not just on component unmount. - UX improvement: When clicking outside to cancel edit mode, users might accidentally click on interactive elements. Consider adding a semi-transparent overlay behind the editing indicator.
- Code consistency: The blur value
blur-[1px]appears in multiple places. Consider extracting it to a constant for maintainability.
Testing
The new editing overlay functionality would benefit from unit tests to ensure the escape key handling and click-outside behavior work correctly.
Overall, this is a good solution to the z-index problem. The above suggestions would help improve accessibility, prevent potential issues, and make the code more maintainable.
|
Hmm, did you look at all into what it would take to get the right z-index for the context menu with the inline editing? Aside from the bug I think the inline editing is more expected behavior. |
|
@mrubens Yeah, I did set the z-index all the way up but that didn't seem to do the trick, however I can try again since that's a simple change. I will report back. |
|
I dug into the z-index problem and it turns out it’s not really a z-index bug. Each ChatRow creates its own stacking context, so the context menu gets trapped inside and ends up behind later rows. That’s why just bumping z-index didn’t help, and why portals or dynamic z-indexes run into issues with Virtuoso. The approach in this PR makes sense: instead of fighting stacking contexts, we move editing to the main input at the bottom, like WhatsApp and similar apps do. That avoids the whole z-index mess, keeps the UX consistent, and lets us reuse all the features of the main textarea. It’s also a lot simpler to maintain. Much cleaner than trying to get inline editing to play nicely with CSS. |
- Move editing message card to same stacking context as text area - Add Escape key to exit edit mode - Add click-outside functionality to exit edit mode - Ensure edit overlay appears above action buttons and auto-approve checks
- Created DraftPersistenceProvider context to manage draft state - Integrated draft saving when starting message edit - Integrated draft restoration when canceling or saving edit - Added automatic cleanup after restoration to prevent memory leaks - Added comprehensive test coverage
9a1a344 to
36a46de
Compare
|
Thanks @NaccOll I tried to come up with something like this but couldn't, I told the rest of the team to evaluate this option as well, I'll let you know what we decide. |
|
@daniel-lxs Okay. Actually, for bugs like this that are related to me, you can just assign the issue directly to me. I'll fix them. That's my fault. |
|
@NaccOll I think you can create a PR with your fix, it's probably the solution that the team prefers |

Summary
Fixes the context mentions menu appearing behind other elements when editing messages by keeping the text area in its fixed position at the bottom instead of creating inline text areas.
Problem
Previously, when editing a message, a new text area would appear at the message's location in the chat. This caused the context mentions menu (triggered by typing @) to appear behind other chat elements due to stacking context issues.
Solution
Keep the text area always in its normal position at the bottom. When editing a message, show an "Editing message" indicator above the text area while the actual editing happens in the fixed text area. This ensures the context mentions menu stays in the proper stacking context.
Changes
Related Issues
Testing
Important
Fixes context menu z-index issue by keeping text area fixed and adds draft persistence for message editing.
ChatView.tsxto resolve context menu z-index issue.useDraftPersistenceinuseDraftPersistence.tsxto save, restore, and clear drafts.ChatViewwithDraftPersistenceProvider.DraftPersistence.spec.tsxto test draft save, restore, and clear functionalities.ChatRow.tsxto handle edit mode using the fixed text area.This description was created by
for c1332c5. You can customize this summary. It will automatically update as commits are pushed.