fix: Right clicking with a custom context menu open should open another context menu #1526
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1526 +/- ##
==========================================
- Coverage 46.42% 46.42% -0.01%
==========================================
Files 558 556 -2
Lines 35714 35711 -3
Branches 8916 8916
==========================================
- Hits 16582 16580 -2
+ Misses 19082 19081 -1
Partials 50 50
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
|
Please suggest a basic test plan we can use to manually check this, because this has proven tricky. I can do the windows testing. We need to test manually on the full matrix:
|
|
Open a table. Right click on a row. Right click on another row. Both clicks should bring up the custom context menu. Right click on different tabs in succession. Each should open the custom context menu. This is the e2e test I added to right click on the console tab then command history tab. |
|
This introduces a new bug which prevents the native menu showing. Right clicking on something that would be a native menu previously (like say you are trying to select text and copy text from a previously run command in the console) now it doesn't generate the native menu anymore, and further blocks all interactivity until another left click is performed. I assume we are generating like an empty menu with an overlay or something still when we shouldn't be. Otherwise passes suggested manual testing on all Windows browsers. |
|
Good catch. It's actually an issue w/ just the console (native menu in other places like the maximize button or user settings button works fine). We set/emit console clear and focus history as context actions, but they get filtered out by the util to turn them into menu items. Easy enough to add another check, but seems like a bug/unintended for those to be passed as context actions. |
|
Also noted same behaviour in console history panel's search field. |
|
Ok should be fixed. I took a quick look back, but looks like those actions never registered properly since the initial fork of DHC. They've always been filtered out. Not sure why they're registered as context menu items really other than to setup the shortcut I guess. |
There was a problem hiding this comment.
Actually read Don's comments. Yeah the native menu still doesn't show up on the first right-click after showing our custom menu, unsure if that's something we should handle.
Also the context action items for clear/focus history were intended for keyboard shortcuts. Anything without a title is just a context action item that's triggered by keyboard and shouldn't appear in the menu.
|
@mofojed Still good to merge this? I don't think we can handle showing the right-click after showing our custom menu. The event is getting re-emitted, but the browser doesn't seem to do anything with it. Probably because we listen for the event, but I'm guessing the native browser menu can only be triggered by trusted/user-interaction events, not our re-emit. |
This is okay on the Windows side. |
Fixes #1525
This was bothering me while working on element plugins which should have a panel w/ the custom context menu enabled. Tested on Chrome/Firefox on Linux