Skip to content

Conversation

qmorozov
Copy link

@qmorozov qmorozov commented Sep 26, 2025

Problem

When a room list item menu is open (three dots or notification bell), right-clicking on menu items causes a duplicate context menu to appear, overlaying the existing menu. This creates a confusing UX where users see two identical menus stacked on top of each other.

Root cause

The context menu event bubbles up from menu content to the parent room list item, which has its own context menu handler, triggering a second menu to open.

Solution

Implemented conditional event handling based on menu type and state:

  1. More Options Menu: Added conditional e.stopPropagation() only when the menu is open if (open). This prevents duplicate menus when the more options menu is active, but allows right-click functionality when the menu is closed.
  2. Notification Menu: Added setTimeout(() => { setOpen(false); setMenuOpen(false); }, 0) to close the notification menu when right-clicked, allowing the parent context menu to open naturally without duplication.
  3. Context Menu Coordination: Enhanced setMenuOpen logic in RoomListItemView to hide hover menus when context menus open, preventing overlapping UI elements.

The solution provides consistent right-click behavior across different menu types while preventing the original duplication issue.

Before/After videos

Before (bug reproduction):

1.mov

After (fixed):

2.mov

Checklist

@qmorozov qmorozov requested a review from a team as a code owner September 26, 2025 13:47
@CLAassistant
Copy link

CLAassistant commented Sep 26, 2025

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@t3chguy t3chguy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd probably better to be consistent with the right-click behaviour of the menu, which closes the existing menu and opens a new one where you right clicked.

@qmorozov qmorozov force-pushed the fix/room-menu-context-event-bubbling branch from 85fefc5 to fcdc8c2 Compare September 26, 2025 20:52
@qmorozov
Copy link
Author

It'd probably be better to be consistent with the right-click behaviour of the menu, which closes the existing menu and opens a new one where you right-clicked.

@t3chguy Thank you for the feedback! You're absolutely right about consistency. I've updated the implementation to provide consistent right-click behavior: close the existing menu and open the context menu where clicked.

Updated approach:

  • More Options Menu: Conditional e.stopPropagation() only when open to prevent duplicates
  • Notification Menu: Closes automatically on right-click, allowing context menu to open naturally

I've updated the PR with a new video demonstration and an updated description that shows the consistent behavior across both menu types. Added comprehensive test coverage as well

@t3chguy
Copy link
Member

t3chguy commented Oct 1, 2025

Looks like your PR causes graphical changes

@qmorozov qmorozov force-pushed the fix/room-menu-context-event-bubbling branch 2 times, most recently from 5939a06 to a6d610d Compare October 3, 2025 20:25
@qmorozov
Copy link
Author

qmorozov commented Oct 3, 2025

Looks like your PR causes graphical changes

Yes, fixed it

@qmorozov qmorozov force-pushed the fix/room-menu-context-event-bubbling branch 2 times, most recently from ea0652a to 0c75e96 Compare October 6, 2025 08:10
@t3chguy
Copy link
Member

t3chguy commented Oct 6, 2025

Looks like there are still unexpected visual changes with the ... button having moved vertically

@qmorozov
Copy link
Author

qmorozov commented Oct 6, 2025

Looks like there are still unexpected visual changes with the ... button having moved vertically

@t3chguy I've run the visual regression tests locally and all 29 screenshot tests pass without any visual differences. The tests specifically check the position of the "..." button when hovered over.

Also, I noticed the SonarQube check is failing with "Could not find a default branch for project with key element-web". This appears to be a SonarQube configuration issue unrelated to my changes. Is this a known issue or something I should address?

@t3chguy
Copy link
Member

t3chguy commented Oct 6, 2025

I've run the visual regression tests locally and all 29 screenshot tests pass without any visual differences.

Yes, because you already committed to update the visual snapshots, so now they "match" but are still different than they are before this PR which is not expected and would require design review.

image

@qmorozov qmorozov force-pushed the fix/room-menu-context-event-bubbling branch 3 times, most recently from 5dc3577 to d7892d7 Compare October 6, 2025 12:13
@qmorozov qmorozov force-pushed the fix/room-menu-context-event-bubbling branch 3 times, most recently from 184bd45 to 370db88 Compare October 6, 2025 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Defect Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants