From 802535cb2162fce627af705a93dab39349fb6bf6 Mon Sep 17 00:00:00 2001 From: Matthew Runyon Date: Tue, 19 Sep 2023 01:04:48 -0500 Subject: [PATCH 1/2] fix: Right clicking with a custom context menu open should open another context menu --- .../src/context-actions/ContextMenuRoot.tsx | 74 +++++++++---------- tests/context-menu.spec.ts | 15 ++++ 2 files changed, 52 insertions(+), 37 deletions(-) create mode 100644 tests/context-menu.spec.ts diff --git a/packages/components/src/context-actions/ContextMenuRoot.tsx b/packages/components/src/context-actions/ContextMenuRoot.tsx index b6e0ed4455..82c5a33877 100644 --- a/packages/components/src/context-actions/ContextMenuRoot.tsx +++ b/packages/components/src/context-actions/ContextMenuRoot.tsx @@ -58,11 +58,46 @@ class ContextMenuRoot extends Component< openMenu: React.RefObject; handleContextMenu(e: MouseEvent): void { - if (!ContextActionUtils.isContextActionEvent(e)) { + if (!this.container.current) { return; } - if (!this.container.current) { + const parentRect = this.container.current.getBoundingClientRect(); + const top = e.clientY - parentRect.top; + const left = e.clientX - parentRect.left; + const { actions } = this.state; + + // Mac and Linux appear to trigger contextmenu events on mousedown vs. mouseup on Windows + // Mouseup on Windows triggers blur before contextmenu which effectively does what this path does + if (actions != null && e.target === this.container.current) { + // re-emit right clicks that hit the context-root blocking layer + // while we already have a custom context menu open + e.preventDefault(); + + // Set actions to null removes the menu + // That allows a new menu to be opened on a different element so initial position is set properly + // Otherwise the instance of this menu may be reused + // A new contextmenu event is triggered on the element at the location the user clicked on the blocking layer + this.setState({ actions: null }, () => { + const element = document.elementFromPoint(left, top); // x y + + const mouseEvent = new MouseEvent('contextmenu', { + clientX: e.clientX, + clientY: e.clientY, + bubbles: true, + cancelable: true, + }); + + element?.dispatchEvent(mouseEvent); + }); + return; + } + + if ( + !ContextActionUtils.isContextActionEvent(e) || + e.contextActions.length === 0 + ) { + // Open native menu if no custom context actions return; } @@ -73,41 +108,6 @@ class ContextMenuRoot extends Component< const contextActions = ContextActionUtils.getMenuItems(e.contextActions); - const parentRect = this.container.current.getBoundingClientRect(); - const top = e.clientY - parentRect.top; - const left = e.clientX - parentRect.left; - - if (contextActions.length === 0) { - // This code path seems to only exist for Chrome on Mac - // Mac appears to trigger contextmenu events on mousedown vs. mouseup on Windows - // Mouseup on Windows triggers blur before contextmenu which effectively does what this path does - if (e.target === this.container.current) { - // re-emit right clicks that hit the context-root blocking layer - e.preventDefault(); - - // Set actions to null removes the menu - // That allows a new menu to be opened on a different element so initial position is set properly - // Otherwise the instance of this menu may be reused - // A new contextmenu event is triggered on the element at the location the user clicked on the blocking layer - this.setState({ actions: null }, () => { - const element = document.elementFromPoint(left, top); // x y - - const mouseEvent = new MouseEvent('contextmenu', { - clientX: e.clientX, - clientY: e.clientY, - bubbles: true, - cancelable: true, - }); - - element?.dispatchEvent(mouseEvent); - }); - return; - } - - // target was a menu item - return; - } - // new clicks, set actions e.preventDefault(); this.setState({ diff --git a/tests/context-menu.spec.ts b/tests/context-menu.spec.ts new file mode 100644 index 0000000000..d095cdc308 --- /dev/null +++ b/tests/context-menu.spec.ts @@ -0,0 +1,15 @@ +import { test, expect } from '@playwright/test'; + +test('open custom context menu with another custom context menu open', async ({ + page, +}) => { + await page.goto(''); + + await page.getByText('Console').click({ button: 'right' }); + await expect(page.getByText('Close', { exact: true })).toHaveCount(1); + + await page + .getByText('Command History') + .click({ button: 'right', force: true }); + await expect(page.getByText('Close', { exact: true })).toHaveCount(1); +}); From 9c546a57abf25e767c2afdf703a9fc58b80d4c46 Mon Sep 17 00:00:00 2001 From: Matthew Runyon Date: Tue, 19 Sep 2023 14:35:22 -0500 Subject: [PATCH 2/2] Fix empty context menu not falling back to native menu --- .../src/context-actions/ContextMenuRoot.tsx | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/packages/components/src/context-actions/ContextMenuRoot.tsx b/packages/components/src/context-actions/ContextMenuRoot.tsx index 82c5a33877..93d3f54c94 100644 --- a/packages/components/src/context-actions/ContextMenuRoot.tsx +++ b/packages/components/src/context-actions/ContextMenuRoot.tsx @@ -67,6 +67,7 @@ class ContextMenuRoot extends Component< const left = e.clientX - parentRect.left; const { actions } = this.state; + // Context menu is open and user clicked on the context-root blocking layer // Mac and Linux appear to trigger contextmenu events on mousedown vs. mouseup on Windows // Mouseup on Windows triggers blur before contextmenu which effectively does what this path does if (actions != null && e.target === this.container.current) { @@ -93,10 +94,7 @@ class ContextMenuRoot extends Component< return; } - if ( - !ContextActionUtils.isContextActionEvent(e) || - e.contextActions.length === 0 - ) { + if (!ContextActionUtils.isContextActionEvent(e)) { // Open native menu if no custom context actions return; } @@ -108,6 +106,11 @@ class ContextMenuRoot extends Component< const contextActions = ContextActionUtils.getMenuItems(e.contextActions); + if (contextActions.length === 0) { + // No actions after filtering. Use native menu + return; + } + // new clicks, set actions e.preventDefault(); this.setState({