Skip to content

Commit

Permalink
fix: Right clicking with a custom context menu open should open anoth…
Browse files Browse the repository at this point in the history
…er context menu (deephaven#1526)

Fixes deephaven#1525
  • Loading branch information
mattrunyon authored Sep 20, 2023
1 parent 9d905fc commit bd08e1f
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 33 deletions.
69 changes: 36 additions & 33 deletions packages/components/src/context-actions/ContextMenuRoot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,44 @@ class ContextMenuRoot extends Component<
openMenu: React.RefObject<ContextMenu>;

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;

// 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) {
// 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)) {
// Open native menu if no custom context actions
return;
}

Expand All @@ -73,38 +106,8 @@ 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
// No actions after filtering. Use native menu
return;
}

Expand Down
15 changes: 15 additions & 0 deletions tests/context-menu.spec.ts
Original file line number Diff line number Diff line change
@@ -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);
});

0 comments on commit bd08e1f

Please sign in to comment.