From 297a943a79bc4621fb5c5188c2e9ed7d1a110f90 Mon Sep 17 00:00:00 2001 From: gagik Date: Tue, 20 May 2025 16:59:43 +0200 Subject: [PATCH 01/46] feat(compass-context-menu): add a headless context menu package --- package-lock.json | 104 ++++++++++++++++++ packages/compass-context-menu/.depcheckrc | 8 ++ packages/compass-context-menu/.eslintignore | 2 + packages/compass-context-menu/.eslintrc.js | 9 ++ packages/compass-context-menu/.mocharc.js | 2 + packages/compass-context-menu/package.json | 72 ++++++++++++ .../src/context-menu-content.ts | 23 ++++ .../src/context-menu-provider.tsx | 68 ++++++++++++ .../compass-context-menu/src/context-menu.tsx | 22 ++++ packages/compass-context-menu/src/types.ts | 21 ++++ .../src/use-context-menu.tsx | 55 +++++++++ .../compass-context-menu/tsconfig-lint.json | 5 + packages/compass-context-menu/tsconfig.json | 8 ++ 13 files changed, 399 insertions(+) create mode 100644 packages/compass-context-menu/.depcheckrc create mode 100644 packages/compass-context-menu/.eslintignore create mode 100644 packages/compass-context-menu/.eslintrc.js create mode 100644 packages/compass-context-menu/.mocharc.js create mode 100644 packages/compass-context-menu/package.json create mode 100644 packages/compass-context-menu/src/context-menu-content.ts create mode 100644 packages/compass-context-menu/src/context-menu-provider.tsx create mode 100644 packages/compass-context-menu/src/context-menu.tsx create mode 100644 packages/compass-context-menu/src/types.ts create mode 100644 packages/compass-context-menu/src/use-context-menu.tsx create mode 100644 packages/compass-context-menu/tsconfig-lint.json create mode 100644 packages/compass-context-menu/tsconfig.json diff --git a/package-lock.json b/package-lock.json index 3cfca4a376d..f006b787bbf 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7893,6 +7893,10 @@ "resolved": "packages/compass-connections-navigation", "link": true }, + "node_modules/@mongodb-js/compass-context-menu": { + "resolved": "packages/compass-context-menu", + "link": true + }, "node_modules/@mongodb-js/compass-crud": { "resolved": "packages/compass-crud", "link": true @@ -44219,6 +44223,62 @@ "node": ">=0.3.1" } }, + "packages/compass-context-menu": { + "name": "@mongodb-js/compass-context-menu", + "version": "0.0.1", + "license": "SSPL", + "dependencies": { + "react": "^17.0.2" + }, + "devDependencies": { + "@mongodb-js/eslint-config-compass": "^1.3.8", + "@mongodb-js/mocha-config-compass": "^1.6.8", + "@mongodb-js/prettier-config-compass": "^1.2.8", + "@mongodb-js/tsconfig-compass": "^1.2.8", + "@types/chai": "^4.2.21", + "@types/mocha": "^9.0.0", + "@types/react": "^17.0.5", + "@types/react-dom": "^17.0.10", + "@types/sinon-chai": "^3.2.5", + "chai": "^4.3.6", + "depcheck": "^1.4.1", + "gen-esm-wrapper": "^1.1.0", + "mocha": "^10.2.0", + "nyc": "^15.1.0", + "sinon": "^9.2.3", + "typescript": "^5.0.4" + } + }, + "packages/compass-context-menu/node_modules/diff": { + "version": "4.0.2", + "resolved": "https://registry.npmjs.org/diff/-/diff-4.0.2.tgz", + "integrity": "sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A==", + "dev": true, + "license": "BSD-3-Clause", + "engines": { + "node": ">=0.3.1" + } + }, + "packages/compass-context-menu/node_modules/sinon": { + "version": "9.2.4", + "resolved": "https://registry.npmjs.org/sinon/-/sinon-9.2.4.tgz", + "integrity": "sha512-zljcULZQsJxVra28qIAL6ow1Z9tpattkCTEJR4RBP3TGc00FcttsP5pK284Nas5WjMZU5Yzy3kAIp3B3KRf5Yg==", + "deprecated": "16.1.1", + "dev": true, + "license": "BSD-3-Clause", + "dependencies": { + "@sinonjs/commons": "^1.8.1", + "@sinonjs/fake-timers": "^6.0.1", + "@sinonjs/samsam": "^5.3.1", + "diff": "^4.0.2", + "nise": "^4.0.4", + "supports-color": "^7.1.0" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/sinon" + } + }, "packages/compass-crud": { "name": "@mongodb-js/compass-crud", "version": "13.56.1", @@ -56727,6 +56787,50 @@ } } }, + "@mongodb-js/compass-context-menu": { + "version": "file:packages/compass-context-menu", + "requires": { + "@mongodb-js/eslint-config-compass": "^1.3.8", + "@mongodb-js/mocha-config-compass": "^1.6.8", + "@mongodb-js/prettier-config-compass": "^1.2.8", + "@mongodb-js/tsconfig-compass": "^1.2.8", + "@types/chai": "^4.2.21", + "@types/mocha": "^9.0.0", + "@types/react": "^17.0.5", + "@types/react-dom": "^17.0.10", + "@types/sinon-chai": "^3.2.5", + "chai": "^4.3.6", + "depcheck": "^1.4.1", + "gen-esm-wrapper": "^1.1.0", + "mocha": "^10.2.0", + "nyc": "^15.1.0", + "react": "^17.0.2", + "sinon": "^9.2.3", + "typescript": "^5.0.4" + }, + "dependencies": { + "diff": { + "version": "4.0.2", + "resolved": "https://registry.npmjs.org/diff/-/diff-4.0.2.tgz", + "integrity": "sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A==", + "dev": true + }, + "sinon": { + "version": "9.2.4", + "resolved": "https://registry.npmjs.org/sinon/-/sinon-9.2.4.tgz", + "integrity": "sha512-zljcULZQsJxVra28qIAL6ow1Z9tpattkCTEJR4RBP3TGc00FcttsP5pK284Nas5WjMZU5Yzy3kAIp3B3KRf5Yg==", + "dev": true, + "requires": { + "@sinonjs/commons": "^1.8.1", + "@sinonjs/fake-timers": "^6.0.1", + "@sinonjs/samsam": "^5.3.1", + "diff": "^4.0.2", + "nise": "^4.0.4", + "supports-color": "^7.1.0" + } + } + } + }, "@mongodb-js/compass-crud": { "version": "file:packages/compass-crud", "requires": { diff --git a/packages/compass-context-menu/.depcheckrc b/packages/compass-context-menu/.depcheckrc new file mode 100644 index 00000000000..ab0ef21b740 --- /dev/null +++ b/packages/compass-context-menu/.depcheckrc @@ -0,0 +1,8 @@ +ignores: + - '@mongodb-js/prettier-config-compass' + - '@mongodb-js/tsconfig-compass' + - '@types/chai' + - '@types/sinon-chai' + - 'sinon' +ignore-patterns: + - 'dist' diff --git a/packages/compass-context-menu/.eslintignore b/packages/compass-context-menu/.eslintignore new file mode 100644 index 00000000000..85a8a75e68c --- /dev/null +++ b/packages/compass-context-menu/.eslintignore @@ -0,0 +1,2 @@ +.nyc-output +dist diff --git a/packages/compass-context-menu/.eslintrc.js b/packages/compass-context-menu/.eslintrc.js new file mode 100644 index 00000000000..9c3ab95632f --- /dev/null +++ b/packages/compass-context-menu/.eslintrc.js @@ -0,0 +1,9 @@ +'use strict'; +module.exports = { + root: true, + extends: ['@mongodb-js/eslint-config-compass'], + parserOptions: { + tsconfigRootDir: __dirname, + project: ['./tsconfig-lint.json'], + }, +}; diff --git a/packages/compass-context-menu/.mocharc.js b/packages/compass-context-menu/.mocharc.js new file mode 100644 index 00000000000..e7eaccd61fa --- /dev/null +++ b/packages/compass-context-menu/.mocharc.js @@ -0,0 +1,2 @@ +'use strict'; +module.exports = require('@mongodb-js/mocha-config-compass'); diff --git a/packages/compass-context-menu/package.json b/packages/compass-context-menu/package.json new file mode 100644 index 00000000000..3cf186e0270 --- /dev/null +++ b/packages/compass-context-menu/package.json @@ -0,0 +1,72 @@ +{ + "name": "@mongodb-js/compass-context-menu", + "author": { + "name": "MongoDB Inc", + "email": "compass@mongodb.com" + }, + "publishConfig": { + "access": "public" + }, + "bugs": { + "url": "https://jira.mongodb.org/projects/COMPASS/issues", + "email": "compass@mongodb.com" + }, + "homepage": "https://github.com/mongodb-js/compass", + "version": "0.0.1", + "repository": { + "type": "git", + "url": "https://github.com/mongodb-js/compass.git" + }, + "files": [ + "dist" + ], + "license": "SSPL", + "main": "dist/index.js", + "compass:main": "src/index.ts", + "exports": { + "import": "./dist/.esm-wrapper.mjs", + "require": "./dist/index.js" + }, + "compass:exports": { + ".": "./src/index.ts" + }, + "types": "./dist/index.d.ts", + "scripts": { + "bootstrap": "npm run compile", + "prepublishOnly": "npm run compile && compass-scripts check-exports-exist", + "compile": "tsc -p tsconfig.json && gen-esm-wrapper . ./dist/.esm-wrapper.mjs", + "typecheck": "tsc -p tsconfig-lint.json --noEmit", + "eslint": "eslint-compass", + "prettier": "prettier-compass", + "lint": "npm run eslint . && npm run prettier -- --check .", + "depcheck": "compass-scripts check-peer-deps && depcheck", + "check": "npm run typecheck && npm run lint && npm run depcheck", + "check-ci": "npm run check", + "test": "mocha", + "test-cov": "nyc --compact=false --produce-source-map=false -x \"**/*.spec.*\" --reporter=lcov --reporter=text --reporter=html npm run test", + "test-watch": "npm run test -- --watch", + "test-ci": "npm run test-cov", + "reformat": "npm run eslint . -- --fix && npm run prettier -- --write ." + }, + "dependencies": { + "react": "^17.0.2" + }, + "devDependencies": { + "@mongodb-js/eslint-config-compass": "^1.3.8", + "@mongodb-js/mocha-config-compass": "^1.6.8", + "@mongodb-js/prettier-config-compass": "^1.2.8", + "@mongodb-js/tsconfig-compass": "^1.2.8", + "@types/chai": "^4.2.21", + "@types/mocha": "^9.0.0", + "@types/react": "^17.0.5", + "@types/react-dom": "^17.0.10", + "@types/sinon-chai": "^3.2.5", + "chai": "^4.3.6", + "depcheck": "^1.4.1", + "gen-esm-wrapper": "^1.1.0", + "mocha": "^10.2.0", + "nyc": "^15.1.0", + "sinon": "^9.2.3", + "typescript": "^5.0.4" + } +} diff --git a/packages/compass-context-menu/src/context-menu-content.ts b/packages/compass-context-menu/src/context-menu-content.ts new file mode 100644 index 00000000000..6856f1fe684 --- /dev/null +++ b/packages/compass-context-menu/src/context-menu-content.ts @@ -0,0 +1,23 @@ +const CONTEXT_MENUS_SYMBOL = Symbol('context_menus'); + +export type EnhancedMouseEvent = MouseEvent & { + [CONTEXT_MENUS_SYMBOL]?: React.ComponentType[]; +}; + +export function getContextMenuContent( + event: EnhancedMouseEvent +): React.ComponentType[] { + return event[CONTEXT_MENUS_SYMBOL] ?? []; +} + +export function appendContextMenuContent( + event: EnhancedMouseEvent, + content: React.ComponentType +) { + // Initialize if not already patched + if (event[CONTEXT_MENUS_SYMBOL] === undefined) { + event[CONTEXT_MENUS_SYMBOL] = [content]; + return; + } + event[CONTEXT_MENUS_SYMBOL].push(content); +} diff --git a/packages/compass-context-menu/src/context-menu-provider.tsx b/packages/compass-context-menu/src/context-menu-provider.tsx new file mode 100644 index 00000000000..7a7b59bf16a --- /dev/null +++ b/packages/compass-context-menu/src/context-menu-provider.tsx @@ -0,0 +1,68 @@ +import React, { + useCallback, + useEffect, + useState, + useMemo, + createContext, +} from 'react'; +import type { ContextMenuContext, MenuState } from './types'; +import { ContextMenu } from './context-menu'; +import type { EnhancedMouseEvent } from './context-menu-content'; +import { getContextMenuContent } from './context-menu-content'; + +export const Context = createContext(null); + +export function ContextMenuProvider({ + children, +}: React.PropsWithChildren) { + const [menu, setMenu] = useState({ isOpen: false }); + const close = useCallback(() => setMenu({ isOpen: false }), [setMenu]); + + useEffect(() => { + function handleContextMenu(event: MouseEvent) { + event.preventDefault(); + setMenu({ + isOpen: true, + children: getContextMenuContent(event as EnhancedMouseEvent).map( + (Content, index) => + ), + position: { + // TODO: Fix handling offset while scrolling + x: event.clientX, + y: event.clientY, + }, + }); + } + document.addEventListener('contextmenu', handleContextMenu); + + function handleClosingEvent(event: Event) { + if (!event.defaultPrevented) { + setMenu({ isOpen: false }); + } + } + document.addEventListener('click', handleClosingEvent); + window.addEventListener('resize', handleClosingEvent); + + return () => { + document.removeEventListener('contextmenu', handleContextMenu); + document.removeEventListener('click', handleClosingEvent); + window.removeEventListener('resize', handleClosingEvent); + }; + }, [setMenu]); + + const value = useMemo( + () => ({ + close, + }), + [close] + ); + + return ( + <> + {children} + {menu.isOpen && ( + {menu.children} + )} + + ); +} diff --git a/packages/compass-context-menu/src/context-menu.tsx b/packages/compass-context-menu/src/context-menu.tsx new file mode 100644 index 00000000000..b053bc4963a --- /dev/null +++ b/packages/compass-context-menu/src/context-menu.tsx @@ -0,0 +1,22 @@ +import { createPortal } from 'react-dom'; +import React from 'react'; + +type ContextMenuProps = React.PropsWithChildren<{ + position: { + x: number; + y: number; + }; +}>; + +export function ContextMenu({ children, position }: ContextMenuProps) { + const container = document.getElementById('context-menu-container'); + if (container === null) { + throw new Error('Expected a container for the context menu in the DOM'); + } + return createPortal( +
+ {children} +
, + container + ); +} diff --git a/packages/compass-context-menu/src/types.ts b/packages/compass-context-menu/src/types.ts new file mode 100644 index 00000000000..07efb491ac6 --- /dev/null +++ b/packages/compass-context-menu/src/types.ts @@ -0,0 +1,21 @@ +export type MenuState = + | { + isOpen: false; + } + | { + isOpen: true; + children: React.ReactNode; + position: { + x: number; + y: number; + }; + }; + +export type ContextMenuContext = { + close(): void; +}; + +export type MenuItem = { + label: string; + onAction: (event: Event) => void; +}; diff --git a/packages/compass-context-menu/src/use-context-menu.tsx b/packages/compass-context-menu/src/use-context-menu.tsx new file mode 100644 index 00000000000..d7ccc114a90 --- /dev/null +++ b/packages/compass-context-menu/src/use-context-menu.tsx @@ -0,0 +1,55 @@ +import React, { useContext, useMemo, useRef } from 'react'; +import { Context } from './context-menu-provider'; +import { appendContextMenuContent } from './context-menu-content'; +import type { MenuItem } from './types'; + +/** + * @returns an object with methods to {@link register} content for the menu and {@link close} the menu + */ +export function useContextMenu({ + Menu, +}: { + Menu: React.ComponentType<{ + items: MenuItem[]; + }>; +}) { + // Get the close function from the ContextProvider + const context = useContext(Context); + const previous = useRef void]>( + null + ); + + return useMemo(() => { + if (!context) { + throw new Error('useContextMenu called outside of the provider'); + } + + return { + close: context.close.bind(context), + /** + * @returns a callback ref, passed onto the element responsible for triggering the menu. + */ + register(content: React.ComponentType) { + function listener(event: MouseEvent) { + appendContextMenuContent(event, content); + } + return (trigger: HTMLElement | null) => { + if (previous.current) { + const [previousTrigger, previousListener] = previous.current; + previousTrigger.removeEventListener( + 'contextmenu', + previousListener + ); + } + if (trigger) { + trigger.addEventListener('contextmenu', listener); + previous.current = [trigger, listener]; + } + }; + }, + registerItems(items: MenuItem[]) { + return this.register(() => ); + }, + }; + }, [context, Menu]); +} diff --git a/packages/compass-context-menu/tsconfig-lint.json b/packages/compass-context-menu/tsconfig-lint.json new file mode 100644 index 00000000000..6bdef84f322 --- /dev/null +++ b/packages/compass-context-menu/tsconfig-lint.json @@ -0,0 +1,5 @@ +{ + "extends": "./tsconfig.json", + "include": ["**/*"], + "exclude": ["node_modules", "dist"] +} diff --git a/packages/compass-context-menu/tsconfig.json b/packages/compass-context-menu/tsconfig.json new file mode 100644 index 00000000000..79bc84584ce --- /dev/null +++ b/packages/compass-context-menu/tsconfig.json @@ -0,0 +1,8 @@ +{ + "extends": "@mongodb-js/tsconfig-compass/tsconfig.react.json", + "compilerOptions": { + "outDir": "dist" + }, + "include": ["src/**/*"], + "exclude": ["./src/**/*.spec.*"] +} From 68385135f237ac6811ad22055a28678637c4e440 Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 21 May 2025 13:41:30 +0200 Subject: [PATCH 02/46] wip --- .../src/context-menu-provider.tsx | 4 +- .../src/use-context-menu.spec.tsx | 144 ++++++++++++++++++ 2 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 packages/compass-context-menu/src/use-context-menu.spec.tsx diff --git a/packages/compass-context-menu/src/context-menu-provider.tsx b/packages/compass-context-menu/src/context-menu-provider.tsx index 7a7b59bf16a..499d9c273c1 100644 --- a/packages/compass-context-menu/src/context-menu-provider.tsx +++ b/packages/compass-context-menu/src/context-menu-provider.tsx @@ -14,7 +14,9 @@ export const Context = createContext(null); export function ContextMenuProvider({ children, -}: React.PropsWithChildren) { +}: { + children: React.ReactNode; +}) { const [menu, setMenu] = useState({ isOpen: false }); const close = useCallback(() => setMenu({ isOpen: false }), [setMenu]); diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx new file mode 100644 index 00000000000..7337463e819 --- /dev/null +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -0,0 +1,144 @@ +import React from 'react'; +import { + render, + screen, + cleanup, + userEvent, +} from '@mongodb-js/testing-library-compass'; +import { expect } from 'chai'; +import sinon from 'sinon'; +import { useContextMenu } from './use-context-menu'; +import { ContextMenuProvider } from './context-menu-provider'; +import type { MenuItem } from './types'; + +describe('useContextMenu', function () { + const TestMenu: React.FC<{ items: MenuItem[] }> = ({ items }) => ( +
+ {items.map((item, idx) => ( +
+ {item.label} +
+ ))} +
+ ); + + const TestComponent = ({ + onRegister, + }: { + onRegister?: (ref: any) => void; + }) => { + const contextMenu = useContextMenu({ Menu: TestMenu }); + const items: MenuItem[] = [ + { + label: 'Test Item', + onAction: () => { + /* noop */ + }, + }, + ]; + const ref = contextMenu.registerItems(items); + + React.useEffect(() => { + onRegister?.(ref); + }, [ref, onRegister]); + + return ( +
+ Test Component +
+ ); + }; + + afterEach(cleanup); + + describe('when used outside provider', function () { + it('throws an error', function () { + expect(() => { + render(); + }).to.throw('useContextMenu called outside of the provider'); + }); + }); + + describe('when used inside provider', function () { + beforeEach(() => { + // Create the container for the context menu portal + const container = document.createElement('div'); + container.id = 'context-menu-container'; + document.body.appendChild(container); + }); + + afterEach(() => { + // Clean up the container + const container = document.getElementById('context-menu-container'); + if (container) { + document.body.removeChild(container); + } + }); + + it('renders without error', function () { + render( + + + + ); + + expect(screen.getByTestId('test-trigger')).to.exist; + }); + + it('registers context menu event listener', function () { + const onRegister = sinon.spy(); + + render( + + + + ); + + expect(onRegister).to.have.been.calledOnce; + expect(onRegister.firstCall.args[0]).to.be.a('function'); + }); + + it('shows context menu on right click', function () { + render( + + + + ); + + const trigger = screen.getByTestId('test-trigger'); + userEvent.click(trigger, { button: 2 }); + + // The menu should be rendered in the portal + expect(screen.getByTestId('menu-item-Test Item')).to.exist; + }); + + it('cleans up previous event listener when ref changes', function () { + const removeEventListenerSpy = sinon.spy(); + const addEventListenerSpy = sinon.spy(); + + const { rerender } = render( + + + + ); + + // Simulate ref change + const ref = screen.getByTestId('test-trigger'); + Object.defineProperty(ref, 'addEventListener', { + value: addEventListenerSpy, + }); + Object.defineProperty(ref, 'removeEventListener', { + value: removeEventListenerSpy, + }); + + rerender( + + + + ); + + expect(removeEventListenerSpy).to.have.been.calledWith('contextmenu'); + expect(addEventListenerSpy).to.have.been.calledWith('contextmenu'); + }); + }); +}); From 8e30a83b02f6c897c960905d4e2b310b3d2aba48 Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 21 May 2025 15:16:18 +0200 Subject: [PATCH 03/46] fix: add tests --- packages/compass-context-menu/.mocharc.js | 2 +- .../src/use-context-menu.spec.tsx | 58 +++++++------------ .../src/use-context-menu.tsx | 6 +- 3 files changed, 26 insertions(+), 40 deletions(-) diff --git a/packages/compass-context-menu/.mocharc.js b/packages/compass-context-menu/.mocharc.js index e7eaccd61fa..5a33f216327 100644 --- a/packages/compass-context-menu/.mocharc.js +++ b/packages/compass-context-menu/.mocharc.js @@ -1,2 +1,2 @@ 'use strict'; -module.exports = require('@mongodb-js/mocha-config-compass'); +module.exports = require('@mongodb-js/mocha-config-compass/react'); diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx index 7337463e819..8a5cb4b6fee 100644 --- a/packages/compass-context-menu/src/use-context-menu.spec.tsx +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -1,36 +1,37 @@ import React from 'react'; -import { - render, - screen, - cleanup, - userEvent, -} from '@mongodb-js/testing-library-compass'; +import { render, screen, userEvent } from '@mongodb-js/testing-library-compass'; import { expect } from 'chai'; import sinon from 'sinon'; import { useContextMenu } from './use-context-menu'; import { ContextMenuProvider } from './context-menu-provider'; import type { MenuItem } from './types'; +type TestMenuItem = MenuItem & { id: number }; + describe('useContextMenu', function () { - const TestMenu: React.FC<{ items: MenuItem[] }> = ({ items }) => ( + const TestMenu: React.FC<{ items: TestMenuItem[] }> = ({ items }) => (
{items.map((item, idx) => ( -
+
{item.label}
))}
); - const TestComponent = ({ - onRegister, - }: { - onRegister?: (ref: any) => void; - }) => { + const TestComponent = () => { const contextMenu = useContextMenu({ Menu: TestMenu }); - const items: MenuItem[] = [ + const items: TestMenuItem[] = [ { - label: 'Test Item', + id: 1, + label: 'Test A', + onAction: () => { + /* noop */ + }, + }, + { + id: 2, + label: 'Test B', onAction: () => { /* noop */ }, @@ -38,10 +39,6 @@ describe('useContextMenu', function () { ]; const ref = contextMenu.registerItems(items); - React.useEffect(() => { - onRegister?.(ref); - }, [ref, onRegister]); - return (
Test Component @@ -49,8 +46,6 @@ describe('useContextMenu', function () { ); }; - afterEach(cleanup); - describe('when used outside provider', function () { it('throws an error', function () { expect(() => { @@ -59,7 +54,7 @@ describe('useContextMenu', function () { }); }); - describe('when used inside provider', function () { + describe('with valid provider', function () { beforeEach(() => { // Create the container for the context menu portal const container = document.createElement('div'); @@ -85,19 +80,6 @@ describe('useContextMenu', function () { expect(screen.getByTestId('test-trigger')).to.exist; }); - it('registers context menu event listener', function () { - const onRegister = sinon.spy(); - - render( - - - - ); - - expect(onRegister).to.have.been.calledOnce; - expect(onRegister.firstCall.args[0]).to.be.a('function'); - }); - it('shows context menu on right click', function () { render( @@ -105,11 +87,15 @@ describe('useContextMenu', function () { ); + expect(screen.queryByTestId('menu-item-1')).not.to.exist; + expect(screen.queryByTestId('menu-item-2')).not.to.exist; + const trigger = screen.getByTestId('test-trigger'); userEvent.click(trigger, { button: 2 }); // The menu should be rendered in the portal - expect(screen.getByTestId('menu-item-Test Item')).to.exist; + expect(screen.getByTestId('menu-item-1')).to.exist; + expect(screen.getByTestId('menu-item-2')).to.exist; }); it('cleans up previous event listener when ref changes', function () { diff --git a/packages/compass-context-menu/src/use-context-menu.tsx b/packages/compass-context-menu/src/use-context-menu.tsx index d7ccc114a90..37993b04f15 100644 --- a/packages/compass-context-menu/src/use-context-menu.tsx +++ b/packages/compass-context-menu/src/use-context-menu.tsx @@ -6,11 +6,11 @@ import type { MenuItem } from './types'; /** * @returns an object with methods to {@link register} content for the menu and {@link close} the menu */ -export function useContextMenu({ +export function useContextMenu({ Menu, }: { Menu: React.ComponentType<{ - items: MenuItem[]; + items: T[]; }>; }) { // Get the close function from the ContextProvider @@ -47,7 +47,7 @@ export function useContextMenu({ } }; }, - registerItems(items: MenuItem[]) { + registerItems(items: T[]) { return this.register(() => ); }, }; From 7d17984cae90058b8ed9fd0e15d2c5525d9c64a7 Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 21 May 2025 16:50:00 +0200 Subject: [PATCH 04/46] fix: add tests and fix types --- packages/compass-context-menu/src/types.ts | 2 +- .../src/use-context-menu.spec.tsx | 219 ++++++++++++++---- 2 files changed, 175 insertions(+), 46 deletions(-) diff --git a/packages/compass-context-menu/src/types.ts b/packages/compass-context-menu/src/types.ts index 07efb491ac6..e9ac549ba63 100644 --- a/packages/compass-context-menu/src/types.ts +++ b/packages/compass-context-menu/src/types.ts @@ -17,5 +17,5 @@ export type ContextMenuContext = { export type MenuItem = { label: string; - onAction: (event: Event) => void; + onAction: (event: React.KeyboardEvent | React.MouseEvent) => void; }; diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx index 8a5cb4b6fee..1d099272104 100644 --- a/packages/compass-context-menu/src/use-context-menu.spec.tsx +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -6,39 +6,48 @@ import { useContextMenu } from './use-context-menu'; import { ContextMenuProvider } from './context-menu-provider'; import type { MenuItem } from './types'; -type TestMenuItem = MenuItem & { id: number }; - describe('useContextMenu', function () { - const TestMenu: React.FC<{ items: TestMenuItem[] }> = ({ items }) => ( + const TestMenu: React.FC<{ items: MenuItem[] }> = ({ items }) => (
{items.map((item, idx) => ( -
+
item.onAction?.(event)} + onKeyDown={(event) => { + if (event.key === 'Enter') { + item.onAction?.(event); + } + }} + > {item.label}
))}
); - const TestComponent = () => { + const TestComponent = ({ + onRegister, + onAction, + }: { + onRegister?: (ref: any) => void; + onAction?: (id) => void; + }) => { const contextMenu = useContextMenu({ Menu: TestMenu }); - const items: TestMenuItem[] = [ - { - id: 1, - label: 'Test A', - onAction: () => { - /* noop */ - }, - }, + const items: MenuItem[] = [ { - id: 2, - label: 'Test B', - onAction: () => { - /* noop */ - }, + label: 'Test Item', + onAction: () => onAction?.(1), }, ]; const ref = contextMenu.registerItems(items); + React.useEffect(() => { + onRegister?.(ref); + }, [ref, onRegister]); + return (
Test Component @@ -46,6 +55,60 @@ describe('useContextMenu', function () { ); }; + // Add new test components for nested context menu scenario + const ParentComponent = ({ + onAction, + children, + }: { + onAction?: (id: number) => void; + children?: React.ReactNode; + }) => { + const contextMenu = useContextMenu({ Menu: TestMenu }); + const parentItems: MenuItem[] = [ + { + label: 'Parent Item 1', + onAction: () => onAction?.(1), + }, + { + label: 'Parent Item 2', + onAction: () => onAction?.(2), + }, + ]; + const ref = contextMenu.registerItems(parentItems); + + return ( +
+
Parent Component
+ {children} +
+ ); + }; + + const ChildComponent = ({ + onAction, + }: { + onAction?: (id: number) => void; + }) => { + const contextMenu = useContextMenu({ Menu: TestMenu }); + const childItems: MenuItem[] = [ + { + label: 'Child Item 1', + onAction: () => onAction?.(1), + }, + { + label: 'Child Item 2', + onAction: () => onAction?.(2), + }, + ]; + const ref = contextMenu.registerItems(childItems); + + return ( +
+ Child Component +
+ ); + }; + describe('when used outside provider', function () { it('throws an error', function () { expect(() => { @@ -54,7 +117,7 @@ describe('useContextMenu', function () { }); }); - describe('with valid provider', function () { + describe('with a valid provider', function () { beforeEach(() => { // Create the container for the context menu portal const container = document.createElement('div'); @@ -80,6 +143,19 @@ describe('useContextMenu', function () { expect(screen.getByTestId('test-trigger')).to.exist; }); + it('registers context menu event listener', function () { + const onRegister = sinon.spy(); + + render( + + + + ); + + expect(onRegister).to.have.been.calledOnce; + expect(onRegister.firstCall.args[0]).to.be.a('function'); + }); + it('shows context menu on right click', function () { render( @@ -87,44 +163,97 @@ describe('useContextMenu', function () { ); - expect(screen.queryByTestId('menu-item-1')).not.to.exist; - expect(screen.queryByTestId('menu-item-2')).not.to.exist; - const trigger = screen.getByTestId('test-trigger'); userEvent.click(trigger, { button: 2 }); // The menu should be rendered in the portal - expect(screen.getByTestId('menu-item-1')).to.exist; - expect(screen.getByTestId('menu-item-2')).to.exist; + expect(screen.getByTestId('menu-item-Test Item')).to.exist; }); - it('cleans up previous event listener when ref changes', function () { - const removeEventListenerSpy = sinon.spy(); - const addEventListenerSpy = sinon.spy(); + describe('with nested context menus', function () { + it('shows only parent items when right clicking parent area', function () { + render( + + + + ); - const { rerender } = render( - - - - ); + const parentTrigger = screen.getByTestId('parent-trigger'); + userEvent.click(parentTrigger, { button: 2 }); + + // Should show parent items + expect(screen.getByTestId('menu-item-Parent Item 1')).to.exist; + expect(screen.getByTestId('menu-item-Parent Item 2')).to.exist; - // Simulate ref change - const ref = screen.getByTestId('test-trigger'); - Object.defineProperty(ref, 'addEventListener', { - value: addEventListenerSpy, + // Should not show child items + expect(() => screen.getByTestId('menu-item-Child Item 1')).to.throw; + expect(() => screen.getByTestId('menu-item-Child Item 2')).to.throw; }); - Object.defineProperty(ref, 'removeEventListener', { - value: removeEventListenerSpy, + + it('shows both parent and child items when right clicking child area', function () { + render( + + + + + + ); + + const childTrigger = screen.getByTestId('child-trigger'); + userEvent.click(childTrigger, { button: 2 }); + + // Should show both parent and child items + expect(screen.getByTestId('menu-item-Parent Item 1')).to.exist; + expect(screen.getByTestId('menu-item-Parent Item 2')).to.exist; + expect(screen.getByTestId('menu-item-Child Item 1')).to.exist; + expect(screen.getByTestId('menu-item-Child Item 2')).to.exist; }); - rerender( - - - - ); + it('triggers only the child action when clicking child menu item', function () { + const parentOnAction = sinon.spy(); + const childOnAction = sinon.spy(); + + render( + + + + + + ); + + const childTrigger = screen.getByTestId('child-trigger'); + userEvent.click(childTrigger, { button: 2 }); + + const childItem1 = screen.getByTestId('menu-item-Child Item 1'); + userEvent.click(childItem1); - expect(removeEventListenerSpy).to.have.been.calledWith('contextmenu'); - expect(addEventListenerSpy).to.have.been.calledWith('contextmenu'); + expect(childOnAction).to.have.been.calledOnceWithExactly(1); + expect(parentOnAction).to.not.have.been.called; + expect(() => screen.getByTestId('test-menu')).to.throw; + }); + + it('triggers only the parent action when clicking a parent menu item from child context', function () { + const parentOnAction = sinon.spy(); + const childOnAction = sinon.spy(); + + render( + + + + + + ); + + const childTrigger = screen.getByTestId('child-trigger'); + userEvent.click(childTrigger, { button: 2 }); + + const parentItem1 = screen.getByTestId('menu-item-Parent Item 1'); + userEvent.click(parentItem1); + + expect(parentOnAction).to.have.been.calledOnceWithExactly(1); + expect(childOnAction).to.not.have.been.called; + expect(() => screen.getByTestId('test-menu')).to.throw; + }); }); }); }); From 6004593f9a0c8a458436eab3c9212fdc7d177e37 Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 21 May 2025 17:07:53 +0200 Subject: [PATCH 05/46] refactor: minor stylistic changes --- .../src/context-menu-provider.tsx | 3 +- .../src/use-context-menu.tsx | 53 +++++++++++-------- 2 files changed, 32 insertions(+), 24 deletions(-) diff --git a/packages/compass-context-menu/src/context-menu-provider.tsx b/packages/compass-context-menu/src/context-menu-provider.tsx index 499d9c273c1..43d9e893367 100644 --- a/packages/compass-context-menu/src/context-menu-provider.tsx +++ b/packages/compass-context-menu/src/context-menu-provider.tsx @@ -35,13 +35,14 @@ export function ContextMenuProvider({ }, }); } - document.addEventListener('contextmenu', handleContextMenu); function handleClosingEvent(event: Event) { if (!event.defaultPrevented) { setMenu({ isOpen: false }); } } + + document.addEventListener('contextmenu', handleContextMenu); document.addEventListener('click', handleClosingEvent); window.addEventListener('resize', handleClosingEvent); diff --git a/packages/compass-context-menu/src/use-context-menu.tsx b/packages/compass-context-menu/src/use-context-menu.tsx index 37993b04f15..91f51c2f849 100644 --- a/packages/compass-context-menu/src/use-context-menu.tsx +++ b/packages/compass-context-menu/src/use-context-menu.tsx @@ -3,16 +3,25 @@ import { Context } from './context-menu-provider'; import { appendContextMenuContent } from './context-menu-content'; import type { MenuItem } from './types'; -/** - * @returns an object with methods to {@link register} content for the menu and {@link close} the menu - */ +export type ContextMenuMethods = { + /** + * Close the context menu. + */ + close: () => void; + /** + * Register the menu items for the context menu. + * @returns a callback ref to be passed onto the element responsible for triggering the menu. + */ + registerItems: (items: T[]) => (trigger: HTMLElement | null) => void; +}; + export function useContextMenu({ Menu, }: { Menu: React.ComponentType<{ items: T[]; }>; -}) { +}): ContextMenuMethods { // Get the close function from the ContextProvider const context = useContext(Context); const previous = useRef void]>( @@ -24,31 +33,29 @@ export function useContextMenu({ throw new Error('useContextMenu called outside of the provider'); } + const register = (content: React.ComponentType) => { + function listener(event: MouseEvent) { + appendContextMenuContent(event, content); + } + return (trigger: HTMLElement | null) => { + if (previous.current) { + const [previousTrigger, previousListener] = previous.current; + previousTrigger.removeEventListener('contextmenu', previousListener); + } + if (trigger) { + trigger.addEventListener('contextmenu', listener); + previous.current = [trigger, listener]; + } + }; + }; + return { close: context.close.bind(context), /** * @returns a callback ref, passed onto the element responsible for triggering the menu. */ - register(content: React.ComponentType) { - function listener(event: MouseEvent) { - appendContextMenuContent(event, content); - } - return (trigger: HTMLElement | null) => { - if (previous.current) { - const [previousTrigger, previousListener] = previous.current; - previousTrigger.removeEventListener( - 'contextmenu', - previousListener - ); - } - if (trigger) { - trigger.addEventListener('contextmenu', listener); - previous.current = [trigger, listener]; - } - }; - }, registerItems(items: T[]) { - return this.register(() => ); + return register(() => ); }, }; }, [context, Menu]); From 43023f4173ec9eb95f0c756c8997d54762c2af38 Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 22 May 2025 11:59:55 +0200 Subject: [PATCH 06/46] fix: export types and rename MenuItem --- packages/compass-context-menu/src/index.ts | 2 ++ packages/compass-context-menu/src/types.ts | 2 +- .../compass-context-menu/src/use-context-menu.spec.tsx | 10 +++++----- packages/compass-context-menu/src/use-context-menu.tsx | 6 +++--- 4 files changed, 11 insertions(+), 9 deletions(-) create mode 100644 packages/compass-context-menu/src/index.ts diff --git a/packages/compass-context-menu/src/index.ts b/packages/compass-context-menu/src/index.ts new file mode 100644 index 00000000000..d60a97e04b3 --- /dev/null +++ b/packages/compass-context-menu/src/index.ts @@ -0,0 +1,2 @@ +export { useContextMenu } from './use-context-menu'; +export type { ContextMenuItem } from './types'; diff --git a/packages/compass-context-menu/src/types.ts b/packages/compass-context-menu/src/types.ts index e9ac549ba63..f453930dcb3 100644 --- a/packages/compass-context-menu/src/types.ts +++ b/packages/compass-context-menu/src/types.ts @@ -15,7 +15,7 @@ export type ContextMenuContext = { close(): void; }; -export type MenuItem = { +export type ContextMenuItem = { label: string; onAction: (event: React.KeyboardEvent | React.MouseEvent) => void; }; diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx index 1d099272104..eb857db9aeb 100644 --- a/packages/compass-context-menu/src/use-context-menu.spec.tsx +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -4,10 +4,10 @@ import { expect } from 'chai'; import sinon from 'sinon'; import { useContextMenu } from './use-context-menu'; import { ContextMenuProvider } from './context-menu-provider'; -import type { MenuItem } from './types'; +import type { ContextMenuItem } from './types'; describe('useContextMenu', function () { - const TestMenu: React.FC<{ items: MenuItem[] }> = ({ items }) => ( + const TestMenu: React.FC<{ items: ContextMenuItem[] }> = ({ items }) => (
{items.map((item, idx) => (
void; }) => { const contextMenu = useContextMenu({ Menu: TestMenu }); - const items: MenuItem[] = [ + const items: ContextMenuItem[] = [ { label: 'Test Item', onAction: () => onAction?.(1), @@ -64,7 +64,7 @@ describe('useContextMenu', function () { children?: React.ReactNode; }) => { const contextMenu = useContextMenu({ Menu: TestMenu }); - const parentItems: MenuItem[] = [ + const parentItems: ContextMenuItem[] = [ { label: 'Parent Item 1', onAction: () => onAction?.(1), @@ -90,7 +90,7 @@ describe('useContextMenu', function () { onAction?: (id: number) => void; }) => { const contextMenu = useContextMenu({ Menu: TestMenu }); - const childItems: MenuItem[] = [ + const childItems: ContextMenuItem[] = [ { label: 'Child Item 1', onAction: () => onAction?.(1), diff --git a/packages/compass-context-menu/src/use-context-menu.tsx b/packages/compass-context-menu/src/use-context-menu.tsx index 91f51c2f849..a0c97dead9b 100644 --- a/packages/compass-context-menu/src/use-context-menu.tsx +++ b/packages/compass-context-menu/src/use-context-menu.tsx @@ -1,9 +1,9 @@ import React, { useContext, useMemo, useRef } from 'react'; import { Context } from './context-menu-provider'; import { appendContextMenuContent } from './context-menu-content'; -import type { MenuItem } from './types'; +import type { ContextMenuItem } from './types'; -export type ContextMenuMethods = { +export type ContextMenuMethods = { /** * Close the context menu. */ @@ -15,7 +15,7 @@ export type ContextMenuMethods = { registerItems: (items: T[]) => (trigger: HTMLElement | null) => void; }; -export function useContextMenu({ +export function useContextMenu({ Menu, }: { Menu: React.ComponentType<{ From 971b0fdca53b556f73888e7acc0a7bd8e5781084 Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 22 May 2025 12:13:53 +0200 Subject: [PATCH 07/46] fix: basic UI implementation --- .../src/components/context-menu.tsx | 54 +++++++++++++++++++ packages/compass-context-menu/src/index.ts | 2 + packages/compass-context-menu/src/types.ts | 2 +- .../src/use-context-menu.spec.tsx | 10 ++-- .../src/use-context-menu.tsx | 12 +++-- 5 files changed, 69 insertions(+), 11 deletions(-) create mode 100644 packages/compass-components/src/components/context-menu.tsx create mode 100644 packages/compass-context-menu/src/index.ts diff --git a/packages/compass-components/src/components/context-menu.tsx b/packages/compass-components/src/components/context-menu.tsx new file mode 100644 index 00000000000..9bb9440a217 --- /dev/null +++ b/packages/compass-components/src/components/context-menu.tsx @@ -0,0 +1,54 @@ +import React from 'react'; +import { css, cx } from '@leafygreen-ui/emotion'; +import { Menu, MenuItem, MenuSeparator } from './leafygreen'; +import type { ContextMenuItem } from '@mongodb-js/compass-context-menu'; +import { useContextMenu } from '@mongodb-js/compass-context-menu'; + +const menuStyle = css({ + position: 'fixed', + zIndex: 9999, +}); + +export type ContextMenuProps = { + items: ContextMenuItem[]; + className?: string; + 'data-testid'?: string; +}; + +export function ContextMenu({ + items, + className, + 'data-testid': dataTestId, +}: ContextMenuProps) { + return ( + + {items.map((item, idx) => { + const { label, onAction } = item; + const isLastItem = idx === items.length - 1; + + return ( + <> + {!isLastItem && } + { + evt.stopPropagation(); + onAction?.(evt); + }} + > + {label} + + + ); + })} + + ); +} + +export function useContextMenuItems( + items: ContextMenuItem[] +): React.RefCallback { + const contextMenu = useContextMenu({ Menu: ContextMenu }); + return contextMenu.registerItems(items); +} diff --git a/packages/compass-context-menu/src/index.ts b/packages/compass-context-menu/src/index.ts new file mode 100644 index 00000000000..d60a97e04b3 --- /dev/null +++ b/packages/compass-context-menu/src/index.ts @@ -0,0 +1,2 @@ +export { useContextMenu } from './use-context-menu'; +export type { ContextMenuItem } from './types'; diff --git a/packages/compass-context-menu/src/types.ts b/packages/compass-context-menu/src/types.ts index e9ac549ba63..f453930dcb3 100644 --- a/packages/compass-context-menu/src/types.ts +++ b/packages/compass-context-menu/src/types.ts @@ -15,7 +15,7 @@ export type ContextMenuContext = { close(): void; }; -export type MenuItem = { +export type ContextMenuItem = { label: string; onAction: (event: React.KeyboardEvent | React.MouseEvent) => void; }; diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx index 1d099272104..eb857db9aeb 100644 --- a/packages/compass-context-menu/src/use-context-menu.spec.tsx +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -4,10 +4,10 @@ import { expect } from 'chai'; import sinon from 'sinon'; import { useContextMenu } from './use-context-menu'; import { ContextMenuProvider } from './context-menu-provider'; -import type { MenuItem } from './types'; +import type { ContextMenuItem } from './types'; describe('useContextMenu', function () { - const TestMenu: React.FC<{ items: MenuItem[] }> = ({ items }) => ( + const TestMenu: React.FC<{ items: ContextMenuItem[] }> = ({ items }) => (
{items.map((item, idx) => (
void; }) => { const contextMenu = useContextMenu({ Menu: TestMenu }); - const items: MenuItem[] = [ + const items: ContextMenuItem[] = [ { label: 'Test Item', onAction: () => onAction?.(1), @@ -64,7 +64,7 @@ describe('useContextMenu', function () { children?: React.ReactNode; }) => { const contextMenu = useContextMenu({ Menu: TestMenu }); - const parentItems: MenuItem[] = [ + const parentItems: ContextMenuItem[] = [ { label: 'Parent Item 1', onAction: () => onAction?.(1), @@ -90,7 +90,7 @@ describe('useContextMenu', function () { onAction?: (id: number) => void; }) => { const contextMenu = useContextMenu({ Menu: TestMenu }); - const childItems: MenuItem[] = [ + const childItems: ContextMenuItem[] = [ { label: 'Child Item 1', onAction: () => onAction?.(1), diff --git a/packages/compass-context-menu/src/use-context-menu.tsx b/packages/compass-context-menu/src/use-context-menu.tsx index 91f51c2f849..fa9e20e7537 100644 --- a/packages/compass-context-menu/src/use-context-menu.tsx +++ b/packages/compass-context-menu/src/use-context-menu.tsx @@ -1,9 +1,9 @@ import React, { useContext, useMemo, useRef } from 'react'; import { Context } from './context-menu-provider'; import { appendContextMenuContent } from './context-menu-content'; -import type { MenuItem } from './types'; +import type { ContextMenuItem } from './types'; -export type ContextMenuMethods = { +export type ContextMenuMethods = { /** * Close the context menu. */ @@ -12,10 +12,10 @@ export type ContextMenuMethods = { * Register the menu items for the context menu. * @returns a callback ref to be passed onto the element responsible for triggering the menu. */ - registerItems: (items: T[]) => (trigger: HTMLElement | null) => void; + registerItems: (items: T[]) => React.RefCallback; }; -export function useContextMenu({ +export function useContextMenu({ Menu, }: { Menu: React.ComponentType<{ @@ -33,7 +33,9 @@ export function useContextMenu({ throw new Error('useContextMenu called outside of the provider'); } - const register = (content: React.ComponentType) => { + const register = ( + content: React.ComponentType + ): React.RefCallback => { function listener(event: MouseEvent) { appendContextMenuContent(event, content); } From 0ad7a63fe23680212cc8be9e00c199a7b144dd2d Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 22 May 2025 12:14:37 +0200 Subject: [PATCH 08/46] fix: use React.RefCallback --- packages/compass-context-menu/src/use-context-menu.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/compass-context-menu/src/use-context-menu.tsx b/packages/compass-context-menu/src/use-context-menu.tsx index a0c97dead9b..fa9e20e7537 100644 --- a/packages/compass-context-menu/src/use-context-menu.tsx +++ b/packages/compass-context-menu/src/use-context-menu.tsx @@ -12,7 +12,7 @@ export type ContextMenuMethods = { * Register the menu items for the context menu. * @returns a callback ref to be passed onto the element responsible for triggering the menu. */ - registerItems: (items: T[]) => (trigger: HTMLElement | null) => void; + registerItems: (items: T[]) => React.RefCallback; }; export function useContextMenu({ @@ -33,7 +33,9 @@ export function useContextMenu({ throw new Error('useContextMenu called outside of the provider'); } - const register = (content: React.ComponentType) => { + const register = ( + content: React.ComponentType + ): React.RefCallback => { function listener(event: MouseEvent) { appendContextMenuContent(event, content); } From aa9f0cf6dc8ecb7f43cff89b4be35dd286cf29db Mon Sep 17 00:00:00 2001 From: gagik Date: Fri, 23 May 2025 12:08:50 +0200 Subject: [PATCH 09/46] fix: switch to item-based organization --- .../src/components/context-menu.spec.tsx | 149 ++++++++++++++++++ .../src/components/context-menu.tsx | 68 ++++---- packages/compass-components/src/index.ts | 5 + .../src/compass-context-menu.tsx | 24 +++ .../src/context-menu-content.ts | 13 +- .../src/context-menu-provider.tsx | 42 +++-- .../compass-context-menu/src/context-menu.tsx | 22 --- packages/compass-context-menu/src/index.ts | 3 +- packages/compass-context-menu/src/types.ts | 9 +- .../src/use-context-menu.spec.tsx | 63 ++++---- .../src/use-context-menu.tsx | 59 ++++--- .../src/components/desktop-welcome-tab.tsx | 73 ++++++--- .../compass/src/app/components/entrypoint.tsx | 6 +- 13 files changed, 381 insertions(+), 155 deletions(-) create mode 100644 packages/compass-components/src/components/context-menu.spec.tsx create mode 100644 packages/compass-context-menu/src/compass-context-menu.tsx delete mode 100644 packages/compass-context-menu/src/context-menu.tsx diff --git a/packages/compass-components/src/components/context-menu.spec.tsx b/packages/compass-components/src/components/context-menu.spec.tsx new file mode 100644 index 00000000000..e3f84335acf --- /dev/null +++ b/packages/compass-components/src/components/context-menu.spec.tsx @@ -0,0 +1,149 @@ +import React from 'react'; +import { render, screen, userEvent } from '@mongodb-js/testing-library-compass'; +import { expect } from 'chai'; +import sinon from 'sinon'; +import { ContextMenuProvider } from '@mongodb-js/compass-context-menu'; +import { useContextMenuItems, ContextMenu } from './context-menu'; +import type { ContextMenuItem } from '@mongodb-js/compass-context-menu'; + +describe('useContextMenuItems', function () { + const TestComponent = ({ items }: { items: ContextMenuItem[] }) => { + const ref = useContextMenuItems(items); + + return ( +
+ Test Component +
+ ); + }; + + describe('when used outside provider', function () { + it('throws an error', function () { + const items = [ + { + label: 'Test Item', + onAction: () => {}, + }, + ]; + + expect(() => { + render(); + }).to.throw('useContextMenu called outside of the provider'); + }); + }); + + describe('with a valid provider', function () { + beforeEach(() => { + // Create the container for the context menu portal + const container = document.createElement('div'); + container.id = 'context-menu-container'; + document.body.appendChild(container); + }); + + afterEach(() => { + // Clean up the container + const container = document.getElementById('context-menu-container'); + if (container) { + document.body.removeChild(container); + } + }); + + it('renders without error', function () { + const items = [ + { + label: 'Test Item', + onAction: () => {}, + }, + ]; + + render( + + + + ); + + expect(screen.getByTestId('test-trigger')).to.exist; + }); + + it('shows context menu with items on right click', function () { + const items = [ + { + label: 'Test Item 1', + onAction: () => {}, + }, + { + label: 'Test Item 2', + onAction: () => {}, + }, + ]; + + render( + + + + ); + + const trigger = screen.getByTestId('test-trigger'); + userEvent.click(trigger, { button: 2 }); + + // The menu items should be rendered + expect(screen.getByTestId('context-menu-item-Test Item 1')).to.exist; + expect(screen.getByTestId('context-menu-item-Test Item 2')).to.exist; + }); + + it('triggers the correct action when menu item is clicked', function () { + const onAction = sinon.spy(); + const items = [ + { + label: 'Test Item 1', + onAction: () => onAction(1), + }, + { + label: 'Test Item 2', + onAction: () => onAction(2), + }, + ]; + + render( + + + + ); + + const trigger = screen.getByTestId('test-trigger'); + userEvent.click(trigger, { button: 2 }); + + const menuItem = screen.getByTestId('context-menu-item-Test Item 2'); + userEvent.click(menuItem); + + expect(onAction).to.have.been.calledOnceWithExactly(2); + }); + + it('renders menu items with separators', function () { + const items = [ + { + label: 'Test Item 1', + onAction: () => {}, + }, + { + label: 'Test Item 2', + onAction: () => {}, + }, + ]; + + render( + + + + ); + + const trigger = screen.getByTestId('test-trigger'); + userEvent.click(trigger, { button: 2 }); + + // Should find both menu items and a separator between them + expect(screen.getByTestId('context-menu-item-Test Item 1')).to.exist; + expect(screen.getByRole('separator')).to.exist; + expect(screen.getByTestId('context-menu-item-Test Item 2')).to.exist; + }); + }); +}); diff --git a/packages/compass-components/src/components/context-menu.tsx b/packages/compass-components/src/components/context-menu.tsx index 9bb9440a217..9790f306772 100644 --- a/packages/compass-components/src/components/context-menu.tsx +++ b/packages/compass-components/src/components/context-menu.tsx @@ -1,45 +1,53 @@ import React from 'react'; -import { css, cx } from '@leafygreen-ui/emotion'; import { Menu, MenuItem, MenuSeparator } from './leafygreen'; import type { ContextMenuItem } from '@mongodb-js/compass-context-menu'; import { useContextMenu } from '@mongodb-js/compass-context-menu'; +import { ContextMenuProvider as ContextMenuProviderBase } from '@mongodb-js/compass-context-menu'; +import type { ContextMenuItemGroup } from '@mongodb-js/compass-context-menu/dist/types'; -const menuStyle = css({ - position: 'fixed', - zIndex: 9999, -}); +export function ContextMenuProvider({ + children, +}: { + children: React.ReactNode; +}) { + return ( + + {children} + + ); +} export type ContextMenuProps = { - items: ContextMenuItem[]; + itemGroups: ContextMenuItemGroup[]; className?: string; 'data-testid'?: string; }; -export function ContextMenu({ - items, - className, - 'data-testid': dataTestId, -}: ContextMenuProps) { +export function ContextMenu({ itemGroups }: ContextMenuProps) { return ( - - {items.map((item, idx) => { - const { label, onAction } = item; - const isLastItem = idx === items.length - 1; - + + {itemGroups.map((itemGroup: ContextMenuItemGroup, groupIndex: number) => { return ( - <> - {!isLastItem && } - { - evt.stopPropagation(); - onAction?.(evt); - }} - > - {label} - - +
+ {itemGroup.items.map((item: ContextMenuItem, itemIndex: number) => { + return ( + { + console.log('clicked', evt); + item.onAction?.(evt); + }} + > + {item.label} {itemIndex} {groupIndex} + + ); + })} + {groupIndex < itemGroups.length - 1 && ( + + )} +
); })} @@ -49,6 +57,6 @@ export function ContextMenu({ export function useContextMenuItems( items: ContextMenuItem[] ): React.RefCallback { - const contextMenu = useContextMenu({ Menu: ContextMenu }); + const contextMenu = useContextMenu(); return contextMenu.registerItems(items); } diff --git a/packages/compass-components/src/index.ts b/packages/compass-components/src/index.ts index 1857adc3c99..7cabba8e671 100644 --- a/packages/compass-components/src/index.ts +++ b/packages/compass-components/src/index.ts @@ -93,6 +93,11 @@ export { ModalHeader } from './components/modals/modal-header'; export { FormModal } from './components/modals/form-modal'; export { InfoModal } from './components/modals/info-modal'; +export { + ContextMenuProvider, + useContextMenuItems, +} from './components/context-menu'; + export type { FileInputBackend, ItemAction, diff --git a/packages/compass-context-menu/src/compass-context-menu.tsx b/packages/compass-context-menu/src/compass-context-menu.tsx new file mode 100644 index 00000000000..113b441a25e --- /dev/null +++ b/packages/compass-context-menu/src/compass-context-menu.tsx @@ -0,0 +1,24 @@ +import React from 'react'; + +type ContextMenuProps = React.PropsWithChildren<{ + position: { + x: number; + y: number; + }; +}>; + +export function ContextMenu({ children, position }: ContextMenuProps) { + console.log('ContextMenu', position); + return ( +
+ {children} +
+ ); +} diff --git a/packages/compass-context-menu/src/context-menu-content.ts b/packages/compass-context-menu/src/context-menu-content.ts index 6856f1fe684..c301983a679 100644 --- a/packages/compass-context-menu/src/context-menu-content.ts +++ b/packages/compass-context-menu/src/context-menu-content.ts @@ -1,23 +1,24 @@ +import type { ContextMenuItemGroup } from './types'; + const CONTEXT_MENUS_SYMBOL = Symbol('context_menus'); export type EnhancedMouseEvent = MouseEvent & { - [CONTEXT_MENUS_SYMBOL]?: React.ComponentType[]; + [CONTEXT_MENUS_SYMBOL]?: ContextMenuItemGroup[]; }; export function getContextMenuContent( event: EnhancedMouseEvent -): React.ComponentType[] { +): ContextMenuItemGroup[] { return event[CONTEXT_MENUS_SYMBOL] ?? []; } export function appendContextMenuContent( event: EnhancedMouseEvent, - content: React.ComponentType + content: ContextMenuItemGroup ) { // Initialize if not already patched - if (event[CONTEXT_MENUS_SYMBOL] === undefined) { - event[CONTEXT_MENUS_SYMBOL] = [content]; - return; + if (!event[CONTEXT_MENUS_SYMBOL]) { + event[CONTEXT_MENUS_SYMBOL] = []; } event[CONTEXT_MENUS_SYMBOL].push(content); } diff --git a/packages/compass-context-menu/src/context-menu-provider.tsx b/packages/compass-context-menu/src/context-menu-provider.tsx index 43d9e893367..621ac7281bc 100644 --- a/packages/compass-context-menu/src/context-menu-provider.tsx +++ b/packages/compass-context-menu/src/context-menu-provider.tsx @@ -5,8 +5,12 @@ import React, { useMemo, createContext, } from 'react'; -import type { ContextMenuContext, MenuState } from './types'; -import { ContextMenu } from './context-menu'; +import type { + ContextMenuContext, + ContextMenuItemGroup, + ContextMenuState, +} from './types'; +import { ContextMenu } from './compass-context-menu'; import type { EnhancedMouseEvent } from './context-menu-content'; import { getContextMenuContent } from './context-menu-content'; @@ -14,20 +18,22 @@ export const Context = createContext(null); export function ContextMenuProvider({ children, + wrapper, }: { children: React.ReactNode; + wrapper: React.ComponentType<{ itemGroups: ContextMenuItemGroup[] }>; }) { - const [menu, setMenu] = useState({ isOpen: false }); + const [menu, setMenu] = useState({ isOpen: false }); const close = useCallback(() => setMenu({ isOpen: false }), [setMenu]); useEffect(() => { function handleContextMenu(event: MouseEvent) { + console.log('handleContextMenu', event); event.preventDefault(); + setMenu({ isOpen: true, - children: getContextMenuContent(event as EnhancedMouseEvent).map( - (Content, index) => - ), + itemGroups: getContextMenuContent(event as EnhancedMouseEvent), position: { // TODO: Fix handling offset while scrolling x: event.clientX, @@ -37,16 +43,20 @@ export function ContextMenuProvider({ } function handleClosingEvent(event: Event) { + console.log('handleClosingEvent', event); if (!event.defaultPrevented) { + console.log('setting menu to false'); setMenu({ isOpen: false }); } } + console.log('adding event listeners'); document.addEventListener('contextmenu', handleContextMenu); - document.addEventListener('click', handleClosingEvent); + window.addEventListener('click', handleClosingEvent); window.addEventListener('resize', handleClosingEvent); return () => { + console.log('removing event listeners'); document.removeEventListener('contextmenu', handleContextMenu); document.removeEventListener('click', handleClosingEvent); window.removeEventListener('resize', handleClosingEvent); @@ -60,12 +70,18 @@ export function ContextMenuProvider({ [close] ); + const Wrapper = wrapper ?? React.Fragment; + return ( - <> - {children} - {menu.isOpen && ( - {menu.children} - )} - + + <> + {children} + {menu.isOpen && ( + + + + )} + + ); } diff --git a/packages/compass-context-menu/src/context-menu.tsx b/packages/compass-context-menu/src/context-menu.tsx deleted file mode 100644 index b053bc4963a..00000000000 --- a/packages/compass-context-menu/src/context-menu.tsx +++ /dev/null @@ -1,22 +0,0 @@ -import { createPortal } from 'react-dom'; -import React from 'react'; - -type ContextMenuProps = React.PropsWithChildren<{ - position: { - x: number; - y: number; - }; -}>; - -export function ContextMenu({ children, position }: ContextMenuProps) { - const container = document.getElementById('context-menu-container'); - if (container === null) { - throw new Error('Expected a container for the context menu in the DOM'); - } - return createPortal( -
- {children} -
, - container - ); -} diff --git a/packages/compass-context-menu/src/index.ts b/packages/compass-context-menu/src/index.ts index d60a97e04b3..f024b578aa9 100644 --- a/packages/compass-context-menu/src/index.ts +++ b/packages/compass-context-menu/src/index.ts @@ -1,2 +1,3 @@ export { useContextMenu } from './use-context-menu'; -export type { ContextMenuItem } from './types'; +export { ContextMenuProvider } from './context-menu-provider'; +export type { ContextMenuItem, ContextMenuItemGroup } from './types'; diff --git a/packages/compass-context-menu/src/types.ts b/packages/compass-context-menu/src/types.ts index f453930dcb3..a0019fa88df 100644 --- a/packages/compass-context-menu/src/types.ts +++ b/packages/compass-context-menu/src/types.ts @@ -1,10 +1,15 @@ -export type MenuState = +export interface ContextMenuItemGroup { + items: ContextMenuItem[]; + originListener: (event: MouseEvent) => void; +} + +export type ContextMenuState = | { isOpen: false; } | { isOpen: true; - children: React.ReactNode; + itemGroups: ContextMenuItemGroup[]; position: { x: number; y: number; diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx index eb857db9aeb..b9ef7b5cd06 100644 --- a/packages/compass-context-menu/src/use-context-menu.spec.tsx +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -4,27 +4,31 @@ import { expect } from 'chai'; import sinon from 'sinon'; import { useContextMenu } from './use-context-menu'; import { ContextMenuProvider } from './context-menu-provider'; -import type { ContextMenuItem } from './types'; +import type { ContextMenuItem, ContextMenuItemGroup } from './types'; describe('useContextMenu', function () { - const TestMenu: React.FC<{ items: ContextMenuItem[] }> = ({ items }) => ( + const TestMenu: React.FC<{ itemGroups: ContextMenuItemGroup[] }> = ({ + itemGroups, + }) => (
- {items.map((item, idx) => ( -
item.onAction?.(event)} - onKeyDown={(event) => { - if (event.key === 'Enter') { - item.onAction?.(event); - } - }} - > - {item.label} -
- ))} + {itemGroups.flatMap((group, groupIdx) => + group.items.map((item, idx) => ( +
item.onAction?.(event)} + onKeyDown={(event) => { + if (event.key === 'Enter') { + item.onAction?.(event); + } + }} + > + {item.label} +
+ )) + )}
); @@ -33,9 +37,9 @@ describe('useContextMenu', function () { onAction, }: { onRegister?: (ref: any) => void; - onAction?: (id) => void; + onAction?: (id: number) => void; }) => { - const contextMenu = useContextMenu({ Menu: TestMenu }); + const contextMenu = useContextMenu(); const items: ContextMenuItem[] = [ { label: 'Test Item', @@ -55,7 +59,6 @@ describe('useContextMenu', function () { ); }; - // Add new test components for nested context menu scenario const ParentComponent = ({ onAction, children, @@ -63,7 +66,7 @@ describe('useContextMenu', function () { onAction?: (id: number) => void; children?: React.ReactNode; }) => { - const contextMenu = useContextMenu({ Menu: TestMenu }); + const contextMenu = useContextMenu(); const parentItems: ContextMenuItem[] = [ { label: 'Parent Item 1', @@ -89,7 +92,7 @@ describe('useContextMenu', function () { }: { onAction?: (id: number) => void; }) => { - const contextMenu = useContextMenu({ Menu: TestMenu }); + const contextMenu = useContextMenu(); const childItems: ContextMenuItem[] = [ { label: 'Child Item 1', @@ -135,7 +138,7 @@ describe('useContextMenu', function () { it('renders without error', function () { render( - + ); @@ -147,7 +150,7 @@ describe('useContextMenu', function () { const onRegister = sinon.spy(); render( - + ); @@ -158,7 +161,7 @@ describe('useContextMenu', function () { it('shows context menu on right click', function () { render( - + ); @@ -173,7 +176,7 @@ describe('useContextMenu', function () { describe('with nested context menus', function () { it('shows only parent items when right clicking parent area', function () { render( - + ); @@ -192,7 +195,7 @@ describe('useContextMenu', function () { it('shows both parent and child items when right clicking child area', function () { render( - + @@ -214,7 +217,7 @@ describe('useContextMenu', function () { const childOnAction = sinon.spy(); render( - + @@ -237,7 +240,7 @@ describe('useContextMenu', function () { const childOnAction = sinon.spy(); render( - + diff --git a/packages/compass-context-menu/src/use-context-menu.tsx b/packages/compass-context-menu/src/use-context-menu.tsx index fa9e20e7537..02b94383faa 100644 --- a/packages/compass-context-menu/src/use-context-menu.tsx +++ b/packages/compass-context-menu/src/use-context-menu.tsx @@ -1,9 +1,10 @@ -import React, { useContext, useMemo, useRef } from 'react'; +import type { RefCallback } from 'react'; +import { useContext, useMemo, useRef } from 'react'; import { Context } from './context-menu-provider'; import { appendContextMenuContent } from './context-menu-content'; import type { ContextMenuItem } from './types'; -export type ContextMenuMethods = { +export type ContextMenuMethods = { /** * Close the context menu. */ @@ -12,17 +13,10 @@ export type ContextMenuMethods = { * Register the menu items for the context menu. * @returns a callback ref to be passed onto the element responsible for triggering the menu. */ - registerItems: (items: T[]) => React.RefCallback; + registerItems: (items: ContextMenuItem[]) => RefCallback; }; -export function useContextMenu({ - Menu, -}: { - Menu: React.ComponentType<{ - items: T[]; - }>; -}): ContextMenuMethods { - // Get the close function from the ContextProvider +export function useContextMenu(): ContextMenuMethods { const context = useContext(Context); const previous = useRef void]>( null @@ -33,32 +27,33 @@ export function useContextMenu({ throw new Error('useContextMenu called outside of the provider'); } - const register = ( - content: React.ComponentType - ): React.RefCallback => { - function listener(event: MouseEvent) { - appendContextMenuContent(event, content); - } - return (trigger: HTMLElement | null) => { - if (previous.current) { - const [previousTrigger, previousListener] = previous.current; - previousTrigger.removeEventListener('contextmenu', previousListener); - } - if (trigger) { - trigger.addEventListener('contextmenu', listener); - previous.current = [trigger, listener]; - } - }; - }; - return { close: context.close.bind(context), /** * @returns a callback ref, passed onto the element responsible for triggering the menu. */ - registerItems(items: T[]) { - return register(() => ); + registerItems(items: ContextMenuItem[]) { + function listener(event: MouseEvent): void { + appendContextMenuContent(event, { + items, + originListener: listener, + }); + } + + return (trigger: HTMLElement | null) => { + if (previous.current) { + const [previousTrigger, previousListener] = previous.current; + previousTrigger.removeEventListener( + 'contextmenu', + previousListener + ); + } + if (trigger) { + trigger.addEventListener('contextmenu', listener); + previous.current = [trigger, listener]; + } + }; }, }; - }, [context, Menu]); + }, [context]); } diff --git a/packages/compass-welcome/src/components/desktop-welcome-tab.tsx b/packages/compass-welcome/src/components/desktop-welcome-tab.tsx index 768a73e0b92..9c39d1005f7 100644 --- a/packages/compass-welcome/src/components/desktop-welcome-tab.tsx +++ b/packages/compass-welcome/src/components/desktop-welcome-tab.tsx @@ -14,6 +14,7 @@ import { cx, useDarkMode, Icon, + useContextMenuItems, } from '@mongodb-js/compass-components'; import { useTelemetry } from '@mongodb-js/compass-telemetry/provider'; import { useConnectionActions } from '@mongodb-js/compass-connections/provider'; @@ -66,11 +67,22 @@ const createClusterButtonLightModeStyles = css({ }); function AtlasHelpSection(): React.ReactElement { - const track = useTelemetry(); const darkMode = useDarkMode(); + const track = useTelemetry(); + const contextRef = useContextMenuItems([ + { + label: '1', + onAction: () => track('Atlas Link Clicked', { screen: 'connect' }), + }, + { + label: '2', + onAction: () => track('Atlas Link Clicked', { screen: 'connect' }), + }, + ]); return (
-
- -
+
); } +function TestClusterButton() { + const track = useTelemetry(); + const darkMode = useDarkMode(); + const contextRef = useContextMenuItems([ + { + label: '123', + onAction: () => track('Atlas Link Clicked', { screen: 'connect' }), + }, + ]); + return ( +
+ +
+ ); +} const welcomeTabStyles = css({ display: 'flex', alignItems: 'center', @@ -125,9 +151,20 @@ export default function DesktopWelcomeTab() { const enableCreatingNewConnections = usePreference( 'enableCreatingNewConnections' ); + const track = useTelemetry(); + const contextRef = useContextMenuItems([ + { + label: '4', + onAction: () => track('Atlas Link Clicked', { screen: 'connect' }), + }, + { + label: '5', + onAction: () => track('Atlas Link Clicked', { screen: 'connect' }), + }, + ]); return ( -
+

Welcome to MongoDB Compass

diff --git a/packages/compass/src/app/components/entrypoint.tsx b/packages/compass/src/app/components/entrypoint.tsx index 493580e6564..1bbcb163d43 100644 --- a/packages/compass/src/app/components/entrypoint.tsx +++ b/packages/compass/src/app/components/entrypoint.tsx @@ -30,6 +30,8 @@ import { createIpcSendTrack, } from '@mongodb-js/compass-telemetry'; import { DataModelStorageServiceProviderElectron } from '@mongodb-js/compass-data-modeling/renderer'; +import { Context } from '@segment/analytics-node'; +import { ContextMenuProvider } from '@mongodb-js/compass-components'; const WithPreferencesAndLoggerProviders: React.FC = ({ children }) => { const loggerProviderValue = useRef({ @@ -101,7 +103,9 @@ export const CompassElectron = (props: React.ComponentProps) => { - + + + From 58df56aa142486422235df04077c702bc7053d0a Mon Sep 17 00:00:00 2001 From: gagik Date: Fri, 23 May 2025 13:41:54 +0200 Subject: [PATCH 10/46] fix: cleanup and switch to menu prop --- .../src/components/context-menu.spec.tsx | 120 +++++++++++------- .../src/components/context-menu.tsx | 96 +++++++++----- .../src/compass-context-menu.tsx | 24 ---- .../src/context-menu-provider.tsx | 61 +++++---- packages/compass-context-menu/src/index.ts | 6 +- packages/compass-context-menu/src/types.ts | 24 ++-- .../src/use-context-menu.spec.tsx | 8 +- .../src/components/desktop-welcome-tab.tsx | 6 +- 8 files changed, 186 insertions(+), 159 deletions(-) delete mode 100644 packages/compass-context-menu/src/compass-context-menu.tsx diff --git a/packages/compass-components/src/components/context-menu.spec.tsx b/packages/compass-components/src/components/context-menu.spec.tsx index e3f84335acf..14177575d9d 100644 --- a/packages/compass-components/src/components/context-menu.spec.tsx +++ b/packages/compass-components/src/components/context-menu.spec.tsx @@ -7,12 +7,23 @@ import { useContextMenuItems, ContextMenu } from './context-menu'; import type { ContextMenuItem } from '@mongodb-js/compass-context-menu'; describe('useContextMenuItems', function () { - const TestComponent = ({ items }: { items: ContextMenuItem[] }) => { + const menuTestTriggerId = 'test-trigger'; + + const TestComponent = ({ + items, + children, + 'data-testid': dataTestId = menuTestTriggerId, + }: { + items: ContextMenuItem[]; + children?: React.ReactNode; + 'data-testid'?: string; + }) => { const ref = useContextMenuItems(items); return ( -
+
Test Component + {children}
); }; @@ -33,21 +44,6 @@ describe('useContextMenuItems', function () { }); describe('with a valid provider', function () { - beforeEach(() => { - // Create the container for the context menu portal - const container = document.createElement('div'); - container.id = 'context-menu-container'; - document.body.appendChild(container); - }); - - afterEach(() => { - // Clean up the container - const container = document.getElementById('context-menu-container'); - if (container) { - document.body.removeChild(container); - } - }); - it('renders without error', function () { const items = [ { @@ -62,7 +58,7 @@ describe('useContextMenuItems', function () { ); - expect(screen.getByTestId('test-trigger')).to.exist; + expect(screen.getByTestId(menuTestTriggerId)).to.exist; }); it('shows context menu with items on right click', function () { @@ -83,12 +79,12 @@ describe('useContextMenuItems', function () { ); - const trigger = screen.getByTestId('test-trigger'); + const trigger = screen.getByTestId(menuTestTriggerId); userEvent.click(trigger, { button: 2 }); // The menu items should be rendered - expect(screen.getByTestId('context-menu-item-Test Item 1')).to.exist; - expect(screen.getByTestId('context-menu-item-Test Item 2')).to.exist; + expect(screen.getByTestId('menu-group-0-item-0')).to.exist; + expect(screen.getByTestId('menu-group-0-item-1')).to.exist; }); it('triggers the correct action when menu item is clicked', function () { @@ -110,40 +106,68 @@ describe('useContextMenuItems', function () { ); - const trigger = screen.getByTestId('test-trigger'); + const trigger = screen.getByTestId(menuTestTriggerId); userEvent.click(trigger, { button: 2 }); - const menuItem = screen.getByTestId('context-menu-item-Test Item 2'); + const menuItem = screen.getByTestId('menu-group-0-item-1'); userEvent.click(menuItem); expect(onAction).to.have.been.calledOnceWithExactly(2); }); - it('renders menu items with separators', function () { - const items = [ - { - label: 'Test Item 1', - onAction: () => {}, - }, - { - label: 'Test Item 2', - onAction: () => {}, - }, - ]; - - render( - - - - ); - - const trigger = screen.getByTestId('test-trigger'); - userEvent.click(trigger, { button: 2 }); - - // Should find both menu items and a separator between them - expect(screen.getByTestId('context-menu-item-Test Item 1')).to.exist; - expect(screen.getByRole('separator')).to.exist; - expect(screen.getByTestId('context-menu-item-Test Item 2')).to.exist; + describe('with nested components', function () { + const childTriggerId = 'child-trigger'; + + beforeEach(function () { + const items = [ + { + label: 'Test Item 1', + onAction: () => {}, + }, + { + label: 'Test Item 2', + onAction: () => {}, + }, + ]; + + const childItems = [ + { + label: 'Child Item 1', + onAction: () => {}, + }, + ]; + + render( + + + + + + ); + }); + + it('renders menu items with separators', function () { + const trigger = screen.getByTestId(childTriggerId); + userEvent.click(trigger, { button: 2 }); + + // Should find the menu item and the separator + expect(screen.getByTestId('menu-group-0').children.length).to.equal(2); + expect( + screen.getByTestId('menu-group-0').children.item(0)?.textContent + ).to.equal('Child Item 1'); + + expect(screen.getByTestId('menu-group-0-separator')).to.exist; + + expect(screen.getByTestId('menu-group-1').children.length).to.equal(2); + expect( + screen.getByTestId('menu-group-1').children.item(0)?.textContent + ).to.equal('Test Item 1'); + expect( + screen.getByTestId('menu-group-1').children.item(1)?.textContent + ).to.equal('Test Item 2'); + + expect(screen.queryByTestId('menu-group-1-separator')).not.to.exist; + }); }); }); }); diff --git a/packages/compass-components/src/components/context-menu.tsx b/packages/compass-components/src/components/context-menu.tsx index 9790f306772..c3fdae09d68 100644 --- a/packages/compass-components/src/components/context-menu.tsx +++ b/packages/compass-components/src/components/context-menu.tsx @@ -1,9 +1,12 @@ -import React from 'react'; +import React, { useEffect } from 'react'; import { Menu, MenuItem, MenuSeparator } from './leafygreen'; import type { ContextMenuItem } from '@mongodb-js/compass-context-menu'; import { useContextMenu } from '@mongodb-js/compass-context-menu'; import { ContextMenuProvider as ContextMenuProviderBase } from '@mongodb-js/compass-context-menu'; -import type { ContextMenuItemGroup } from '@mongodb-js/compass-context-menu/dist/types'; +import type { + ContextMenuItemGroup, + ContextMenuWrapperProps, +} from '@mongodb-js/compass-context-menu/dist/types'; export function ContextMenuProvider({ children, @@ -17,40 +20,65 @@ export function ContextMenuProvider({ ); } -export type ContextMenuProps = { - itemGroups: ContextMenuItemGroup[]; - className?: string; - 'data-testid'?: string; -}; +export function ContextMenu({ menu }: ContextMenuWrapperProps) { + const position = menu.position; + const itemGroups = menu.itemGroups; + + useEffect(() => { + if (!menu.isOpen) { + menu.close(); + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [menu.isOpen]); -export function ContextMenu({ itemGroups }: ContextMenuProps) { return ( - - {itemGroups.map((itemGroup: ContextMenuItemGroup, groupIndex: number) => { - return ( -
- {itemGroup.items.map((item: ContextMenuItem, itemIndex: number) => { - return ( - { - console.log('clicked', evt); - item.onAction?.(evt); - }} - > - {item.label} {itemIndex} {groupIndex} - - ); - })} - {groupIndex < itemGroups.length - 1 && ( - - )} -
- ); - })} - +
+ + {itemGroups.map( + (itemGroup: ContextMenuItemGroup, groupIndex: number) => { + return ( +
+ {itemGroup.items.map( + (item: ContextMenuItem, itemIndex: number) => { + return ( + { + item.onAction?.(evt); + menu.close(); + }} + > + {item.label} + + ); + } + )} + {groupIndex < itemGroups.length - 1 && ( +
+ +
+ )} +
+ ); + } + )} + +
); } diff --git a/packages/compass-context-menu/src/compass-context-menu.tsx b/packages/compass-context-menu/src/compass-context-menu.tsx deleted file mode 100644 index 113b441a25e..00000000000 --- a/packages/compass-context-menu/src/compass-context-menu.tsx +++ /dev/null @@ -1,24 +0,0 @@ -import React from 'react'; - -type ContextMenuProps = React.PropsWithChildren<{ - position: { - x: number; - y: number; - }; -}>; - -export function ContextMenu({ children, position }: ContextMenuProps) { - console.log('ContextMenu', position); - return ( -
- {children} -
- ); -} diff --git a/packages/compass-context-menu/src/context-menu-provider.tsx b/packages/compass-context-menu/src/context-menu-provider.tsx index 621ac7281bc..5dd1cba2cff 100644 --- a/packages/compass-context-menu/src/context-menu-provider.tsx +++ b/packages/compass-context-menu/src/context-menu-provider.tsx @@ -5,12 +5,7 @@ import React, { useMemo, createContext, } from 'react'; -import type { - ContextMenuContext, - ContextMenuItemGroup, - ContextMenuState, -} from './types'; -import { ContextMenu } from './compass-context-menu'; +import type { ContextMenuContext, ContextMenuState } from './types'; import type { EnhancedMouseEvent } from './context-menu-content'; import { getContextMenuContent } from './context-menu-content'; @@ -21,19 +16,39 @@ export function ContextMenuProvider({ wrapper, }: { children: React.ReactNode; - wrapper: React.ComponentType<{ itemGroups: ContextMenuItemGroup[] }>; + wrapper: React.ComponentType<{ + menu: ContextMenuState & { close: () => void }; + }>; }) { - const [menu, setMenu] = useState({ isOpen: false }); - const close = useCallback(() => setMenu({ isOpen: false }), [setMenu]); + const [menu, setMenu] = useState({ + isOpen: false, + itemGroups: [], + position: { x: 0, y: 0 }, + }); + const close = useCallback(() => setMenu({ ...menu, isOpen: false }), [menu]); + + const handleClosingEvent = useCallback( + (event: Event) => { + if (!event.defaultPrevented) { + setMenu({ ...menu, isOpen: false }); + } + }, + [menu] + ); useEffect(() => { function handleContextMenu(event: MouseEvent) { - console.log('handleContextMenu', event); event.preventDefault(); + const itemGroups = getContextMenuContent(event as EnhancedMouseEvent); + + if (itemGroups.length === 0) { + return; + } + setMenu({ isOpen: true, - itemGroups: getContextMenuContent(event as EnhancedMouseEvent), + itemGroups, position: { // TODO: Fix handling offset while scrolling x: event.clientX, @@ -42,26 +57,14 @@ export function ContextMenuProvider({ }); } - function handleClosingEvent(event: Event) { - console.log('handleClosingEvent', event); - if (!event.defaultPrevented) { - console.log('setting menu to false'); - setMenu({ isOpen: false }); - } - } - - console.log('adding event listeners'); document.addEventListener('contextmenu', handleContextMenu); - window.addEventListener('click', handleClosingEvent); window.addEventListener('resize', handleClosingEvent); return () => { - console.log('removing event listeners'); document.removeEventListener('contextmenu', handleContextMenu); - document.removeEventListener('click', handleClosingEvent); window.removeEventListener('resize', handleClosingEvent); }; - }, [setMenu]); + }, [handleClosingEvent]); const value = useMemo( () => ({ @@ -74,14 +77,8 @@ export function ContextMenuProvider({ return ( - <> - {children} - {menu.isOpen && ( - - - - )} - + {children} + ); } diff --git a/packages/compass-context-menu/src/index.ts b/packages/compass-context-menu/src/index.ts index f024b578aa9..75d933ef767 100644 --- a/packages/compass-context-menu/src/index.ts +++ b/packages/compass-context-menu/src/index.ts @@ -1,3 +1,7 @@ export { useContextMenu } from './use-context-menu'; export { ContextMenuProvider } from './context-menu-provider'; -export type { ContextMenuItem, ContextMenuItemGroup } from './types'; +export type { + ContextMenuItem, + ContextMenuItemGroup, + ContextMenuWrapperProps, +} from './types'; diff --git a/packages/compass-context-menu/src/types.ts b/packages/compass-context-menu/src/types.ts index a0019fa88df..91e8d65cdcc 100644 --- a/packages/compass-context-menu/src/types.ts +++ b/packages/compass-context-menu/src/types.ts @@ -3,18 +3,18 @@ export interface ContextMenuItemGroup { originListener: (event: MouseEvent) => void; } -export type ContextMenuState = - | { - isOpen: false; - } - | { - isOpen: true; - itemGroups: ContextMenuItemGroup[]; - position: { - x: number; - y: number; - }; - }; +export type ContextMenuState = { + isOpen: boolean; + itemGroups: ContextMenuItemGroup[]; + position: { + x: number; + y: number; + }; +}; + +export type ContextMenuWrapperProps = { + menu: ContextMenuState & { close: () => void }; +}; export type ContextMenuContext = { close(): void; diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx index b9ef7b5cd06..cfe0bfc7ef4 100644 --- a/packages/compass-context-menu/src/use-context-menu.spec.tsx +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -4,14 +4,12 @@ import { expect } from 'chai'; import sinon from 'sinon'; import { useContextMenu } from './use-context-menu'; import { ContextMenuProvider } from './context-menu-provider'; -import type { ContextMenuItem, ContextMenuItemGroup } from './types'; +import type { ContextMenuItem, ContextMenuWrapperProps } from './types'; describe('useContextMenu', function () { - const TestMenu: React.FC<{ itemGroups: ContextMenuItemGroup[] }> = ({ - itemGroups, - }) => ( + const TestMenu: React.FC = ({ menu }) => (
- {itemGroups.flatMap((group, groupIdx) => + {menu.itemGroups.flatMap((group, groupIdx) => group.items.map((item, idx) => (
track('Atlas Link Clicked', { screen: 'connect' }), + onAction: () => console.log('Atlas Link Clicked', { screen: 'connect' }), }, { label: '2', - onAction: () => track('Atlas Link Clicked', { screen: 'connect' }), + onAction: () => console.log('Atlas Link Clicked', { screen: 'connect' }), }, ]); @@ -111,7 +111,7 @@ function TestClusterButton() { const contextRef = useContextMenuItems([ { label: '123', - onAction: () => track('Atlas Link Clicked', { screen: 'connect' }), + onAction: () => console.log('Atlas Link Clicked', { screen: 'connect' }), }, ]); return ( From a54c7384b434ccfce970eacd5e32db7597d45555 Mon Sep 17 00:00:00 2001 From: gagik Date: Fri, 23 May 2025 13:50:50 +0200 Subject: [PATCH 11/46] refactor: use item groups instead of React elements, use wrapper, keep menu state consistent The refactor is meant to make the Leafygreen integration more straightforward: 1. We stick to item groups and instead have a single wrapper to handle any rendering differences between groups. This allows the wrapper to always have context of all items when rendering which is useful when inserting menu seperators in Leafygreen. Also encourages consistent UI (while allowing per-case customization if needed at wrapper-level). We could introduce itemWrappers instead of itemGroups but having one wrapper handling all seems cleaner to me. 2. More of the responsibility is moved to a parent wrapper component that will house the context menu. This allows us to standardize the right click menu and make better use of Leafygreen's menu component including its click handling (which has been removed from the context menu library). 3. Menu state (i.e. position) is now preserved even closed; this is useful for leafygreen's menu to animate in the same position instead of losing the position all together. --- .../src/context-menu-content.ts | 13 ++-- .../src/context-menu-provider.tsx | 57 ++++++++++------- packages/compass-context-menu/src/index.ts | 7 ++- packages/compass-context-menu/src/types.ts | 29 +++++---- .../src/use-context-menu.spec.tsx | 61 ++++++++++--------- .../src/use-context-menu.tsx | 59 +++++++++--------- 6 files changed, 124 insertions(+), 102 deletions(-) diff --git a/packages/compass-context-menu/src/context-menu-content.ts b/packages/compass-context-menu/src/context-menu-content.ts index 6856f1fe684..c301983a679 100644 --- a/packages/compass-context-menu/src/context-menu-content.ts +++ b/packages/compass-context-menu/src/context-menu-content.ts @@ -1,23 +1,24 @@ +import type { ContextMenuItemGroup } from './types'; + const CONTEXT_MENUS_SYMBOL = Symbol('context_menus'); export type EnhancedMouseEvent = MouseEvent & { - [CONTEXT_MENUS_SYMBOL]?: React.ComponentType[]; + [CONTEXT_MENUS_SYMBOL]?: ContextMenuItemGroup[]; }; export function getContextMenuContent( event: EnhancedMouseEvent -): React.ComponentType[] { +): ContextMenuItemGroup[] { return event[CONTEXT_MENUS_SYMBOL] ?? []; } export function appendContextMenuContent( event: EnhancedMouseEvent, - content: React.ComponentType + content: ContextMenuItemGroup ) { // Initialize if not already patched - if (event[CONTEXT_MENUS_SYMBOL] === undefined) { - event[CONTEXT_MENUS_SYMBOL] = [content]; - return; + if (!event[CONTEXT_MENUS_SYMBOL]) { + event[CONTEXT_MENUS_SYMBOL] = []; } event[CONTEXT_MENUS_SYMBOL].push(content); } diff --git a/packages/compass-context-menu/src/context-menu-provider.tsx b/packages/compass-context-menu/src/context-menu-provider.tsx index 43d9e893367..5dd1cba2cff 100644 --- a/packages/compass-context-menu/src/context-menu-provider.tsx +++ b/packages/compass-context-menu/src/context-menu-provider.tsx @@ -5,8 +5,7 @@ import React, { useMemo, createContext, } from 'react'; -import type { ContextMenuContext, MenuState } from './types'; -import { ContextMenu } from './context-menu'; +import type { ContextMenuContext, ContextMenuState } from './types'; import type { EnhancedMouseEvent } from './context-menu-content'; import { getContextMenuContent } from './context-menu-content'; @@ -14,20 +13,42 @@ export const Context = createContext(null); export function ContextMenuProvider({ children, + wrapper, }: { children: React.ReactNode; + wrapper: React.ComponentType<{ + menu: ContextMenuState & { close: () => void }; + }>; }) { - const [menu, setMenu] = useState({ isOpen: false }); - const close = useCallback(() => setMenu({ isOpen: false }), [setMenu]); + const [menu, setMenu] = useState({ + isOpen: false, + itemGroups: [], + position: { x: 0, y: 0 }, + }); + const close = useCallback(() => setMenu({ ...menu, isOpen: false }), [menu]); + + const handleClosingEvent = useCallback( + (event: Event) => { + if (!event.defaultPrevented) { + setMenu({ ...menu, isOpen: false }); + } + }, + [menu] + ); useEffect(() => { function handleContextMenu(event: MouseEvent) { event.preventDefault(); + + const itemGroups = getContextMenuContent(event as EnhancedMouseEvent); + + if (itemGroups.length === 0) { + return; + } + setMenu({ isOpen: true, - children: getContextMenuContent(event as EnhancedMouseEvent).map( - (Content, index) => - ), + itemGroups, position: { // TODO: Fix handling offset while scrolling x: event.clientX, @@ -36,22 +57,14 @@ export function ContextMenuProvider({ }); } - function handleClosingEvent(event: Event) { - if (!event.defaultPrevented) { - setMenu({ isOpen: false }); - } - } - document.addEventListener('contextmenu', handleContextMenu); - document.addEventListener('click', handleClosingEvent); window.addEventListener('resize', handleClosingEvent); return () => { document.removeEventListener('contextmenu', handleContextMenu); - document.removeEventListener('click', handleClosingEvent); window.removeEventListener('resize', handleClosingEvent); }; - }, [setMenu]); + }, [handleClosingEvent]); const value = useMemo( () => ({ @@ -60,12 +73,12 @@ export function ContextMenuProvider({ [close] ); + const Wrapper = wrapper ?? React.Fragment; + return ( - <> - {children} - {menu.isOpen && ( - {menu.children} - )} - + + {children} + + ); } diff --git a/packages/compass-context-menu/src/index.ts b/packages/compass-context-menu/src/index.ts index d60a97e04b3..75d933ef767 100644 --- a/packages/compass-context-menu/src/index.ts +++ b/packages/compass-context-menu/src/index.ts @@ -1,2 +1,7 @@ export { useContextMenu } from './use-context-menu'; -export type { ContextMenuItem } from './types'; +export { ContextMenuProvider } from './context-menu-provider'; +export type { + ContextMenuItem, + ContextMenuItemGroup, + ContextMenuWrapperProps, +} from './types'; diff --git a/packages/compass-context-menu/src/types.ts b/packages/compass-context-menu/src/types.ts index f453930dcb3..91e8d65cdcc 100644 --- a/packages/compass-context-menu/src/types.ts +++ b/packages/compass-context-menu/src/types.ts @@ -1,15 +1,20 @@ -export type MenuState = - | { - isOpen: false; - } - | { - isOpen: true; - children: React.ReactNode; - position: { - x: number; - y: number; - }; - }; +export interface ContextMenuItemGroup { + items: ContextMenuItem[]; + originListener: (event: MouseEvent) => void; +} + +export type ContextMenuState = { + isOpen: boolean; + itemGroups: ContextMenuItemGroup[]; + position: { + x: number; + y: number; + }; +}; + +export type ContextMenuWrapperProps = { + menu: ContextMenuState & { close: () => void }; +}; export type ContextMenuContext = { close(): void; diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx index eb857db9aeb..cfe0bfc7ef4 100644 --- a/packages/compass-context-menu/src/use-context-menu.spec.tsx +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -4,27 +4,29 @@ import { expect } from 'chai'; import sinon from 'sinon'; import { useContextMenu } from './use-context-menu'; import { ContextMenuProvider } from './context-menu-provider'; -import type { ContextMenuItem } from './types'; +import type { ContextMenuItem, ContextMenuWrapperProps } from './types'; describe('useContextMenu', function () { - const TestMenu: React.FC<{ items: ContextMenuItem[] }> = ({ items }) => ( + const TestMenu: React.FC = ({ menu }) => (
- {items.map((item, idx) => ( -
item.onAction?.(event)} - onKeyDown={(event) => { - if (event.key === 'Enter') { - item.onAction?.(event); - } - }} - > - {item.label} -
- ))} + {menu.itemGroups.flatMap((group, groupIdx) => + group.items.map((item, idx) => ( +
item.onAction?.(event)} + onKeyDown={(event) => { + if (event.key === 'Enter') { + item.onAction?.(event); + } + }} + > + {item.label} +
+ )) + )}
); @@ -33,9 +35,9 @@ describe('useContextMenu', function () { onAction, }: { onRegister?: (ref: any) => void; - onAction?: (id) => void; + onAction?: (id: number) => void; }) => { - const contextMenu = useContextMenu({ Menu: TestMenu }); + const contextMenu = useContextMenu(); const items: ContextMenuItem[] = [ { label: 'Test Item', @@ -55,7 +57,6 @@ describe('useContextMenu', function () { ); }; - // Add new test components for nested context menu scenario const ParentComponent = ({ onAction, children, @@ -63,7 +64,7 @@ describe('useContextMenu', function () { onAction?: (id: number) => void; children?: React.ReactNode; }) => { - const contextMenu = useContextMenu({ Menu: TestMenu }); + const contextMenu = useContextMenu(); const parentItems: ContextMenuItem[] = [ { label: 'Parent Item 1', @@ -89,7 +90,7 @@ describe('useContextMenu', function () { }: { onAction?: (id: number) => void; }) => { - const contextMenu = useContextMenu({ Menu: TestMenu }); + const contextMenu = useContextMenu(); const childItems: ContextMenuItem[] = [ { label: 'Child Item 1', @@ -135,7 +136,7 @@ describe('useContextMenu', function () { it('renders without error', function () { render( - + ); @@ -147,7 +148,7 @@ describe('useContextMenu', function () { const onRegister = sinon.spy(); render( - + ); @@ -158,7 +159,7 @@ describe('useContextMenu', function () { it('shows context menu on right click', function () { render( - + ); @@ -173,7 +174,7 @@ describe('useContextMenu', function () { describe('with nested context menus', function () { it('shows only parent items when right clicking parent area', function () { render( - + ); @@ -192,7 +193,7 @@ describe('useContextMenu', function () { it('shows both parent and child items when right clicking child area', function () { render( - + @@ -214,7 +215,7 @@ describe('useContextMenu', function () { const childOnAction = sinon.spy(); render( - + @@ -237,7 +238,7 @@ describe('useContextMenu', function () { const childOnAction = sinon.spy(); render( - + diff --git a/packages/compass-context-menu/src/use-context-menu.tsx b/packages/compass-context-menu/src/use-context-menu.tsx index fa9e20e7537..a60aeba4c69 100644 --- a/packages/compass-context-menu/src/use-context-menu.tsx +++ b/packages/compass-context-menu/src/use-context-menu.tsx @@ -1,4 +1,5 @@ -import React, { useContext, useMemo, useRef } from 'react'; +import type { RefCallback } from 'react'; +import { useContext, useMemo, useRef } from 'react'; import { Context } from './context-menu-provider'; import { appendContextMenuContent } from './context-menu-content'; import type { ContextMenuItem } from './types'; @@ -12,17 +13,12 @@ export type ContextMenuMethods = { * Register the menu items for the context menu. * @returns a callback ref to be passed onto the element responsible for triggering the menu. */ - registerItems: (items: T[]) => React.RefCallback; + registerItems: (items: T[]) => RefCallback; }; -export function useContextMenu({ - Menu, -}: { - Menu: React.ComponentType<{ - items: T[]; - }>; -}): ContextMenuMethods { - // Get the close function from the ContextProvider +export function useContextMenu< + T extends ContextMenuItem = ContextMenuItem +>(): ContextMenuMethods { const context = useContext(Context); const previous = useRef void]>( null @@ -33,32 +29,33 @@ export function useContextMenu({ throw new Error('useContextMenu called outside of the provider'); } - const register = ( - content: React.ComponentType - ): React.RefCallback => { - function listener(event: MouseEvent) { - appendContextMenuContent(event, content); - } - return (trigger: HTMLElement | null) => { - if (previous.current) { - const [previousTrigger, previousListener] = previous.current; - previousTrigger.removeEventListener('contextmenu', previousListener); - } - if (trigger) { - trigger.addEventListener('contextmenu', listener); - previous.current = [trigger, listener]; - } - }; - }; - return { close: context.close.bind(context), /** * @returns a callback ref, passed onto the element responsible for triggering the menu. */ - registerItems(items: T[]) { - return register(() => ); + registerItems(items: ContextMenuItem[]) { + function listener(event: MouseEvent): void { + appendContextMenuContent(event, { + items, + originListener: listener, + }); + } + + return (trigger: HTMLElement | null) => { + if (previous.current) { + const [previousTrigger, previousListener] = previous.current; + previousTrigger.removeEventListener( + 'contextmenu', + previousListener + ); + } + if (trigger) { + trigger.addEventListener('contextmenu', listener); + previous.current = [trigger, listener]; + } + }; }, }; - }, [context, Menu]); + }, [context]); } From 56d11b60c1c6136466a7f14a3508d01789ae86bb Mon Sep 17 00:00:00 2001 From: gagik Date: Fri, 23 May 2025 15:17:47 +0200 Subject: [PATCH 12/46] fix: revert test environment --- .../src/components/desktop-welcome-tab.tsx | 73 +++++-------------- 1 file changed, 18 insertions(+), 55 deletions(-) diff --git a/packages/compass-welcome/src/components/desktop-welcome-tab.tsx b/packages/compass-welcome/src/components/desktop-welcome-tab.tsx index 59eaeba9da7..768a73e0b92 100644 --- a/packages/compass-welcome/src/components/desktop-welcome-tab.tsx +++ b/packages/compass-welcome/src/components/desktop-welcome-tab.tsx @@ -14,7 +14,6 @@ import { cx, useDarkMode, Icon, - useContextMenuItems, } from '@mongodb-js/compass-components'; import { useTelemetry } from '@mongodb-js/compass-telemetry/provider'; import { useConnectionActions } from '@mongodb-js/compass-connections/provider'; @@ -67,22 +66,11 @@ const createClusterButtonLightModeStyles = css({ }); function AtlasHelpSection(): React.ReactElement { - const darkMode = useDarkMode(); const track = useTelemetry(); - const contextRef = useContextMenuItems([ - { - label: '1', - onAction: () => console.log('Atlas Link Clicked', { screen: 'connect' }), - }, - { - label: '2', - onAction: () => console.log('Atlas Link Clicked', { screen: 'connect' }), - }, - ]); + const darkMode = useDarkMode(); return (
- +
+ +
); } -function TestClusterButton() { - const track = useTelemetry(); - const darkMode = useDarkMode(); - const contextRef = useContextMenuItems([ - { - label: '123', - onAction: () => console.log('Atlas Link Clicked', { screen: 'connect' }), - }, - ]); - return ( -
- -
- ); -} const welcomeTabStyles = css({ display: 'flex', alignItems: 'center', @@ -151,20 +125,9 @@ export default function DesktopWelcomeTab() { const enableCreatingNewConnections = usePreference( 'enableCreatingNewConnections' ); - const track = useTelemetry(); - const contextRef = useContextMenuItems([ - { - label: '4', - onAction: () => track('Atlas Link Clicked', { screen: 'connect' }), - }, - { - label: '5', - onAction: () => track('Atlas Link Clicked', { screen: 'connect' }), - }, - ]); return ( -
+

Welcome to MongoDB Compass

From 743f0688320a1ad0c87782ac828f24b8c96d1284 Mon Sep 17 00:00:00 2001 From: gagik Date: Fri, 23 May 2025 16:54:04 +0200 Subject: [PATCH 13/46] fix: justify start to make it prefer right way popups and remove comment --- .../compass-components/src/components/context-menu.tsx | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/compass-components/src/components/context-menu.tsx b/packages/compass-components/src/components/context-menu.tsx index c3fdae09d68..c78519e399c 100644 --- a/packages/compass-components/src/components/context-menu.tsx +++ b/packages/compass-components/src/components/context-menu.tsx @@ -28,8 +28,7 @@ export function ContextMenu({ menu }: ContextMenuWrapperProps) { if (!menu.isOpen) { menu.close(); } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [menu.isOpen]); + }, [menu, menu.isOpen]); return (
- + {itemGroups.map( (itemGroup: ContextMenuItemGroup, groupIndex: number) => { return ( From ab14b78be3f20211d9f189789d3906afd2dd908a Mon Sep 17 00:00:00 2001 From: gagik Date: Fri, 23 May 2025 17:19:26 +0200 Subject: [PATCH 14/46] fix: remove redundant context import --- packages/compass/src/app/components/entrypoint.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/compass/src/app/components/entrypoint.tsx b/packages/compass/src/app/components/entrypoint.tsx index 1bbcb163d43..866b3414188 100644 --- a/packages/compass/src/app/components/entrypoint.tsx +++ b/packages/compass/src/app/components/entrypoint.tsx @@ -30,7 +30,6 @@ import { createIpcSendTrack, } from '@mongodb-js/compass-telemetry'; import { DataModelStorageServiceProviderElectron } from '@mongodb-js/compass-data-modeling/renderer'; -import { Context } from '@segment/analytics-node'; import { ContextMenuProvider } from '@mongodb-js/compass-components'; const WithPreferencesAndLoggerProviders: React.FC = ({ children }) => { From 1d279caf13cf4a1fe03dc39fda901441f8d28c3d Mon Sep 17 00:00:00 2001 From: gagik Date: Tue, 20 May 2025 16:59:43 +0200 Subject: [PATCH 15/46] feat(compass-context-menu): add a headless context menu package --- package-lock.json | 104 ++++++++++++++++++ packages/compass-context-menu/.depcheckrc | 8 ++ packages/compass-context-menu/.eslintignore | 2 + packages/compass-context-menu/.eslintrc.js | 9 ++ packages/compass-context-menu/.mocharc.js | 2 + packages/compass-context-menu/package.json | 72 ++++++++++++ .../src/context-menu-content.ts | 23 ++++ .../src/context-menu-provider.tsx | 68 ++++++++++++ .../compass-context-menu/src/context-menu.tsx | 22 ++++ packages/compass-context-menu/src/types.ts | 21 ++++ .../src/use-context-menu.tsx | 55 +++++++++ .../compass-context-menu/tsconfig-lint.json | 5 + packages/compass-context-menu/tsconfig.json | 8 ++ 13 files changed, 399 insertions(+) create mode 100644 packages/compass-context-menu/.depcheckrc create mode 100644 packages/compass-context-menu/.eslintignore create mode 100644 packages/compass-context-menu/.eslintrc.js create mode 100644 packages/compass-context-menu/.mocharc.js create mode 100644 packages/compass-context-menu/package.json create mode 100644 packages/compass-context-menu/src/context-menu-content.ts create mode 100644 packages/compass-context-menu/src/context-menu-provider.tsx create mode 100644 packages/compass-context-menu/src/context-menu.tsx create mode 100644 packages/compass-context-menu/src/types.ts create mode 100644 packages/compass-context-menu/src/use-context-menu.tsx create mode 100644 packages/compass-context-menu/tsconfig-lint.json create mode 100644 packages/compass-context-menu/tsconfig.json diff --git a/package-lock.json b/package-lock.json index 284ef8ff00f..136ac1527de 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7864,6 +7864,10 @@ "resolved": "packages/compass-connections-navigation", "link": true }, + "node_modules/@mongodb-js/compass-context-menu": { + "resolved": "packages/compass-context-menu", + "link": true + }, "node_modules/@mongodb-js/compass-crud": { "resolved": "packages/compass-crud", "link": true @@ -44105,6 +44109,62 @@ "node": ">=0.3.1" } }, + "packages/compass-context-menu": { + "name": "@mongodb-js/compass-context-menu", + "version": "0.0.1", + "license": "SSPL", + "dependencies": { + "react": "^17.0.2" + }, + "devDependencies": { + "@mongodb-js/eslint-config-compass": "^1.3.8", + "@mongodb-js/mocha-config-compass": "^1.6.8", + "@mongodb-js/prettier-config-compass": "^1.2.8", + "@mongodb-js/tsconfig-compass": "^1.2.8", + "@types/chai": "^4.2.21", + "@types/mocha": "^9.0.0", + "@types/react": "^17.0.5", + "@types/react-dom": "^17.0.10", + "@types/sinon-chai": "^3.2.5", + "chai": "^4.3.6", + "depcheck": "^1.4.1", + "gen-esm-wrapper": "^1.1.0", + "mocha": "^10.2.0", + "nyc": "^15.1.0", + "sinon": "^9.2.3", + "typescript": "^5.0.4" + } + }, + "packages/compass-context-menu/node_modules/diff": { + "version": "4.0.2", + "resolved": "https://registry.npmjs.org/diff/-/diff-4.0.2.tgz", + "integrity": "sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A==", + "dev": true, + "license": "BSD-3-Clause", + "engines": { + "node": ">=0.3.1" + } + }, + "packages/compass-context-menu/node_modules/sinon": { + "version": "9.2.4", + "resolved": "https://registry.npmjs.org/sinon/-/sinon-9.2.4.tgz", + "integrity": "sha512-zljcULZQsJxVra28qIAL6ow1Z9tpattkCTEJR4RBP3TGc00FcttsP5pK284Nas5WjMZU5Yzy3kAIp3B3KRf5Yg==", + "deprecated": "16.1.1", + "dev": true, + "license": "BSD-3-Clause", + "dependencies": { + "@sinonjs/commons": "^1.8.1", + "@sinonjs/fake-timers": "^6.0.1", + "@sinonjs/samsam": "^5.3.1", + "diff": "^4.0.2", + "nise": "^4.0.4", + "supports-color": "^7.1.0" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/sinon" + } + }, "packages/compass-crud": { "name": "@mongodb-js/compass-crud", "version": "13.56.1", @@ -56549,6 +56609,50 @@ } } }, + "@mongodb-js/compass-context-menu": { + "version": "file:packages/compass-context-menu", + "requires": { + "@mongodb-js/eslint-config-compass": "^1.3.8", + "@mongodb-js/mocha-config-compass": "^1.6.8", + "@mongodb-js/prettier-config-compass": "^1.2.8", + "@mongodb-js/tsconfig-compass": "^1.2.8", + "@types/chai": "^4.2.21", + "@types/mocha": "^9.0.0", + "@types/react": "^17.0.5", + "@types/react-dom": "^17.0.10", + "@types/sinon-chai": "^3.2.5", + "chai": "^4.3.6", + "depcheck": "^1.4.1", + "gen-esm-wrapper": "^1.1.0", + "mocha": "^10.2.0", + "nyc": "^15.1.0", + "react": "^17.0.2", + "sinon": "^9.2.3", + "typescript": "^5.0.4" + }, + "dependencies": { + "diff": { + "version": "4.0.2", + "resolved": "https://registry.npmjs.org/diff/-/diff-4.0.2.tgz", + "integrity": "sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A==", + "dev": true + }, + "sinon": { + "version": "9.2.4", + "resolved": "https://registry.npmjs.org/sinon/-/sinon-9.2.4.tgz", + "integrity": "sha512-zljcULZQsJxVra28qIAL6ow1Z9tpattkCTEJR4RBP3TGc00FcttsP5pK284Nas5WjMZU5Yzy3kAIp3B3KRf5Yg==", + "dev": true, + "requires": { + "@sinonjs/commons": "^1.8.1", + "@sinonjs/fake-timers": "^6.0.1", + "@sinonjs/samsam": "^5.3.1", + "diff": "^4.0.2", + "nise": "^4.0.4", + "supports-color": "^7.1.0" + } + } + } + }, "@mongodb-js/compass-crud": { "version": "file:packages/compass-crud", "requires": { diff --git a/packages/compass-context-menu/.depcheckrc b/packages/compass-context-menu/.depcheckrc new file mode 100644 index 00000000000..ab0ef21b740 --- /dev/null +++ b/packages/compass-context-menu/.depcheckrc @@ -0,0 +1,8 @@ +ignores: + - '@mongodb-js/prettier-config-compass' + - '@mongodb-js/tsconfig-compass' + - '@types/chai' + - '@types/sinon-chai' + - 'sinon' +ignore-patterns: + - 'dist' diff --git a/packages/compass-context-menu/.eslintignore b/packages/compass-context-menu/.eslintignore new file mode 100644 index 00000000000..85a8a75e68c --- /dev/null +++ b/packages/compass-context-menu/.eslintignore @@ -0,0 +1,2 @@ +.nyc-output +dist diff --git a/packages/compass-context-menu/.eslintrc.js b/packages/compass-context-menu/.eslintrc.js new file mode 100644 index 00000000000..9c3ab95632f --- /dev/null +++ b/packages/compass-context-menu/.eslintrc.js @@ -0,0 +1,9 @@ +'use strict'; +module.exports = { + root: true, + extends: ['@mongodb-js/eslint-config-compass'], + parserOptions: { + tsconfigRootDir: __dirname, + project: ['./tsconfig-lint.json'], + }, +}; diff --git a/packages/compass-context-menu/.mocharc.js b/packages/compass-context-menu/.mocharc.js new file mode 100644 index 00000000000..e7eaccd61fa --- /dev/null +++ b/packages/compass-context-menu/.mocharc.js @@ -0,0 +1,2 @@ +'use strict'; +module.exports = require('@mongodb-js/mocha-config-compass'); diff --git a/packages/compass-context-menu/package.json b/packages/compass-context-menu/package.json new file mode 100644 index 00000000000..3cf186e0270 --- /dev/null +++ b/packages/compass-context-menu/package.json @@ -0,0 +1,72 @@ +{ + "name": "@mongodb-js/compass-context-menu", + "author": { + "name": "MongoDB Inc", + "email": "compass@mongodb.com" + }, + "publishConfig": { + "access": "public" + }, + "bugs": { + "url": "https://jira.mongodb.org/projects/COMPASS/issues", + "email": "compass@mongodb.com" + }, + "homepage": "https://github.com/mongodb-js/compass", + "version": "0.0.1", + "repository": { + "type": "git", + "url": "https://github.com/mongodb-js/compass.git" + }, + "files": [ + "dist" + ], + "license": "SSPL", + "main": "dist/index.js", + "compass:main": "src/index.ts", + "exports": { + "import": "./dist/.esm-wrapper.mjs", + "require": "./dist/index.js" + }, + "compass:exports": { + ".": "./src/index.ts" + }, + "types": "./dist/index.d.ts", + "scripts": { + "bootstrap": "npm run compile", + "prepublishOnly": "npm run compile && compass-scripts check-exports-exist", + "compile": "tsc -p tsconfig.json && gen-esm-wrapper . ./dist/.esm-wrapper.mjs", + "typecheck": "tsc -p tsconfig-lint.json --noEmit", + "eslint": "eslint-compass", + "prettier": "prettier-compass", + "lint": "npm run eslint . && npm run prettier -- --check .", + "depcheck": "compass-scripts check-peer-deps && depcheck", + "check": "npm run typecheck && npm run lint && npm run depcheck", + "check-ci": "npm run check", + "test": "mocha", + "test-cov": "nyc --compact=false --produce-source-map=false -x \"**/*.spec.*\" --reporter=lcov --reporter=text --reporter=html npm run test", + "test-watch": "npm run test -- --watch", + "test-ci": "npm run test-cov", + "reformat": "npm run eslint . -- --fix && npm run prettier -- --write ." + }, + "dependencies": { + "react": "^17.0.2" + }, + "devDependencies": { + "@mongodb-js/eslint-config-compass": "^1.3.8", + "@mongodb-js/mocha-config-compass": "^1.6.8", + "@mongodb-js/prettier-config-compass": "^1.2.8", + "@mongodb-js/tsconfig-compass": "^1.2.8", + "@types/chai": "^4.2.21", + "@types/mocha": "^9.0.0", + "@types/react": "^17.0.5", + "@types/react-dom": "^17.0.10", + "@types/sinon-chai": "^3.2.5", + "chai": "^4.3.6", + "depcheck": "^1.4.1", + "gen-esm-wrapper": "^1.1.0", + "mocha": "^10.2.0", + "nyc": "^15.1.0", + "sinon": "^9.2.3", + "typescript": "^5.0.4" + } +} diff --git a/packages/compass-context-menu/src/context-menu-content.ts b/packages/compass-context-menu/src/context-menu-content.ts new file mode 100644 index 00000000000..6856f1fe684 --- /dev/null +++ b/packages/compass-context-menu/src/context-menu-content.ts @@ -0,0 +1,23 @@ +const CONTEXT_MENUS_SYMBOL = Symbol('context_menus'); + +export type EnhancedMouseEvent = MouseEvent & { + [CONTEXT_MENUS_SYMBOL]?: React.ComponentType[]; +}; + +export function getContextMenuContent( + event: EnhancedMouseEvent +): React.ComponentType[] { + return event[CONTEXT_MENUS_SYMBOL] ?? []; +} + +export function appendContextMenuContent( + event: EnhancedMouseEvent, + content: React.ComponentType +) { + // Initialize if not already patched + if (event[CONTEXT_MENUS_SYMBOL] === undefined) { + event[CONTEXT_MENUS_SYMBOL] = [content]; + return; + } + event[CONTEXT_MENUS_SYMBOL].push(content); +} diff --git a/packages/compass-context-menu/src/context-menu-provider.tsx b/packages/compass-context-menu/src/context-menu-provider.tsx new file mode 100644 index 00000000000..7a7b59bf16a --- /dev/null +++ b/packages/compass-context-menu/src/context-menu-provider.tsx @@ -0,0 +1,68 @@ +import React, { + useCallback, + useEffect, + useState, + useMemo, + createContext, +} from 'react'; +import type { ContextMenuContext, MenuState } from './types'; +import { ContextMenu } from './context-menu'; +import type { EnhancedMouseEvent } from './context-menu-content'; +import { getContextMenuContent } from './context-menu-content'; + +export const Context = createContext(null); + +export function ContextMenuProvider({ + children, +}: React.PropsWithChildren) { + const [menu, setMenu] = useState({ isOpen: false }); + const close = useCallback(() => setMenu({ isOpen: false }), [setMenu]); + + useEffect(() => { + function handleContextMenu(event: MouseEvent) { + event.preventDefault(); + setMenu({ + isOpen: true, + children: getContextMenuContent(event as EnhancedMouseEvent).map( + (Content, index) => + ), + position: { + // TODO: Fix handling offset while scrolling + x: event.clientX, + y: event.clientY, + }, + }); + } + document.addEventListener('contextmenu', handleContextMenu); + + function handleClosingEvent(event: Event) { + if (!event.defaultPrevented) { + setMenu({ isOpen: false }); + } + } + document.addEventListener('click', handleClosingEvent); + window.addEventListener('resize', handleClosingEvent); + + return () => { + document.removeEventListener('contextmenu', handleContextMenu); + document.removeEventListener('click', handleClosingEvent); + window.removeEventListener('resize', handleClosingEvent); + }; + }, [setMenu]); + + const value = useMemo( + () => ({ + close, + }), + [close] + ); + + return ( + <> + {children} + {menu.isOpen && ( + {menu.children} + )} + + ); +} diff --git a/packages/compass-context-menu/src/context-menu.tsx b/packages/compass-context-menu/src/context-menu.tsx new file mode 100644 index 00000000000..b053bc4963a --- /dev/null +++ b/packages/compass-context-menu/src/context-menu.tsx @@ -0,0 +1,22 @@ +import { createPortal } from 'react-dom'; +import React from 'react'; + +type ContextMenuProps = React.PropsWithChildren<{ + position: { + x: number; + y: number; + }; +}>; + +export function ContextMenu({ children, position }: ContextMenuProps) { + const container = document.getElementById('context-menu-container'); + if (container === null) { + throw new Error('Expected a container for the context menu in the DOM'); + } + return createPortal( +
+ {children} +
, + container + ); +} diff --git a/packages/compass-context-menu/src/types.ts b/packages/compass-context-menu/src/types.ts new file mode 100644 index 00000000000..07efb491ac6 --- /dev/null +++ b/packages/compass-context-menu/src/types.ts @@ -0,0 +1,21 @@ +export type MenuState = + | { + isOpen: false; + } + | { + isOpen: true; + children: React.ReactNode; + position: { + x: number; + y: number; + }; + }; + +export type ContextMenuContext = { + close(): void; +}; + +export type MenuItem = { + label: string; + onAction: (event: Event) => void; +}; diff --git a/packages/compass-context-menu/src/use-context-menu.tsx b/packages/compass-context-menu/src/use-context-menu.tsx new file mode 100644 index 00000000000..d7ccc114a90 --- /dev/null +++ b/packages/compass-context-menu/src/use-context-menu.tsx @@ -0,0 +1,55 @@ +import React, { useContext, useMemo, useRef } from 'react'; +import { Context } from './context-menu-provider'; +import { appendContextMenuContent } from './context-menu-content'; +import type { MenuItem } from './types'; + +/** + * @returns an object with methods to {@link register} content for the menu and {@link close} the menu + */ +export function useContextMenu({ + Menu, +}: { + Menu: React.ComponentType<{ + items: MenuItem[]; + }>; +}) { + // Get the close function from the ContextProvider + const context = useContext(Context); + const previous = useRef void]>( + null + ); + + return useMemo(() => { + if (!context) { + throw new Error('useContextMenu called outside of the provider'); + } + + return { + close: context.close.bind(context), + /** + * @returns a callback ref, passed onto the element responsible for triggering the menu. + */ + register(content: React.ComponentType) { + function listener(event: MouseEvent) { + appendContextMenuContent(event, content); + } + return (trigger: HTMLElement | null) => { + if (previous.current) { + const [previousTrigger, previousListener] = previous.current; + previousTrigger.removeEventListener( + 'contextmenu', + previousListener + ); + } + if (trigger) { + trigger.addEventListener('contextmenu', listener); + previous.current = [trigger, listener]; + } + }; + }, + registerItems(items: MenuItem[]) { + return this.register(() => ); + }, + }; + }, [context, Menu]); +} diff --git a/packages/compass-context-menu/tsconfig-lint.json b/packages/compass-context-menu/tsconfig-lint.json new file mode 100644 index 00000000000..6bdef84f322 --- /dev/null +++ b/packages/compass-context-menu/tsconfig-lint.json @@ -0,0 +1,5 @@ +{ + "extends": "./tsconfig.json", + "include": ["**/*"], + "exclude": ["node_modules", "dist"] +} diff --git a/packages/compass-context-menu/tsconfig.json b/packages/compass-context-menu/tsconfig.json new file mode 100644 index 00000000000..79bc84584ce --- /dev/null +++ b/packages/compass-context-menu/tsconfig.json @@ -0,0 +1,8 @@ +{ + "extends": "@mongodb-js/tsconfig-compass/tsconfig.react.json", + "compilerOptions": { + "outDir": "dist" + }, + "include": ["src/**/*"], + "exclude": ["./src/**/*.spec.*"] +} From 1efdb2ecc1ed8a7d8211bc61ef391238273405a7 Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 21 May 2025 13:41:30 +0200 Subject: [PATCH 16/46] wip --- .../src/context-menu-provider.tsx | 4 +- .../src/use-context-menu.spec.tsx | 144 ++++++++++++++++++ 2 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 packages/compass-context-menu/src/use-context-menu.spec.tsx diff --git a/packages/compass-context-menu/src/context-menu-provider.tsx b/packages/compass-context-menu/src/context-menu-provider.tsx index 7a7b59bf16a..499d9c273c1 100644 --- a/packages/compass-context-menu/src/context-menu-provider.tsx +++ b/packages/compass-context-menu/src/context-menu-provider.tsx @@ -14,7 +14,9 @@ export const Context = createContext(null); export function ContextMenuProvider({ children, -}: React.PropsWithChildren) { +}: { + children: React.ReactNode; +}) { const [menu, setMenu] = useState({ isOpen: false }); const close = useCallback(() => setMenu({ isOpen: false }), [setMenu]); diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx new file mode 100644 index 00000000000..7337463e819 --- /dev/null +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -0,0 +1,144 @@ +import React from 'react'; +import { + render, + screen, + cleanup, + userEvent, +} from '@mongodb-js/testing-library-compass'; +import { expect } from 'chai'; +import sinon from 'sinon'; +import { useContextMenu } from './use-context-menu'; +import { ContextMenuProvider } from './context-menu-provider'; +import type { MenuItem } from './types'; + +describe('useContextMenu', function () { + const TestMenu: React.FC<{ items: MenuItem[] }> = ({ items }) => ( +
+ {items.map((item, idx) => ( +
+ {item.label} +
+ ))} +
+ ); + + const TestComponent = ({ + onRegister, + }: { + onRegister?: (ref: any) => void; + }) => { + const contextMenu = useContextMenu({ Menu: TestMenu }); + const items: MenuItem[] = [ + { + label: 'Test Item', + onAction: () => { + /* noop */ + }, + }, + ]; + const ref = contextMenu.registerItems(items); + + React.useEffect(() => { + onRegister?.(ref); + }, [ref, onRegister]); + + return ( +
+ Test Component +
+ ); + }; + + afterEach(cleanup); + + describe('when used outside provider', function () { + it('throws an error', function () { + expect(() => { + render(); + }).to.throw('useContextMenu called outside of the provider'); + }); + }); + + describe('when used inside provider', function () { + beforeEach(() => { + // Create the container for the context menu portal + const container = document.createElement('div'); + container.id = 'context-menu-container'; + document.body.appendChild(container); + }); + + afterEach(() => { + // Clean up the container + const container = document.getElementById('context-menu-container'); + if (container) { + document.body.removeChild(container); + } + }); + + it('renders without error', function () { + render( + + + + ); + + expect(screen.getByTestId('test-trigger')).to.exist; + }); + + it('registers context menu event listener', function () { + const onRegister = sinon.spy(); + + render( + + + + ); + + expect(onRegister).to.have.been.calledOnce; + expect(onRegister.firstCall.args[0]).to.be.a('function'); + }); + + it('shows context menu on right click', function () { + render( + + + + ); + + const trigger = screen.getByTestId('test-trigger'); + userEvent.click(trigger, { button: 2 }); + + // The menu should be rendered in the portal + expect(screen.getByTestId('menu-item-Test Item')).to.exist; + }); + + it('cleans up previous event listener when ref changes', function () { + const removeEventListenerSpy = sinon.spy(); + const addEventListenerSpy = sinon.spy(); + + const { rerender } = render( + + + + ); + + // Simulate ref change + const ref = screen.getByTestId('test-trigger'); + Object.defineProperty(ref, 'addEventListener', { + value: addEventListenerSpy, + }); + Object.defineProperty(ref, 'removeEventListener', { + value: removeEventListenerSpy, + }); + + rerender( + + + + ); + + expect(removeEventListenerSpy).to.have.been.calledWith('contextmenu'); + expect(addEventListenerSpy).to.have.been.calledWith('contextmenu'); + }); + }); +}); From 0f5303bb41171aa96d959cba498984d0678a0026 Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 21 May 2025 15:16:18 +0200 Subject: [PATCH 17/46] fix: add tests --- packages/compass-context-menu/.mocharc.js | 2 +- .../src/use-context-menu.spec.tsx | 58 +++++++------------ .../src/use-context-menu.tsx | 6 +- 3 files changed, 26 insertions(+), 40 deletions(-) diff --git a/packages/compass-context-menu/.mocharc.js b/packages/compass-context-menu/.mocharc.js index e7eaccd61fa..5a33f216327 100644 --- a/packages/compass-context-menu/.mocharc.js +++ b/packages/compass-context-menu/.mocharc.js @@ -1,2 +1,2 @@ 'use strict'; -module.exports = require('@mongodb-js/mocha-config-compass'); +module.exports = require('@mongodb-js/mocha-config-compass/react'); diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx index 7337463e819..8a5cb4b6fee 100644 --- a/packages/compass-context-menu/src/use-context-menu.spec.tsx +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -1,36 +1,37 @@ import React from 'react'; -import { - render, - screen, - cleanup, - userEvent, -} from '@mongodb-js/testing-library-compass'; +import { render, screen, userEvent } from '@mongodb-js/testing-library-compass'; import { expect } from 'chai'; import sinon from 'sinon'; import { useContextMenu } from './use-context-menu'; import { ContextMenuProvider } from './context-menu-provider'; import type { MenuItem } from './types'; +type TestMenuItem = MenuItem & { id: number }; + describe('useContextMenu', function () { - const TestMenu: React.FC<{ items: MenuItem[] }> = ({ items }) => ( + const TestMenu: React.FC<{ items: TestMenuItem[] }> = ({ items }) => (
{items.map((item, idx) => ( -
+
{item.label}
))}
); - const TestComponent = ({ - onRegister, - }: { - onRegister?: (ref: any) => void; - }) => { + const TestComponent = () => { const contextMenu = useContextMenu({ Menu: TestMenu }); - const items: MenuItem[] = [ + const items: TestMenuItem[] = [ { - label: 'Test Item', + id: 1, + label: 'Test A', + onAction: () => { + /* noop */ + }, + }, + { + id: 2, + label: 'Test B', onAction: () => { /* noop */ }, @@ -38,10 +39,6 @@ describe('useContextMenu', function () { ]; const ref = contextMenu.registerItems(items); - React.useEffect(() => { - onRegister?.(ref); - }, [ref, onRegister]); - return (
Test Component @@ -49,8 +46,6 @@ describe('useContextMenu', function () { ); }; - afterEach(cleanup); - describe('when used outside provider', function () { it('throws an error', function () { expect(() => { @@ -59,7 +54,7 @@ describe('useContextMenu', function () { }); }); - describe('when used inside provider', function () { + describe('with valid provider', function () { beforeEach(() => { // Create the container for the context menu portal const container = document.createElement('div'); @@ -85,19 +80,6 @@ describe('useContextMenu', function () { expect(screen.getByTestId('test-trigger')).to.exist; }); - it('registers context menu event listener', function () { - const onRegister = sinon.spy(); - - render( - - - - ); - - expect(onRegister).to.have.been.calledOnce; - expect(onRegister.firstCall.args[0]).to.be.a('function'); - }); - it('shows context menu on right click', function () { render( @@ -105,11 +87,15 @@ describe('useContextMenu', function () { ); + expect(screen.queryByTestId('menu-item-1')).not.to.exist; + expect(screen.queryByTestId('menu-item-2')).not.to.exist; + const trigger = screen.getByTestId('test-trigger'); userEvent.click(trigger, { button: 2 }); // The menu should be rendered in the portal - expect(screen.getByTestId('menu-item-Test Item')).to.exist; + expect(screen.getByTestId('menu-item-1')).to.exist; + expect(screen.getByTestId('menu-item-2')).to.exist; }); it('cleans up previous event listener when ref changes', function () { diff --git a/packages/compass-context-menu/src/use-context-menu.tsx b/packages/compass-context-menu/src/use-context-menu.tsx index d7ccc114a90..37993b04f15 100644 --- a/packages/compass-context-menu/src/use-context-menu.tsx +++ b/packages/compass-context-menu/src/use-context-menu.tsx @@ -6,11 +6,11 @@ import type { MenuItem } from './types'; /** * @returns an object with methods to {@link register} content for the menu and {@link close} the menu */ -export function useContextMenu({ +export function useContextMenu({ Menu, }: { Menu: React.ComponentType<{ - items: MenuItem[]; + items: T[]; }>; }) { // Get the close function from the ContextProvider @@ -47,7 +47,7 @@ export function useContextMenu({ } }; }, - registerItems(items: MenuItem[]) { + registerItems(items: T[]) { return this.register(() => ); }, }; From 33b2a41af24cf04760ac4e24f75873c8402e7f36 Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 21 May 2025 16:50:00 +0200 Subject: [PATCH 18/46] fix: add tests and fix types --- packages/compass-context-menu/src/types.ts | 2 +- .../src/use-context-menu.spec.tsx | 219 ++++++++++++++---- 2 files changed, 175 insertions(+), 46 deletions(-) diff --git a/packages/compass-context-menu/src/types.ts b/packages/compass-context-menu/src/types.ts index 07efb491ac6..e9ac549ba63 100644 --- a/packages/compass-context-menu/src/types.ts +++ b/packages/compass-context-menu/src/types.ts @@ -17,5 +17,5 @@ export type ContextMenuContext = { export type MenuItem = { label: string; - onAction: (event: Event) => void; + onAction: (event: React.KeyboardEvent | React.MouseEvent) => void; }; diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx index 8a5cb4b6fee..1d099272104 100644 --- a/packages/compass-context-menu/src/use-context-menu.spec.tsx +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -6,39 +6,48 @@ import { useContextMenu } from './use-context-menu'; import { ContextMenuProvider } from './context-menu-provider'; import type { MenuItem } from './types'; -type TestMenuItem = MenuItem & { id: number }; - describe('useContextMenu', function () { - const TestMenu: React.FC<{ items: TestMenuItem[] }> = ({ items }) => ( + const TestMenu: React.FC<{ items: MenuItem[] }> = ({ items }) => (
{items.map((item, idx) => ( -
+
item.onAction?.(event)} + onKeyDown={(event) => { + if (event.key === 'Enter') { + item.onAction?.(event); + } + }} + > {item.label}
))}
); - const TestComponent = () => { + const TestComponent = ({ + onRegister, + onAction, + }: { + onRegister?: (ref: any) => void; + onAction?: (id) => void; + }) => { const contextMenu = useContextMenu({ Menu: TestMenu }); - const items: TestMenuItem[] = [ - { - id: 1, - label: 'Test A', - onAction: () => { - /* noop */ - }, - }, + const items: MenuItem[] = [ { - id: 2, - label: 'Test B', - onAction: () => { - /* noop */ - }, + label: 'Test Item', + onAction: () => onAction?.(1), }, ]; const ref = contextMenu.registerItems(items); + React.useEffect(() => { + onRegister?.(ref); + }, [ref, onRegister]); + return (
Test Component @@ -46,6 +55,60 @@ describe('useContextMenu', function () { ); }; + // Add new test components for nested context menu scenario + const ParentComponent = ({ + onAction, + children, + }: { + onAction?: (id: number) => void; + children?: React.ReactNode; + }) => { + const contextMenu = useContextMenu({ Menu: TestMenu }); + const parentItems: MenuItem[] = [ + { + label: 'Parent Item 1', + onAction: () => onAction?.(1), + }, + { + label: 'Parent Item 2', + onAction: () => onAction?.(2), + }, + ]; + const ref = contextMenu.registerItems(parentItems); + + return ( +
+
Parent Component
+ {children} +
+ ); + }; + + const ChildComponent = ({ + onAction, + }: { + onAction?: (id: number) => void; + }) => { + const contextMenu = useContextMenu({ Menu: TestMenu }); + const childItems: MenuItem[] = [ + { + label: 'Child Item 1', + onAction: () => onAction?.(1), + }, + { + label: 'Child Item 2', + onAction: () => onAction?.(2), + }, + ]; + const ref = contextMenu.registerItems(childItems); + + return ( +
+ Child Component +
+ ); + }; + describe('when used outside provider', function () { it('throws an error', function () { expect(() => { @@ -54,7 +117,7 @@ describe('useContextMenu', function () { }); }); - describe('with valid provider', function () { + describe('with a valid provider', function () { beforeEach(() => { // Create the container for the context menu portal const container = document.createElement('div'); @@ -80,6 +143,19 @@ describe('useContextMenu', function () { expect(screen.getByTestId('test-trigger')).to.exist; }); + it('registers context menu event listener', function () { + const onRegister = sinon.spy(); + + render( + + + + ); + + expect(onRegister).to.have.been.calledOnce; + expect(onRegister.firstCall.args[0]).to.be.a('function'); + }); + it('shows context menu on right click', function () { render( @@ -87,44 +163,97 @@ describe('useContextMenu', function () { ); - expect(screen.queryByTestId('menu-item-1')).not.to.exist; - expect(screen.queryByTestId('menu-item-2')).not.to.exist; - const trigger = screen.getByTestId('test-trigger'); userEvent.click(trigger, { button: 2 }); // The menu should be rendered in the portal - expect(screen.getByTestId('menu-item-1')).to.exist; - expect(screen.getByTestId('menu-item-2')).to.exist; + expect(screen.getByTestId('menu-item-Test Item')).to.exist; }); - it('cleans up previous event listener when ref changes', function () { - const removeEventListenerSpy = sinon.spy(); - const addEventListenerSpy = sinon.spy(); + describe('with nested context menus', function () { + it('shows only parent items when right clicking parent area', function () { + render( + + + + ); - const { rerender } = render( - - - - ); + const parentTrigger = screen.getByTestId('parent-trigger'); + userEvent.click(parentTrigger, { button: 2 }); + + // Should show parent items + expect(screen.getByTestId('menu-item-Parent Item 1')).to.exist; + expect(screen.getByTestId('menu-item-Parent Item 2')).to.exist; - // Simulate ref change - const ref = screen.getByTestId('test-trigger'); - Object.defineProperty(ref, 'addEventListener', { - value: addEventListenerSpy, + // Should not show child items + expect(() => screen.getByTestId('menu-item-Child Item 1')).to.throw; + expect(() => screen.getByTestId('menu-item-Child Item 2')).to.throw; }); - Object.defineProperty(ref, 'removeEventListener', { - value: removeEventListenerSpy, + + it('shows both parent and child items when right clicking child area', function () { + render( + + + + + + ); + + const childTrigger = screen.getByTestId('child-trigger'); + userEvent.click(childTrigger, { button: 2 }); + + // Should show both parent and child items + expect(screen.getByTestId('menu-item-Parent Item 1')).to.exist; + expect(screen.getByTestId('menu-item-Parent Item 2')).to.exist; + expect(screen.getByTestId('menu-item-Child Item 1')).to.exist; + expect(screen.getByTestId('menu-item-Child Item 2')).to.exist; }); - rerender( - - - - ); + it('triggers only the child action when clicking child menu item', function () { + const parentOnAction = sinon.spy(); + const childOnAction = sinon.spy(); + + render( + + + + + + ); + + const childTrigger = screen.getByTestId('child-trigger'); + userEvent.click(childTrigger, { button: 2 }); + + const childItem1 = screen.getByTestId('menu-item-Child Item 1'); + userEvent.click(childItem1); - expect(removeEventListenerSpy).to.have.been.calledWith('contextmenu'); - expect(addEventListenerSpy).to.have.been.calledWith('contextmenu'); + expect(childOnAction).to.have.been.calledOnceWithExactly(1); + expect(parentOnAction).to.not.have.been.called; + expect(() => screen.getByTestId('test-menu')).to.throw; + }); + + it('triggers only the parent action when clicking a parent menu item from child context', function () { + const parentOnAction = sinon.spy(); + const childOnAction = sinon.spy(); + + render( + + + + + + ); + + const childTrigger = screen.getByTestId('child-trigger'); + userEvent.click(childTrigger, { button: 2 }); + + const parentItem1 = screen.getByTestId('menu-item-Parent Item 1'); + userEvent.click(parentItem1); + + expect(parentOnAction).to.have.been.calledOnceWithExactly(1); + expect(childOnAction).to.not.have.been.called; + expect(() => screen.getByTestId('test-menu')).to.throw; + }); }); }); }); From 4a2f0327b37be0b3e506db54288f3e0a16daf168 Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 21 May 2025 17:07:53 +0200 Subject: [PATCH 19/46] refactor: minor stylistic changes --- .../src/context-menu-provider.tsx | 3 +- .../src/use-context-menu.tsx | 53 +++++++++++-------- 2 files changed, 32 insertions(+), 24 deletions(-) diff --git a/packages/compass-context-menu/src/context-menu-provider.tsx b/packages/compass-context-menu/src/context-menu-provider.tsx index 499d9c273c1..43d9e893367 100644 --- a/packages/compass-context-menu/src/context-menu-provider.tsx +++ b/packages/compass-context-menu/src/context-menu-provider.tsx @@ -35,13 +35,14 @@ export function ContextMenuProvider({ }, }); } - document.addEventListener('contextmenu', handleContextMenu); function handleClosingEvent(event: Event) { if (!event.defaultPrevented) { setMenu({ isOpen: false }); } } + + document.addEventListener('contextmenu', handleContextMenu); document.addEventListener('click', handleClosingEvent); window.addEventListener('resize', handleClosingEvent); diff --git a/packages/compass-context-menu/src/use-context-menu.tsx b/packages/compass-context-menu/src/use-context-menu.tsx index 37993b04f15..91f51c2f849 100644 --- a/packages/compass-context-menu/src/use-context-menu.tsx +++ b/packages/compass-context-menu/src/use-context-menu.tsx @@ -3,16 +3,25 @@ import { Context } from './context-menu-provider'; import { appendContextMenuContent } from './context-menu-content'; import type { MenuItem } from './types'; -/** - * @returns an object with methods to {@link register} content for the menu and {@link close} the menu - */ +export type ContextMenuMethods = { + /** + * Close the context menu. + */ + close: () => void; + /** + * Register the menu items for the context menu. + * @returns a callback ref to be passed onto the element responsible for triggering the menu. + */ + registerItems: (items: T[]) => (trigger: HTMLElement | null) => void; +}; + export function useContextMenu({ Menu, }: { Menu: React.ComponentType<{ items: T[]; }>; -}) { +}): ContextMenuMethods { // Get the close function from the ContextProvider const context = useContext(Context); const previous = useRef void]>( @@ -24,31 +33,29 @@ export function useContextMenu({ throw new Error('useContextMenu called outside of the provider'); } + const register = (content: React.ComponentType) => { + function listener(event: MouseEvent) { + appendContextMenuContent(event, content); + } + return (trigger: HTMLElement | null) => { + if (previous.current) { + const [previousTrigger, previousListener] = previous.current; + previousTrigger.removeEventListener('contextmenu', previousListener); + } + if (trigger) { + trigger.addEventListener('contextmenu', listener); + previous.current = [trigger, listener]; + } + }; + }; + return { close: context.close.bind(context), /** * @returns a callback ref, passed onto the element responsible for triggering the menu. */ - register(content: React.ComponentType) { - function listener(event: MouseEvent) { - appendContextMenuContent(event, content); - } - return (trigger: HTMLElement | null) => { - if (previous.current) { - const [previousTrigger, previousListener] = previous.current; - previousTrigger.removeEventListener( - 'contextmenu', - previousListener - ); - } - if (trigger) { - trigger.addEventListener('contextmenu', listener); - previous.current = [trigger, listener]; - } - }; - }, registerItems(items: T[]) { - return this.register(() => ); + return register(() => ); }, }; }, [context, Menu]); From 8e1feb3f6ba7591fa1b6bae14e3465fa04cb300b Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 22 May 2025 11:59:55 +0200 Subject: [PATCH 20/46] fix: export types and rename MenuItem --- packages/compass-context-menu/src/index.ts | 2 ++ packages/compass-context-menu/src/types.ts | 2 +- .../compass-context-menu/src/use-context-menu.spec.tsx | 10 +++++----- packages/compass-context-menu/src/use-context-menu.tsx | 6 +++--- 4 files changed, 11 insertions(+), 9 deletions(-) create mode 100644 packages/compass-context-menu/src/index.ts diff --git a/packages/compass-context-menu/src/index.ts b/packages/compass-context-menu/src/index.ts new file mode 100644 index 00000000000..d60a97e04b3 --- /dev/null +++ b/packages/compass-context-menu/src/index.ts @@ -0,0 +1,2 @@ +export { useContextMenu } from './use-context-menu'; +export type { ContextMenuItem } from './types'; diff --git a/packages/compass-context-menu/src/types.ts b/packages/compass-context-menu/src/types.ts index e9ac549ba63..f453930dcb3 100644 --- a/packages/compass-context-menu/src/types.ts +++ b/packages/compass-context-menu/src/types.ts @@ -15,7 +15,7 @@ export type ContextMenuContext = { close(): void; }; -export type MenuItem = { +export type ContextMenuItem = { label: string; onAction: (event: React.KeyboardEvent | React.MouseEvent) => void; }; diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx index 1d099272104..eb857db9aeb 100644 --- a/packages/compass-context-menu/src/use-context-menu.spec.tsx +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -4,10 +4,10 @@ import { expect } from 'chai'; import sinon from 'sinon'; import { useContextMenu } from './use-context-menu'; import { ContextMenuProvider } from './context-menu-provider'; -import type { MenuItem } from './types'; +import type { ContextMenuItem } from './types'; describe('useContextMenu', function () { - const TestMenu: React.FC<{ items: MenuItem[] }> = ({ items }) => ( + const TestMenu: React.FC<{ items: ContextMenuItem[] }> = ({ items }) => (
{items.map((item, idx) => (
void; }) => { const contextMenu = useContextMenu({ Menu: TestMenu }); - const items: MenuItem[] = [ + const items: ContextMenuItem[] = [ { label: 'Test Item', onAction: () => onAction?.(1), @@ -64,7 +64,7 @@ describe('useContextMenu', function () { children?: React.ReactNode; }) => { const contextMenu = useContextMenu({ Menu: TestMenu }); - const parentItems: MenuItem[] = [ + const parentItems: ContextMenuItem[] = [ { label: 'Parent Item 1', onAction: () => onAction?.(1), @@ -90,7 +90,7 @@ describe('useContextMenu', function () { onAction?: (id: number) => void; }) => { const contextMenu = useContextMenu({ Menu: TestMenu }); - const childItems: MenuItem[] = [ + const childItems: ContextMenuItem[] = [ { label: 'Child Item 1', onAction: () => onAction?.(1), diff --git a/packages/compass-context-menu/src/use-context-menu.tsx b/packages/compass-context-menu/src/use-context-menu.tsx index 91f51c2f849..a0c97dead9b 100644 --- a/packages/compass-context-menu/src/use-context-menu.tsx +++ b/packages/compass-context-menu/src/use-context-menu.tsx @@ -1,9 +1,9 @@ import React, { useContext, useMemo, useRef } from 'react'; import { Context } from './context-menu-provider'; import { appendContextMenuContent } from './context-menu-content'; -import type { MenuItem } from './types'; +import type { ContextMenuItem } from './types'; -export type ContextMenuMethods = { +export type ContextMenuMethods = { /** * Close the context menu. */ @@ -15,7 +15,7 @@ export type ContextMenuMethods = { registerItems: (items: T[]) => (trigger: HTMLElement | null) => void; }; -export function useContextMenu({ +export function useContextMenu({ Menu, }: { Menu: React.ComponentType<{ From 1f55000a7789f08ab8e1b40eb34611f4737aee95 Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 22 May 2025 12:14:37 +0200 Subject: [PATCH 21/46] fix: use React.RefCallback --- packages/compass-context-menu/src/use-context-menu.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/compass-context-menu/src/use-context-menu.tsx b/packages/compass-context-menu/src/use-context-menu.tsx index a0c97dead9b..fa9e20e7537 100644 --- a/packages/compass-context-menu/src/use-context-menu.tsx +++ b/packages/compass-context-menu/src/use-context-menu.tsx @@ -12,7 +12,7 @@ export type ContextMenuMethods = { * Register the menu items for the context menu. * @returns a callback ref to be passed onto the element responsible for triggering the menu. */ - registerItems: (items: T[]) => (trigger: HTMLElement | null) => void; + registerItems: (items: T[]) => React.RefCallback; }; export function useContextMenu({ @@ -33,7 +33,9 @@ export function useContextMenu({ throw new Error('useContextMenu called outside of the provider'); } - const register = (content: React.ComponentType) => { + const register = ( + content: React.ComponentType + ): React.RefCallback => { function listener(event: MouseEvent) { appendContextMenuContent(event, content); } From 9618e8e187e761e80fdc421a9516e69db87badd7 Mon Sep 17 00:00:00 2001 From: gagik Date: Fri, 23 May 2025 13:50:50 +0200 Subject: [PATCH 22/46] refactor: use item groups instead of React elements, use wrapper, keep menu state consistent The refactor is meant to make the Leafygreen integration more straightforward: 1. We stick to item groups and instead have a single wrapper to handle any rendering differences between groups. This allows the wrapper to always have context of all items when rendering which is useful when inserting menu seperators in Leafygreen. Also encourages consistent UI (while allowing per-case customization if needed at wrapper-level). We could introduce itemWrappers instead of itemGroups but having one wrapper handling all seems cleaner to me. 2. More of the responsibility is moved to a parent wrapper component that will house the context menu. This allows us to standardize the right click menu and make better use of Leafygreen's menu component including its click handling (which has been removed from the context menu library). 3. Menu state (i.e. position) is now preserved even closed; this is useful for leafygreen's menu to animate in the same position instead of losing the position all together. --- .../src/context-menu-content.ts | 13 ++-- .../src/context-menu-provider.tsx | 57 ++++++++++------- packages/compass-context-menu/src/index.ts | 7 ++- packages/compass-context-menu/src/types.ts | 29 +++++---- .../src/use-context-menu.spec.tsx | 61 ++++++++++--------- .../src/use-context-menu.tsx | 59 +++++++++--------- 6 files changed, 124 insertions(+), 102 deletions(-) diff --git a/packages/compass-context-menu/src/context-menu-content.ts b/packages/compass-context-menu/src/context-menu-content.ts index 6856f1fe684..c301983a679 100644 --- a/packages/compass-context-menu/src/context-menu-content.ts +++ b/packages/compass-context-menu/src/context-menu-content.ts @@ -1,23 +1,24 @@ +import type { ContextMenuItemGroup } from './types'; + const CONTEXT_MENUS_SYMBOL = Symbol('context_menus'); export type EnhancedMouseEvent = MouseEvent & { - [CONTEXT_MENUS_SYMBOL]?: React.ComponentType[]; + [CONTEXT_MENUS_SYMBOL]?: ContextMenuItemGroup[]; }; export function getContextMenuContent( event: EnhancedMouseEvent -): React.ComponentType[] { +): ContextMenuItemGroup[] { return event[CONTEXT_MENUS_SYMBOL] ?? []; } export function appendContextMenuContent( event: EnhancedMouseEvent, - content: React.ComponentType + content: ContextMenuItemGroup ) { // Initialize if not already patched - if (event[CONTEXT_MENUS_SYMBOL] === undefined) { - event[CONTEXT_MENUS_SYMBOL] = [content]; - return; + if (!event[CONTEXT_MENUS_SYMBOL]) { + event[CONTEXT_MENUS_SYMBOL] = []; } event[CONTEXT_MENUS_SYMBOL].push(content); } diff --git a/packages/compass-context-menu/src/context-menu-provider.tsx b/packages/compass-context-menu/src/context-menu-provider.tsx index 43d9e893367..5dd1cba2cff 100644 --- a/packages/compass-context-menu/src/context-menu-provider.tsx +++ b/packages/compass-context-menu/src/context-menu-provider.tsx @@ -5,8 +5,7 @@ import React, { useMemo, createContext, } from 'react'; -import type { ContextMenuContext, MenuState } from './types'; -import { ContextMenu } from './context-menu'; +import type { ContextMenuContext, ContextMenuState } from './types'; import type { EnhancedMouseEvent } from './context-menu-content'; import { getContextMenuContent } from './context-menu-content'; @@ -14,20 +13,42 @@ export const Context = createContext(null); export function ContextMenuProvider({ children, + wrapper, }: { children: React.ReactNode; + wrapper: React.ComponentType<{ + menu: ContextMenuState & { close: () => void }; + }>; }) { - const [menu, setMenu] = useState({ isOpen: false }); - const close = useCallback(() => setMenu({ isOpen: false }), [setMenu]); + const [menu, setMenu] = useState({ + isOpen: false, + itemGroups: [], + position: { x: 0, y: 0 }, + }); + const close = useCallback(() => setMenu({ ...menu, isOpen: false }), [menu]); + + const handleClosingEvent = useCallback( + (event: Event) => { + if (!event.defaultPrevented) { + setMenu({ ...menu, isOpen: false }); + } + }, + [menu] + ); useEffect(() => { function handleContextMenu(event: MouseEvent) { event.preventDefault(); + + const itemGroups = getContextMenuContent(event as EnhancedMouseEvent); + + if (itemGroups.length === 0) { + return; + } + setMenu({ isOpen: true, - children: getContextMenuContent(event as EnhancedMouseEvent).map( - (Content, index) => - ), + itemGroups, position: { // TODO: Fix handling offset while scrolling x: event.clientX, @@ -36,22 +57,14 @@ export function ContextMenuProvider({ }); } - function handleClosingEvent(event: Event) { - if (!event.defaultPrevented) { - setMenu({ isOpen: false }); - } - } - document.addEventListener('contextmenu', handleContextMenu); - document.addEventListener('click', handleClosingEvent); window.addEventListener('resize', handleClosingEvent); return () => { document.removeEventListener('contextmenu', handleContextMenu); - document.removeEventListener('click', handleClosingEvent); window.removeEventListener('resize', handleClosingEvent); }; - }, [setMenu]); + }, [handleClosingEvent]); const value = useMemo( () => ({ @@ -60,12 +73,12 @@ export function ContextMenuProvider({ [close] ); + const Wrapper = wrapper ?? React.Fragment; + return ( - <> - {children} - {menu.isOpen && ( - {menu.children} - )} - + + {children} + + ); } diff --git a/packages/compass-context-menu/src/index.ts b/packages/compass-context-menu/src/index.ts index d60a97e04b3..75d933ef767 100644 --- a/packages/compass-context-menu/src/index.ts +++ b/packages/compass-context-menu/src/index.ts @@ -1,2 +1,7 @@ export { useContextMenu } from './use-context-menu'; -export type { ContextMenuItem } from './types'; +export { ContextMenuProvider } from './context-menu-provider'; +export type { + ContextMenuItem, + ContextMenuItemGroup, + ContextMenuWrapperProps, +} from './types'; diff --git a/packages/compass-context-menu/src/types.ts b/packages/compass-context-menu/src/types.ts index f453930dcb3..91e8d65cdcc 100644 --- a/packages/compass-context-menu/src/types.ts +++ b/packages/compass-context-menu/src/types.ts @@ -1,15 +1,20 @@ -export type MenuState = - | { - isOpen: false; - } - | { - isOpen: true; - children: React.ReactNode; - position: { - x: number; - y: number; - }; - }; +export interface ContextMenuItemGroup { + items: ContextMenuItem[]; + originListener: (event: MouseEvent) => void; +} + +export type ContextMenuState = { + isOpen: boolean; + itemGroups: ContextMenuItemGroup[]; + position: { + x: number; + y: number; + }; +}; + +export type ContextMenuWrapperProps = { + menu: ContextMenuState & { close: () => void }; +}; export type ContextMenuContext = { close(): void; diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx index eb857db9aeb..cfe0bfc7ef4 100644 --- a/packages/compass-context-menu/src/use-context-menu.spec.tsx +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -4,27 +4,29 @@ import { expect } from 'chai'; import sinon from 'sinon'; import { useContextMenu } from './use-context-menu'; import { ContextMenuProvider } from './context-menu-provider'; -import type { ContextMenuItem } from './types'; +import type { ContextMenuItem, ContextMenuWrapperProps } from './types'; describe('useContextMenu', function () { - const TestMenu: React.FC<{ items: ContextMenuItem[] }> = ({ items }) => ( + const TestMenu: React.FC = ({ menu }) => (
- {items.map((item, idx) => ( -
item.onAction?.(event)} - onKeyDown={(event) => { - if (event.key === 'Enter') { - item.onAction?.(event); - } - }} - > - {item.label} -
- ))} + {menu.itemGroups.flatMap((group, groupIdx) => + group.items.map((item, idx) => ( +
item.onAction?.(event)} + onKeyDown={(event) => { + if (event.key === 'Enter') { + item.onAction?.(event); + } + }} + > + {item.label} +
+ )) + )}
); @@ -33,9 +35,9 @@ describe('useContextMenu', function () { onAction, }: { onRegister?: (ref: any) => void; - onAction?: (id) => void; + onAction?: (id: number) => void; }) => { - const contextMenu = useContextMenu({ Menu: TestMenu }); + const contextMenu = useContextMenu(); const items: ContextMenuItem[] = [ { label: 'Test Item', @@ -55,7 +57,6 @@ describe('useContextMenu', function () { ); }; - // Add new test components for nested context menu scenario const ParentComponent = ({ onAction, children, @@ -63,7 +64,7 @@ describe('useContextMenu', function () { onAction?: (id: number) => void; children?: React.ReactNode; }) => { - const contextMenu = useContextMenu({ Menu: TestMenu }); + const contextMenu = useContextMenu(); const parentItems: ContextMenuItem[] = [ { label: 'Parent Item 1', @@ -89,7 +90,7 @@ describe('useContextMenu', function () { }: { onAction?: (id: number) => void; }) => { - const contextMenu = useContextMenu({ Menu: TestMenu }); + const contextMenu = useContextMenu(); const childItems: ContextMenuItem[] = [ { label: 'Child Item 1', @@ -135,7 +136,7 @@ describe('useContextMenu', function () { it('renders without error', function () { render( - + ); @@ -147,7 +148,7 @@ describe('useContextMenu', function () { const onRegister = sinon.spy(); render( - + ); @@ -158,7 +159,7 @@ describe('useContextMenu', function () { it('shows context menu on right click', function () { render( - + ); @@ -173,7 +174,7 @@ describe('useContextMenu', function () { describe('with nested context menus', function () { it('shows only parent items when right clicking parent area', function () { render( - + ); @@ -192,7 +193,7 @@ describe('useContextMenu', function () { it('shows both parent and child items when right clicking child area', function () { render( - + @@ -214,7 +215,7 @@ describe('useContextMenu', function () { const childOnAction = sinon.spy(); render( - + @@ -237,7 +238,7 @@ describe('useContextMenu', function () { const childOnAction = sinon.spy(); render( - + diff --git a/packages/compass-context-menu/src/use-context-menu.tsx b/packages/compass-context-menu/src/use-context-menu.tsx index fa9e20e7537..a60aeba4c69 100644 --- a/packages/compass-context-menu/src/use-context-menu.tsx +++ b/packages/compass-context-menu/src/use-context-menu.tsx @@ -1,4 +1,5 @@ -import React, { useContext, useMemo, useRef } from 'react'; +import type { RefCallback } from 'react'; +import { useContext, useMemo, useRef } from 'react'; import { Context } from './context-menu-provider'; import { appendContextMenuContent } from './context-menu-content'; import type { ContextMenuItem } from './types'; @@ -12,17 +13,12 @@ export type ContextMenuMethods = { * Register the menu items for the context menu. * @returns a callback ref to be passed onto the element responsible for triggering the menu. */ - registerItems: (items: T[]) => React.RefCallback; + registerItems: (items: T[]) => RefCallback; }; -export function useContextMenu({ - Menu, -}: { - Menu: React.ComponentType<{ - items: T[]; - }>; -}): ContextMenuMethods { - // Get the close function from the ContextProvider +export function useContextMenu< + T extends ContextMenuItem = ContextMenuItem +>(): ContextMenuMethods { const context = useContext(Context); const previous = useRef void]>( null @@ -33,32 +29,33 @@ export function useContextMenu({ throw new Error('useContextMenu called outside of the provider'); } - const register = ( - content: React.ComponentType - ): React.RefCallback => { - function listener(event: MouseEvent) { - appendContextMenuContent(event, content); - } - return (trigger: HTMLElement | null) => { - if (previous.current) { - const [previousTrigger, previousListener] = previous.current; - previousTrigger.removeEventListener('contextmenu', previousListener); - } - if (trigger) { - trigger.addEventListener('contextmenu', listener); - previous.current = [trigger, listener]; - } - }; - }; - return { close: context.close.bind(context), /** * @returns a callback ref, passed onto the element responsible for triggering the menu. */ - registerItems(items: T[]) { - return register(() => ); + registerItems(items: ContextMenuItem[]) { + function listener(event: MouseEvent): void { + appendContextMenuContent(event, { + items, + originListener: listener, + }); + } + + return (trigger: HTMLElement | null) => { + if (previous.current) { + const [previousTrigger, previousListener] = previous.current; + previousTrigger.removeEventListener( + 'contextmenu', + previousListener + ); + } + if (trigger) { + trigger.addEventListener('contextmenu', listener); + previous.current = [trigger, listener]; + } + }; }, }; - }, [context, Menu]); + }, [context]); } From ca1fb86e94310726b48829e599e2409f82ceb3ac Mon Sep 17 00:00:00 2001 From: gagik Date: Fri, 23 May 2025 17:12:31 +0200 Subject: [PATCH 23/46] fix: delete redundant context menu --- .../compass-context-menu/src/context-menu.tsx | 22 ------------------- 1 file changed, 22 deletions(-) delete mode 100644 packages/compass-context-menu/src/context-menu.tsx diff --git a/packages/compass-context-menu/src/context-menu.tsx b/packages/compass-context-menu/src/context-menu.tsx deleted file mode 100644 index b053bc4963a..00000000000 --- a/packages/compass-context-menu/src/context-menu.tsx +++ /dev/null @@ -1,22 +0,0 @@ -import { createPortal } from 'react-dom'; -import React from 'react'; - -type ContextMenuProps = React.PropsWithChildren<{ - position: { - x: number; - y: number; - }; -}>; - -export function ContextMenu({ children, position }: ContextMenuProps) { - const container = document.getElementById('context-menu-container'); - if (container === null) { - throw new Error('Expected a container for the context menu in the DOM'); - } - return createPortal( -
- {children} -
, - container - ); -} From aacdf1dc4f93b88dba7fce497ddbe8d262ee18cb Mon Sep 17 00:00:00 2001 From: gagik Date: Tue, 20 May 2025 16:59:43 +0200 Subject: [PATCH 24/46] feat(compass-context-menu): add a headless context menu package --- package-lock.json | 104 ++++++++++++++++++ packages/compass-context-menu/.depcheckrc | 8 ++ packages/compass-context-menu/.eslintignore | 2 + packages/compass-context-menu/.eslintrc.js | 9 ++ packages/compass-context-menu/.mocharc.js | 2 + packages/compass-context-menu/package.json | 72 ++++++++++++ .../src/context-menu-content.ts | 23 ++++ .../src/context-menu-provider.tsx | 68 ++++++++++++ .../compass-context-menu/src/context-menu.tsx | 22 ++++ packages/compass-context-menu/src/types.ts | 21 ++++ .../src/use-context-menu.tsx | 55 +++++++++ .../compass-context-menu/tsconfig-lint.json | 5 + packages/compass-context-menu/tsconfig.json | 8 ++ 13 files changed, 399 insertions(+) create mode 100644 packages/compass-context-menu/.depcheckrc create mode 100644 packages/compass-context-menu/.eslintignore create mode 100644 packages/compass-context-menu/.eslintrc.js create mode 100644 packages/compass-context-menu/.mocharc.js create mode 100644 packages/compass-context-menu/package.json create mode 100644 packages/compass-context-menu/src/context-menu-content.ts create mode 100644 packages/compass-context-menu/src/context-menu-provider.tsx create mode 100644 packages/compass-context-menu/src/context-menu.tsx create mode 100644 packages/compass-context-menu/src/types.ts create mode 100644 packages/compass-context-menu/src/use-context-menu.tsx create mode 100644 packages/compass-context-menu/tsconfig-lint.json create mode 100644 packages/compass-context-menu/tsconfig.json diff --git a/package-lock.json b/package-lock.json index 80e503995ba..a205c4ec913 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7864,6 +7864,10 @@ "resolved": "packages/compass-connections-navigation", "link": true }, + "node_modules/@mongodb-js/compass-context-menu": { + "resolved": "packages/compass-context-menu", + "link": true + }, "node_modules/@mongodb-js/compass-crud": { "resolved": "packages/compass-crud", "link": true @@ -43201,6 +43205,62 @@ "node": ">=0.3.1" } }, + "packages/compass-context-menu": { + "name": "@mongodb-js/compass-context-menu", + "version": "0.0.1", + "license": "SSPL", + "dependencies": { + "react": "^17.0.2" + }, + "devDependencies": { + "@mongodb-js/eslint-config-compass": "^1.3.8", + "@mongodb-js/mocha-config-compass": "^1.6.8", + "@mongodb-js/prettier-config-compass": "^1.2.8", + "@mongodb-js/tsconfig-compass": "^1.2.8", + "@types/chai": "^4.2.21", + "@types/mocha": "^9.0.0", + "@types/react": "^17.0.5", + "@types/react-dom": "^17.0.10", + "@types/sinon-chai": "^3.2.5", + "chai": "^4.3.6", + "depcheck": "^1.4.1", + "gen-esm-wrapper": "^1.1.0", + "mocha": "^10.2.0", + "nyc": "^15.1.0", + "sinon": "^9.2.3", + "typescript": "^5.0.4" + } + }, + "packages/compass-context-menu/node_modules/diff": { + "version": "4.0.2", + "resolved": "https://registry.npmjs.org/diff/-/diff-4.0.2.tgz", + "integrity": "sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A==", + "dev": true, + "license": "BSD-3-Clause", + "engines": { + "node": ">=0.3.1" + } + }, + "packages/compass-context-menu/node_modules/sinon": { + "version": "9.2.4", + "resolved": "https://registry.npmjs.org/sinon/-/sinon-9.2.4.tgz", + "integrity": "sha512-zljcULZQsJxVra28qIAL6ow1Z9tpattkCTEJR4RBP3TGc00FcttsP5pK284Nas5WjMZU5Yzy3kAIp3B3KRf5Yg==", + "deprecated": "16.1.1", + "dev": true, + "license": "BSD-3-Clause", + "dependencies": { + "@sinonjs/commons": "^1.8.1", + "@sinonjs/fake-timers": "^6.0.1", + "@sinonjs/samsam": "^5.3.1", + "diff": "^4.0.2", + "nise": "^4.0.4", + "supports-color": "^7.1.0" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/sinon" + } + }, "packages/compass-crud": { "name": "@mongodb-js/compass-crud", "version": "13.57.0", @@ -55646,6 +55706,50 @@ } } }, + "@mongodb-js/compass-context-menu": { + "version": "file:packages/compass-context-menu", + "requires": { + "@mongodb-js/eslint-config-compass": "^1.3.8", + "@mongodb-js/mocha-config-compass": "^1.6.8", + "@mongodb-js/prettier-config-compass": "^1.2.8", + "@mongodb-js/tsconfig-compass": "^1.2.8", + "@types/chai": "^4.2.21", + "@types/mocha": "^9.0.0", + "@types/react": "^17.0.5", + "@types/react-dom": "^17.0.10", + "@types/sinon-chai": "^3.2.5", + "chai": "^4.3.6", + "depcheck": "^1.4.1", + "gen-esm-wrapper": "^1.1.0", + "mocha": "^10.2.0", + "nyc": "^15.1.0", + "react": "^17.0.2", + "sinon": "^9.2.3", + "typescript": "^5.0.4" + }, + "dependencies": { + "diff": { + "version": "4.0.2", + "resolved": "https://registry.npmjs.org/diff/-/diff-4.0.2.tgz", + "integrity": "sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A==", + "dev": true + }, + "sinon": { + "version": "9.2.4", + "resolved": "https://registry.npmjs.org/sinon/-/sinon-9.2.4.tgz", + "integrity": "sha512-zljcULZQsJxVra28qIAL6ow1Z9tpattkCTEJR4RBP3TGc00FcttsP5pK284Nas5WjMZU5Yzy3kAIp3B3KRf5Yg==", + "dev": true, + "requires": { + "@sinonjs/commons": "^1.8.1", + "@sinonjs/fake-timers": "^6.0.1", + "@sinonjs/samsam": "^5.3.1", + "diff": "^4.0.2", + "nise": "^4.0.4", + "supports-color": "^7.1.0" + } + } + } + }, "@mongodb-js/compass-crud": { "version": "file:packages/compass-crud", "requires": { diff --git a/packages/compass-context-menu/.depcheckrc b/packages/compass-context-menu/.depcheckrc new file mode 100644 index 00000000000..ab0ef21b740 --- /dev/null +++ b/packages/compass-context-menu/.depcheckrc @@ -0,0 +1,8 @@ +ignores: + - '@mongodb-js/prettier-config-compass' + - '@mongodb-js/tsconfig-compass' + - '@types/chai' + - '@types/sinon-chai' + - 'sinon' +ignore-patterns: + - 'dist' diff --git a/packages/compass-context-menu/.eslintignore b/packages/compass-context-menu/.eslintignore new file mode 100644 index 00000000000..85a8a75e68c --- /dev/null +++ b/packages/compass-context-menu/.eslintignore @@ -0,0 +1,2 @@ +.nyc-output +dist diff --git a/packages/compass-context-menu/.eslintrc.js b/packages/compass-context-menu/.eslintrc.js new file mode 100644 index 00000000000..9c3ab95632f --- /dev/null +++ b/packages/compass-context-menu/.eslintrc.js @@ -0,0 +1,9 @@ +'use strict'; +module.exports = { + root: true, + extends: ['@mongodb-js/eslint-config-compass'], + parserOptions: { + tsconfigRootDir: __dirname, + project: ['./tsconfig-lint.json'], + }, +}; diff --git a/packages/compass-context-menu/.mocharc.js b/packages/compass-context-menu/.mocharc.js new file mode 100644 index 00000000000..e7eaccd61fa --- /dev/null +++ b/packages/compass-context-menu/.mocharc.js @@ -0,0 +1,2 @@ +'use strict'; +module.exports = require('@mongodb-js/mocha-config-compass'); diff --git a/packages/compass-context-menu/package.json b/packages/compass-context-menu/package.json new file mode 100644 index 00000000000..3cf186e0270 --- /dev/null +++ b/packages/compass-context-menu/package.json @@ -0,0 +1,72 @@ +{ + "name": "@mongodb-js/compass-context-menu", + "author": { + "name": "MongoDB Inc", + "email": "compass@mongodb.com" + }, + "publishConfig": { + "access": "public" + }, + "bugs": { + "url": "https://jira.mongodb.org/projects/COMPASS/issues", + "email": "compass@mongodb.com" + }, + "homepage": "https://github.com/mongodb-js/compass", + "version": "0.0.1", + "repository": { + "type": "git", + "url": "https://github.com/mongodb-js/compass.git" + }, + "files": [ + "dist" + ], + "license": "SSPL", + "main": "dist/index.js", + "compass:main": "src/index.ts", + "exports": { + "import": "./dist/.esm-wrapper.mjs", + "require": "./dist/index.js" + }, + "compass:exports": { + ".": "./src/index.ts" + }, + "types": "./dist/index.d.ts", + "scripts": { + "bootstrap": "npm run compile", + "prepublishOnly": "npm run compile && compass-scripts check-exports-exist", + "compile": "tsc -p tsconfig.json && gen-esm-wrapper . ./dist/.esm-wrapper.mjs", + "typecheck": "tsc -p tsconfig-lint.json --noEmit", + "eslint": "eslint-compass", + "prettier": "prettier-compass", + "lint": "npm run eslint . && npm run prettier -- --check .", + "depcheck": "compass-scripts check-peer-deps && depcheck", + "check": "npm run typecheck && npm run lint && npm run depcheck", + "check-ci": "npm run check", + "test": "mocha", + "test-cov": "nyc --compact=false --produce-source-map=false -x \"**/*.spec.*\" --reporter=lcov --reporter=text --reporter=html npm run test", + "test-watch": "npm run test -- --watch", + "test-ci": "npm run test-cov", + "reformat": "npm run eslint . -- --fix && npm run prettier -- --write ." + }, + "dependencies": { + "react": "^17.0.2" + }, + "devDependencies": { + "@mongodb-js/eslint-config-compass": "^1.3.8", + "@mongodb-js/mocha-config-compass": "^1.6.8", + "@mongodb-js/prettier-config-compass": "^1.2.8", + "@mongodb-js/tsconfig-compass": "^1.2.8", + "@types/chai": "^4.2.21", + "@types/mocha": "^9.0.0", + "@types/react": "^17.0.5", + "@types/react-dom": "^17.0.10", + "@types/sinon-chai": "^3.2.5", + "chai": "^4.3.6", + "depcheck": "^1.4.1", + "gen-esm-wrapper": "^1.1.0", + "mocha": "^10.2.0", + "nyc": "^15.1.0", + "sinon": "^9.2.3", + "typescript": "^5.0.4" + } +} diff --git a/packages/compass-context-menu/src/context-menu-content.ts b/packages/compass-context-menu/src/context-menu-content.ts new file mode 100644 index 00000000000..6856f1fe684 --- /dev/null +++ b/packages/compass-context-menu/src/context-menu-content.ts @@ -0,0 +1,23 @@ +const CONTEXT_MENUS_SYMBOL = Symbol('context_menus'); + +export type EnhancedMouseEvent = MouseEvent & { + [CONTEXT_MENUS_SYMBOL]?: React.ComponentType[]; +}; + +export function getContextMenuContent( + event: EnhancedMouseEvent +): React.ComponentType[] { + return event[CONTEXT_MENUS_SYMBOL] ?? []; +} + +export function appendContextMenuContent( + event: EnhancedMouseEvent, + content: React.ComponentType +) { + // Initialize if not already patched + if (event[CONTEXT_MENUS_SYMBOL] === undefined) { + event[CONTEXT_MENUS_SYMBOL] = [content]; + return; + } + event[CONTEXT_MENUS_SYMBOL].push(content); +} diff --git a/packages/compass-context-menu/src/context-menu-provider.tsx b/packages/compass-context-menu/src/context-menu-provider.tsx new file mode 100644 index 00000000000..7a7b59bf16a --- /dev/null +++ b/packages/compass-context-menu/src/context-menu-provider.tsx @@ -0,0 +1,68 @@ +import React, { + useCallback, + useEffect, + useState, + useMemo, + createContext, +} from 'react'; +import type { ContextMenuContext, MenuState } from './types'; +import { ContextMenu } from './context-menu'; +import type { EnhancedMouseEvent } from './context-menu-content'; +import { getContextMenuContent } from './context-menu-content'; + +export const Context = createContext(null); + +export function ContextMenuProvider({ + children, +}: React.PropsWithChildren) { + const [menu, setMenu] = useState({ isOpen: false }); + const close = useCallback(() => setMenu({ isOpen: false }), [setMenu]); + + useEffect(() => { + function handleContextMenu(event: MouseEvent) { + event.preventDefault(); + setMenu({ + isOpen: true, + children: getContextMenuContent(event as EnhancedMouseEvent).map( + (Content, index) => + ), + position: { + // TODO: Fix handling offset while scrolling + x: event.clientX, + y: event.clientY, + }, + }); + } + document.addEventListener('contextmenu', handleContextMenu); + + function handleClosingEvent(event: Event) { + if (!event.defaultPrevented) { + setMenu({ isOpen: false }); + } + } + document.addEventListener('click', handleClosingEvent); + window.addEventListener('resize', handleClosingEvent); + + return () => { + document.removeEventListener('contextmenu', handleContextMenu); + document.removeEventListener('click', handleClosingEvent); + window.removeEventListener('resize', handleClosingEvent); + }; + }, [setMenu]); + + const value = useMemo( + () => ({ + close, + }), + [close] + ); + + return ( + <> + {children} + {menu.isOpen && ( + {menu.children} + )} + + ); +} diff --git a/packages/compass-context-menu/src/context-menu.tsx b/packages/compass-context-menu/src/context-menu.tsx new file mode 100644 index 00000000000..b053bc4963a --- /dev/null +++ b/packages/compass-context-menu/src/context-menu.tsx @@ -0,0 +1,22 @@ +import { createPortal } from 'react-dom'; +import React from 'react'; + +type ContextMenuProps = React.PropsWithChildren<{ + position: { + x: number; + y: number; + }; +}>; + +export function ContextMenu({ children, position }: ContextMenuProps) { + const container = document.getElementById('context-menu-container'); + if (container === null) { + throw new Error('Expected a container for the context menu in the DOM'); + } + return createPortal( +
+ {children} +
, + container + ); +} diff --git a/packages/compass-context-menu/src/types.ts b/packages/compass-context-menu/src/types.ts new file mode 100644 index 00000000000..07efb491ac6 --- /dev/null +++ b/packages/compass-context-menu/src/types.ts @@ -0,0 +1,21 @@ +export type MenuState = + | { + isOpen: false; + } + | { + isOpen: true; + children: React.ReactNode; + position: { + x: number; + y: number; + }; + }; + +export type ContextMenuContext = { + close(): void; +}; + +export type MenuItem = { + label: string; + onAction: (event: Event) => void; +}; diff --git a/packages/compass-context-menu/src/use-context-menu.tsx b/packages/compass-context-menu/src/use-context-menu.tsx new file mode 100644 index 00000000000..d7ccc114a90 --- /dev/null +++ b/packages/compass-context-menu/src/use-context-menu.tsx @@ -0,0 +1,55 @@ +import React, { useContext, useMemo, useRef } from 'react'; +import { Context } from './context-menu-provider'; +import { appendContextMenuContent } from './context-menu-content'; +import type { MenuItem } from './types'; + +/** + * @returns an object with methods to {@link register} content for the menu and {@link close} the menu + */ +export function useContextMenu({ + Menu, +}: { + Menu: React.ComponentType<{ + items: MenuItem[]; + }>; +}) { + // Get the close function from the ContextProvider + const context = useContext(Context); + const previous = useRef void]>( + null + ); + + return useMemo(() => { + if (!context) { + throw new Error('useContextMenu called outside of the provider'); + } + + return { + close: context.close.bind(context), + /** + * @returns a callback ref, passed onto the element responsible for triggering the menu. + */ + register(content: React.ComponentType) { + function listener(event: MouseEvent) { + appendContextMenuContent(event, content); + } + return (trigger: HTMLElement | null) => { + if (previous.current) { + const [previousTrigger, previousListener] = previous.current; + previousTrigger.removeEventListener( + 'contextmenu', + previousListener + ); + } + if (trigger) { + trigger.addEventListener('contextmenu', listener); + previous.current = [trigger, listener]; + } + }; + }, + registerItems(items: MenuItem[]) { + return this.register(() => ); + }, + }; + }, [context, Menu]); +} diff --git a/packages/compass-context-menu/tsconfig-lint.json b/packages/compass-context-menu/tsconfig-lint.json new file mode 100644 index 00000000000..6bdef84f322 --- /dev/null +++ b/packages/compass-context-menu/tsconfig-lint.json @@ -0,0 +1,5 @@ +{ + "extends": "./tsconfig.json", + "include": ["**/*"], + "exclude": ["node_modules", "dist"] +} diff --git a/packages/compass-context-menu/tsconfig.json b/packages/compass-context-menu/tsconfig.json new file mode 100644 index 00000000000..79bc84584ce --- /dev/null +++ b/packages/compass-context-menu/tsconfig.json @@ -0,0 +1,8 @@ +{ + "extends": "@mongodb-js/tsconfig-compass/tsconfig.react.json", + "compilerOptions": { + "outDir": "dist" + }, + "include": ["src/**/*"], + "exclude": ["./src/**/*.spec.*"] +} From cb99706c351a314da3f718f1e7aada9a06ce12fa Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 21 May 2025 13:41:30 +0200 Subject: [PATCH 25/46] wip --- .../src/context-menu-provider.tsx | 4 +- .../src/use-context-menu.spec.tsx | 144 ++++++++++++++++++ 2 files changed, 147 insertions(+), 1 deletion(-) create mode 100644 packages/compass-context-menu/src/use-context-menu.spec.tsx diff --git a/packages/compass-context-menu/src/context-menu-provider.tsx b/packages/compass-context-menu/src/context-menu-provider.tsx index 7a7b59bf16a..499d9c273c1 100644 --- a/packages/compass-context-menu/src/context-menu-provider.tsx +++ b/packages/compass-context-menu/src/context-menu-provider.tsx @@ -14,7 +14,9 @@ export const Context = createContext(null); export function ContextMenuProvider({ children, -}: React.PropsWithChildren) { +}: { + children: React.ReactNode; +}) { const [menu, setMenu] = useState({ isOpen: false }); const close = useCallback(() => setMenu({ isOpen: false }), [setMenu]); diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx new file mode 100644 index 00000000000..7337463e819 --- /dev/null +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -0,0 +1,144 @@ +import React from 'react'; +import { + render, + screen, + cleanup, + userEvent, +} from '@mongodb-js/testing-library-compass'; +import { expect } from 'chai'; +import sinon from 'sinon'; +import { useContextMenu } from './use-context-menu'; +import { ContextMenuProvider } from './context-menu-provider'; +import type { MenuItem } from './types'; + +describe('useContextMenu', function () { + const TestMenu: React.FC<{ items: MenuItem[] }> = ({ items }) => ( +
+ {items.map((item, idx) => ( +
+ {item.label} +
+ ))} +
+ ); + + const TestComponent = ({ + onRegister, + }: { + onRegister?: (ref: any) => void; + }) => { + const contextMenu = useContextMenu({ Menu: TestMenu }); + const items: MenuItem[] = [ + { + label: 'Test Item', + onAction: () => { + /* noop */ + }, + }, + ]; + const ref = contextMenu.registerItems(items); + + React.useEffect(() => { + onRegister?.(ref); + }, [ref, onRegister]); + + return ( +
+ Test Component +
+ ); + }; + + afterEach(cleanup); + + describe('when used outside provider', function () { + it('throws an error', function () { + expect(() => { + render(); + }).to.throw('useContextMenu called outside of the provider'); + }); + }); + + describe('when used inside provider', function () { + beforeEach(() => { + // Create the container for the context menu portal + const container = document.createElement('div'); + container.id = 'context-menu-container'; + document.body.appendChild(container); + }); + + afterEach(() => { + // Clean up the container + const container = document.getElementById('context-menu-container'); + if (container) { + document.body.removeChild(container); + } + }); + + it('renders without error', function () { + render( + + + + ); + + expect(screen.getByTestId('test-trigger')).to.exist; + }); + + it('registers context menu event listener', function () { + const onRegister = sinon.spy(); + + render( + + + + ); + + expect(onRegister).to.have.been.calledOnce; + expect(onRegister.firstCall.args[0]).to.be.a('function'); + }); + + it('shows context menu on right click', function () { + render( + + + + ); + + const trigger = screen.getByTestId('test-trigger'); + userEvent.click(trigger, { button: 2 }); + + // The menu should be rendered in the portal + expect(screen.getByTestId('menu-item-Test Item')).to.exist; + }); + + it('cleans up previous event listener when ref changes', function () { + const removeEventListenerSpy = sinon.spy(); + const addEventListenerSpy = sinon.spy(); + + const { rerender } = render( + + + + ); + + // Simulate ref change + const ref = screen.getByTestId('test-trigger'); + Object.defineProperty(ref, 'addEventListener', { + value: addEventListenerSpy, + }); + Object.defineProperty(ref, 'removeEventListener', { + value: removeEventListenerSpy, + }); + + rerender( + + + + ); + + expect(removeEventListenerSpy).to.have.been.calledWith('contextmenu'); + expect(addEventListenerSpy).to.have.been.calledWith('contextmenu'); + }); + }); +}); From da22b51b2bd887bb34b824eabee9c3b1cd2a59e1 Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 21 May 2025 15:16:18 +0200 Subject: [PATCH 26/46] fix: add tests --- packages/compass-context-menu/.mocharc.js | 2 +- .../src/use-context-menu.spec.tsx | 58 +++++++------------ .../src/use-context-menu.tsx | 6 +- 3 files changed, 26 insertions(+), 40 deletions(-) diff --git a/packages/compass-context-menu/.mocharc.js b/packages/compass-context-menu/.mocharc.js index e7eaccd61fa..5a33f216327 100644 --- a/packages/compass-context-menu/.mocharc.js +++ b/packages/compass-context-menu/.mocharc.js @@ -1,2 +1,2 @@ 'use strict'; -module.exports = require('@mongodb-js/mocha-config-compass'); +module.exports = require('@mongodb-js/mocha-config-compass/react'); diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx index 7337463e819..8a5cb4b6fee 100644 --- a/packages/compass-context-menu/src/use-context-menu.spec.tsx +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -1,36 +1,37 @@ import React from 'react'; -import { - render, - screen, - cleanup, - userEvent, -} from '@mongodb-js/testing-library-compass'; +import { render, screen, userEvent } from '@mongodb-js/testing-library-compass'; import { expect } from 'chai'; import sinon from 'sinon'; import { useContextMenu } from './use-context-menu'; import { ContextMenuProvider } from './context-menu-provider'; import type { MenuItem } from './types'; +type TestMenuItem = MenuItem & { id: number }; + describe('useContextMenu', function () { - const TestMenu: React.FC<{ items: MenuItem[] }> = ({ items }) => ( + const TestMenu: React.FC<{ items: TestMenuItem[] }> = ({ items }) => (
{items.map((item, idx) => ( -
+
{item.label}
))}
); - const TestComponent = ({ - onRegister, - }: { - onRegister?: (ref: any) => void; - }) => { + const TestComponent = () => { const contextMenu = useContextMenu({ Menu: TestMenu }); - const items: MenuItem[] = [ + const items: TestMenuItem[] = [ { - label: 'Test Item', + id: 1, + label: 'Test A', + onAction: () => { + /* noop */ + }, + }, + { + id: 2, + label: 'Test B', onAction: () => { /* noop */ }, @@ -38,10 +39,6 @@ describe('useContextMenu', function () { ]; const ref = contextMenu.registerItems(items); - React.useEffect(() => { - onRegister?.(ref); - }, [ref, onRegister]); - return (
Test Component @@ -49,8 +46,6 @@ describe('useContextMenu', function () { ); }; - afterEach(cleanup); - describe('when used outside provider', function () { it('throws an error', function () { expect(() => { @@ -59,7 +54,7 @@ describe('useContextMenu', function () { }); }); - describe('when used inside provider', function () { + describe('with valid provider', function () { beforeEach(() => { // Create the container for the context menu portal const container = document.createElement('div'); @@ -85,19 +80,6 @@ describe('useContextMenu', function () { expect(screen.getByTestId('test-trigger')).to.exist; }); - it('registers context menu event listener', function () { - const onRegister = sinon.spy(); - - render( - - - - ); - - expect(onRegister).to.have.been.calledOnce; - expect(onRegister.firstCall.args[0]).to.be.a('function'); - }); - it('shows context menu on right click', function () { render( @@ -105,11 +87,15 @@ describe('useContextMenu', function () { ); + expect(screen.queryByTestId('menu-item-1')).not.to.exist; + expect(screen.queryByTestId('menu-item-2')).not.to.exist; + const trigger = screen.getByTestId('test-trigger'); userEvent.click(trigger, { button: 2 }); // The menu should be rendered in the portal - expect(screen.getByTestId('menu-item-Test Item')).to.exist; + expect(screen.getByTestId('menu-item-1')).to.exist; + expect(screen.getByTestId('menu-item-2')).to.exist; }); it('cleans up previous event listener when ref changes', function () { diff --git a/packages/compass-context-menu/src/use-context-menu.tsx b/packages/compass-context-menu/src/use-context-menu.tsx index d7ccc114a90..37993b04f15 100644 --- a/packages/compass-context-menu/src/use-context-menu.tsx +++ b/packages/compass-context-menu/src/use-context-menu.tsx @@ -6,11 +6,11 @@ import type { MenuItem } from './types'; /** * @returns an object with methods to {@link register} content for the menu and {@link close} the menu */ -export function useContextMenu({ +export function useContextMenu({ Menu, }: { Menu: React.ComponentType<{ - items: MenuItem[]; + items: T[]; }>; }) { // Get the close function from the ContextProvider @@ -47,7 +47,7 @@ export function useContextMenu({ } }; }, - registerItems(items: MenuItem[]) { + registerItems(items: T[]) { return this.register(() => ); }, }; From 78ff94cac09c3457adae201d09746c9ccadf93cb Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 21 May 2025 16:50:00 +0200 Subject: [PATCH 27/46] fix: add tests and fix types --- packages/compass-context-menu/src/types.ts | 2 +- .../src/use-context-menu.spec.tsx | 219 ++++++++++++++---- 2 files changed, 175 insertions(+), 46 deletions(-) diff --git a/packages/compass-context-menu/src/types.ts b/packages/compass-context-menu/src/types.ts index 07efb491ac6..e9ac549ba63 100644 --- a/packages/compass-context-menu/src/types.ts +++ b/packages/compass-context-menu/src/types.ts @@ -17,5 +17,5 @@ export type ContextMenuContext = { export type MenuItem = { label: string; - onAction: (event: Event) => void; + onAction: (event: React.KeyboardEvent | React.MouseEvent) => void; }; diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx index 8a5cb4b6fee..1d099272104 100644 --- a/packages/compass-context-menu/src/use-context-menu.spec.tsx +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -6,39 +6,48 @@ import { useContextMenu } from './use-context-menu'; import { ContextMenuProvider } from './context-menu-provider'; import type { MenuItem } from './types'; -type TestMenuItem = MenuItem & { id: number }; - describe('useContextMenu', function () { - const TestMenu: React.FC<{ items: TestMenuItem[] }> = ({ items }) => ( + const TestMenu: React.FC<{ items: MenuItem[] }> = ({ items }) => (
{items.map((item, idx) => ( -
+
item.onAction?.(event)} + onKeyDown={(event) => { + if (event.key === 'Enter') { + item.onAction?.(event); + } + }} + > {item.label}
))}
); - const TestComponent = () => { + const TestComponent = ({ + onRegister, + onAction, + }: { + onRegister?: (ref: any) => void; + onAction?: (id) => void; + }) => { const contextMenu = useContextMenu({ Menu: TestMenu }); - const items: TestMenuItem[] = [ - { - id: 1, - label: 'Test A', - onAction: () => { - /* noop */ - }, - }, + const items: MenuItem[] = [ { - id: 2, - label: 'Test B', - onAction: () => { - /* noop */ - }, + label: 'Test Item', + onAction: () => onAction?.(1), }, ]; const ref = contextMenu.registerItems(items); + React.useEffect(() => { + onRegister?.(ref); + }, [ref, onRegister]); + return (
Test Component @@ -46,6 +55,60 @@ describe('useContextMenu', function () { ); }; + // Add new test components for nested context menu scenario + const ParentComponent = ({ + onAction, + children, + }: { + onAction?: (id: number) => void; + children?: React.ReactNode; + }) => { + const contextMenu = useContextMenu({ Menu: TestMenu }); + const parentItems: MenuItem[] = [ + { + label: 'Parent Item 1', + onAction: () => onAction?.(1), + }, + { + label: 'Parent Item 2', + onAction: () => onAction?.(2), + }, + ]; + const ref = contextMenu.registerItems(parentItems); + + return ( +
+
Parent Component
+ {children} +
+ ); + }; + + const ChildComponent = ({ + onAction, + }: { + onAction?: (id: number) => void; + }) => { + const contextMenu = useContextMenu({ Menu: TestMenu }); + const childItems: MenuItem[] = [ + { + label: 'Child Item 1', + onAction: () => onAction?.(1), + }, + { + label: 'Child Item 2', + onAction: () => onAction?.(2), + }, + ]; + const ref = contextMenu.registerItems(childItems); + + return ( +
+ Child Component +
+ ); + }; + describe('when used outside provider', function () { it('throws an error', function () { expect(() => { @@ -54,7 +117,7 @@ describe('useContextMenu', function () { }); }); - describe('with valid provider', function () { + describe('with a valid provider', function () { beforeEach(() => { // Create the container for the context menu portal const container = document.createElement('div'); @@ -80,6 +143,19 @@ describe('useContextMenu', function () { expect(screen.getByTestId('test-trigger')).to.exist; }); + it('registers context menu event listener', function () { + const onRegister = sinon.spy(); + + render( + + + + ); + + expect(onRegister).to.have.been.calledOnce; + expect(onRegister.firstCall.args[0]).to.be.a('function'); + }); + it('shows context menu on right click', function () { render( @@ -87,44 +163,97 @@ describe('useContextMenu', function () { ); - expect(screen.queryByTestId('menu-item-1')).not.to.exist; - expect(screen.queryByTestId('menu-item-2')).not.to.exist; - const trigger = screen.getByTestId('test-trigger'); userEvent.click(trigger, { button: 2 }); // The menu should be rendered in the portal - expect(screen.getByTestId('menu-item-1')).to.exist; - expect(screen.getByTestId('menu-item-2')).to.exist; + expect(screen.getByTestId('menu-item-Test Item')).to.exist; }); - it('cleans up previous event listener when ref changes', function () { - const removeEventListenerSpy = sinon.spy(); - const addEventListenerSpy = sinon.spy(); + describe('with nested context menus', function () { + it('shows only parent items when right clicking parent area', function () { + render( + + + + ); - const { rerender } = render( - - - - ); + const parentTrigger = screen.getByTestId('parent-trigger'); + userEvent.click(parentTrigger, { button: 2 }); + + // Should show parent items + expect(screen.getByTestId('menu-item-Parent Item 1')).to.exist; + expect(screen.getByTestId('menu-item-Parent Item 2')).to.exist; - // Simulate ref change - const ref = screen.getByTestId('test-trigger'); - Object.defineProperty(ref, 'addEventListener', { - value: addEventListenerSpy, + // Should not show child items + expect(() => screen.getByTestId('menu-item-Child Item 1')).to.throw; + expect(() => screen.getByTestId('menu-item-Child Item 2')).to.throw; }); - Object.defineProperty(ref, 'removeEventListener', { - value: removeEventListenerSpy, + + it('shows both parent and child items when right clicking child area', function () { + render( + + + + + + ); + + const childTrigger = screen.getByTestId('child-trigger'); + userEvent.click(childTrigger, { button: 2 }); + + // Should show both parent and child items + expect(screen.getByTestId('menu-item-Parent Item 1')).to.exist; + expect(screen.getByTestId('menu-item-Parent Item 2')).to.exist; + expect(screen.getByTestId('menu-item-Child Item 1')).to.exist; + expect(screen.getByTestId('menu-item-Child Item 2')).to.exist; }); - rerender( - - - - ); + it('triggers only the child action when clicking child menu item', function () { + const parentOnAction = sinon.spy(); + const childOnAction = sinon.spy(); + + render( + + + + + + ); + + const childTrigger = screen.getByTestId('child-trigger'); + userEvent.click(childTrigger, { button: 2 }); + + const childItem1 = screen.getByTestId('menu-item-Child Item 1'); + userEvent.click(childItem1); - expect(removeEventListenerSpy).to.have.been.calledWith('contextmenu'); - expect(addEventListenerSpy).to.have.been.calledWith('contextmenu'); + expect(childOnAction).to.have.been.calledOnceWithExactly(1); + expect(parentOnAction).to.not.have.been.called; + expect(() => screen.getByTestId('test-menu')).to.throw; + }); + + it('triggers only the parent action when clicking a parent menu item from child context', function () { + const parentOnAction = sinon.spy(); + const childOnAction = sinon.spy(); + + render( + + + + + + ); + + const childTrigger = screen.getByTestId('child-trigger'); + userEvent.click(childTrigger, { button: 2 }); + + const parentItem1 = screen.getByTestId('menu-item-Parent Item 1'); + userEvent.click(parentItem1); + + expect(parentOnAction).to.have.been.calledOnceWithExactly(1); + expect(childOnAction).to.not.have.been.called; + expect(() => screen.getByTestId('test-menu')).to.throw; + }); }); }); }); From a24e89cf1206972ac7d4a799e8837d21ae10250a Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 21 May 2025 17:07:53 +0200 Subject: [PATCH 28/46] refactor: minor stylistic changes --- .../src/context-menu-provider.tsx | 3 +- .../src/use-context-menu.tsx | 53 +++++++++++-------- 2 files changed, 32 insertions(+), 24 deletions(-) diff --git a/packages/compass-context-menu/src/context-menu-provider.tsx b/packages/compass-context-menu/src/context-menu-provider.tsx index 499d9c273c1..43d9e893367 100644 --- a/packages/compass-context-menu/src/context-menu-provider.tsx +++ b/packages/compass-context-menu/src/context-menu-provider.tsx @@ -35,13 +35,14 @@ export function ContextMenuProvider({ }, }); } - document.addEventListener('contextmenu', handleContextMenu); function handleClosingEvent(event: Event) { if (!event.defaultPrevented) { setMenu({ isOpen: false }); } } + + document.addEventListener('contextmenu', handleContextMenu); document.addEventListener('click', handleClosingEvent); window.addEventListener('resize', handleClosingEvent); diff --git a/packages/compass-context-menu/src/use-context-menu.tsx b/packages/compass-context-menu/src/use-context-menu.tsx index 37993b04f15..91f51c2f849 100644 --- a/packages/compass-context-menu/src/use-context-menu.tsx +++ b/packages/compass-context-menu/src/use-context-menu.tsx @@ -3,16 +3,25 @@ import { Context } from './context-menu-provider'; import { appendContextMenuContent } from './context-menu-content'; import type { MenuItem } from './types'; -/** - * @returns an object with methods to {@link register} content for the menu and {@link close} the menu - */ +export type ContextMenuMethods = { + /** + * Close the context menu. + */ + close: () => void; + /** + * Register the menu items for the context menu. + * @returns a callback ref to be passed onto the element responsible for triggering the menu. + */ + registerItems: (items: T[]) => (trigger: HTMLElement | null) => void; +}; + export function useContextMenu({ Menu, }: { Menu: React.ComponentType<{ items: T[]; }>; -}) { +}): ContextMenuMethods { // Get the close function from the ContextProvider const context = useContext(Context); const previous = useRef void]>( @@ -24,31 +33,29 @@ export function useContextMenu({ throw new Error('useContextMenu called outside of the provider'); } + const register = (content: React.ComponentType) => { + function listener(event: MouseEvent) { + appendContextMenuContent(event, content); + } + return (trigger: HTMLElement | null) => { + if (previous.current) { + const [previousTrigger, previousListener] = previous.current; + previousTrigger.removeEventListener('contextmenu', previousListener); + } + if (trigger) { + trigger.addEventListener('contextmenu', listener); + previous.current = [trigger, listener]; + } + }; + }; + return { close: context.close.bind(context), /** * @returns a callback ref, passed onto the element responsible for triggering the menu. */ - register(content: React.ComponentType) { - function listener(event: MouseEvent) { - appendContextMenuContent(event, content); - } - return (trigger: HTMLElement | null) => { - if (previous.current) { - const [previousTrigger, previousListener] = previous.current; - previousTrigger.removeEventListener( - 'contextmenu', - previousListener - ); - } - if (trigger) { - trigger.addEventListener('contextmenu', listener); - previous.current = [trigger, listener]; - } - }; - }, registerItems(items: T[]) { - return this.register(() => ); + return register(() => ); }, }; }, [context, Menu]); From f3869eac0cab0fa2e0ddd1dd494cc418109b5bf0 Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 22 May 2025 11:59:55 +0200 Subject: [PATCH 29/46] fix: export types and rename MenuItem --- packages/compass-context-menu/src/index.ts | 2 ++ packages/compass-context-menu/src/types.ts | 2 +- .../compass-context-menu/src/use-context-menu.spec.tsx | 10 +++++----- packages/compass-context-menu/src/use-context-menu.tsx | 6 +++--- 4 files changed, 11 insertions(+), 9 deletions(-) create mode 100644 packages/compass-context-menu/src/index.ts diff --git a/packages/compass-context-menu/src/index.ts b/packages/compass-context-menu/src/index.ts new file mode 100644 index 00000000000..d60a97e04b3 --- /dev/null +++ b/packages/compass-context-menu/src/index.ts @@ -0,0 +1,2 @@ +export { useContextMenu } from './use-context-menu'; +export type { ContextMenuItem } from './types'; diff --git a/packages/compass-context-menu/src/types.ts b/packages/compass-context-menu/src/types.ts index e9ac549ba63..f453930dcb3 100644 --- a/packages/compass-context-menu/src/types.ts +++ b/packages/compass-context-menu/src/types.ts @@ -15,7 +15,7 @@ export type ContextMenuContext = { close(): void; }; -export type MenuItem = { +export type ContextMenuItem = { label: string; onAction: (event: React.KeyboardEvent | React.MouseEvent) => void; }; diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx index 1d099272104..eb857db9aeb 100644 --- a/packages/compass-context-menu/src/use-context-menu.spec.tsx +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -4,10 +4,10 @@ import { expect } from 'chai'; import sinon from 'sinon'; import { useContextMenu } from './use-context-menu'; import { ContextMenuProvider } from './context-menu-provider'; -import type { MenuItem } from './types'; +import type { ContextMenuItem } from './types'; describe('useContextMenu', function () { - const TestMenu: React.FC<{ items: MenuItem[] }> = ({ items }) => ( + const TestMenu: React.FC<{ items: ContextMenuItem[] }> = ({ items }) => (
{items.map((item, idx) => (
void; }) => { const contextMenu = useContextMenu({ Menu: TestMenu }); - const items: MenuItem[] = [ + const items: ContextMenuItem[] = [ { label: 'Test Item', onAction: () => onAction?.(1), @@ -64,7 +64,7 @@ describe('useContextMenu', function () { children?: React.ReactNode; }) => { const contextMenu = useContextMenu({ Menu: TestMenu }); - const parentItems: MenuItem[] = [ + const parentItems: ContextMenuItem[] = [ { label: 'Parent Item 1', onAction: () => onAction?.(1), @@ -90,7 +90,7 @@ describe('useContextMenu', function () { onAction?: (id: number) => void; }) => { const contextMenu = useContextMenu({ Menu: TestMenu }); - const childItems: MenuItem[] = [ + const childItems: ContextMenuItem[] = [ { label: 'Child Item 1', onAction: () => onAction?.(1), diff --git a/packages/compass-context-menu/src/use-context-menu.tsx b/packages/compass-context-menu/src/use-context-menu.tsx index 91f51c2f849..a0c97dead9b 100644 --- a/packages/compass-context-menu/src/use-context-menu.tsx +++ b/packages/compass-context-menu/src/use-context-menu.tsx @@ -1,9 +1,9 @@ import React, { useContext, useMemo, useRef } from 'react'; import { Context } from './context-menu-provider'; import { appendContextMenuContent } from './context-menu-content'; -import type { MenuItem } from './types'; +import type { ContextMenuItem } from './types'; -export type ContextMenuMethods = { +export type ContextMenuMethods = { /** * Close the context menu. */ @@ -15,7 +15,7 @@ export type ContextMenuMethods = { registerItems: (items: T[]) => (trigger: HTMLElement | null) => void; }; -export function useContextMenu({ +export function useContextMenu({ Menu, }: { Menu: React.ComponentType<{ From c2d9ac141d35f3b6b945d71beb79795072989d79 Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 22 May 2025 12:14:37 +0200 Subject: [PATCH 30/46] fix: use React.RefCallback --- packages/compass-context-menu/src/use-context-menu.tsx | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/compass-context-menu/src/use-context-menu.tsx b/packages/compass-context-menu/src/use-context-menu.tsx index a0c97dead9b..fa9e20e7537 100644 --- a/packages/compass-context-menu/src/use-context-menu.tsx +++ b/packages/compass-context-menu/src/use-context-menu.tsx @@ -12,7 +12,7 @@ export type ContextMenuMethods = { * Register the menu items for the context menu. * @returns a callback ref to be passed onto the element responsible for triggering the menu. */ - registerItems: (items: T[]) => (trigger: HTMLElement | null) => void; + registerItems: (items: T[]) => React.RefCallback; }; export function useContextMenu({ @@ -33,7 +33,9 @@ export function useContextMenu({ throw new Error('useContextMenu called outside of the provider'); } - const register = (content: React.ComponentType) => { + const register = ( + content: React.ComponentType + ): React.RefCallback => { function listener(event: MouseEvent) { appendContextMenuContent(event, content); } From 1c6ef03024612cf493f60fe3a124748a39e8dd47 Mon Sep 17 00:00:00 2001 From: gagik Date: Fri, 23 May 2025 13:50:50 +0200 Subject: [PATCH 31/46] refactor: use item groups instead of React elements, use wrapper, keep menu state consistent The refactor is meant to make the Leafygreen integration more straightforward: 1. We stick to item groups and instead have a single wrapper to handle any rendering differences between groups. This allows the wrapper to always have context of all items when rendering which is useful when inserting menu seperators in Leafygreen. Also encourages consistent UI (while allowing per-case customization if needed at wrapper-level). We could introduce itemWrappers instead of itemGroups but having one wrapper handling all seems cleaner to me. 2. More of the responsibility is moved to a parent wrapper component that will house the context menu. This allows us to standardize the right click menu and make better use of Leafygreen's menu component including its click handling (which has been removed from the context menu library). 3. Menu state (i.e. position) is now preserved even closed; this is useful for leafygreen's menu to animate in the same position instead of losing the position all together. --- .../src/context-menu-content.ts | 13 ++-- .../src/context-menu-provider.tsx | 57 ++++++++++------- packages/compass-context-menu/src/index.ts | 7 ++- packages/compass-context-menu/src/types.ts | 29 +++++---- .../src/use-context-menu.spec.tsx | 61 ++++++++++--------- .../src/use-context-menu.tsx | 59 +++++++++--------- 6 files changed, 124 insertions(+), 102 deletions(-) diff --git a/packages/compass-context-menu/src/context-menu-content.ts b/packages/compass-context-menu/src/context-menu-content.ts index 6856f1fe684..c301983a679 100644 --- a/packages/compass-context-menu/src/context-menu-content.ts +++ b/packages/compass-context-menu/src/context-menu-content.ts @@ -1,23 +1,24 @@ +import type { ContextMenuItemGroup } from './types'; + const CONTEXT_MENUS_SYMBOL = Symbol('context_menus'); export type EnhancedMouseEvent = MouseEvent & { - [CONTEXT_MENUS_SYMBOL]?: React.ComponentType[]; + [CONTEXT_MENUS_SYMBOL]?: ContextMenuItemGroup[]; }; export function getContextMenuContent( event: EnhancedMouseEvent -): React.ComponentType[] { +): ContextMenuItemGroup[] { return event[CONTEXT_MENUS_SYMBOL] ?? []; } export function appendContextMenuContent( event: EnhancedMouseEvent, - content: React.ComponentType + content: ContextMenuItemGroup ) { // Initialize if not already patched - if (event[CONTEXT_MENUS_SYMBOL] === undefined) { - event[CONTEXT_MENUS_SYMBOL] = [content]; - return; + if (!event[CONTEXT_MENUS_SYMBOL]) { + event[CONTEXT_MENUS_SYMBOL] = []; } event[CONTEXT_MENUS_SYMBOL].push(content); } diff --git a/packages/compass-context-menu/src/context-menu-provider.tsx b/packages/compass-context-menu/src/context-menu-provider.tsx index 43d9e893367..5dd1cba2cff 100644 --- a/packages/compass-context-menu/src/context-menu-provider.tsx +++ b/packages/compass-context-menu/src/context-menu-provider.tsx @@ -5,8 +5,7 @@ import React, { useMemo, createContext, } from 'react'; -import type { ContextMenuContext, MenuState } from './types'; -import { ContextMenu } from './context-menu'; +import type { ContextMenuContext, ContextMenuState } from './types'; import type { EnhancedMouseEvent } from './context-menu-content'; import { getContextMenuContent } from './context-menu-content'; @@ -14,20 +13,42 @@ export const Context = createContext(null); export function ContextMenuProvider({ children, + wrapper, }: { children: React.ReactNode; + wrapper: React.ComponentType<{ + menu: ContextMenuState & { close: () => void }; + }>; }) { - const [menu, setMenu] = useState({ isOpen: false }); - const close = useCallback(() => setMenu({ isOpen: false }), [setMenu]); + const [menu, setMenu] = useState({ + isOpen: false, + itemGroups: [], + position: { x: 0, y: 0 }, + }); + const close = useCallback(() => setMenu({ ...menu, isOpen: false }), [menu]); + + const handleClosingEvent = useCallback( + (event: Event) => { + if (!event.defaultPrevented) { + setMenu({ ...menu, isOpen: false }); + } + }, + [menu] + ); useEffect(() => { function handleContextMenu(event: MouseEvent) { event.preventDefault(); + + const itemGroups = getContextMenuContent(event as EnhancedMouseEvent); + + if (itemGroups.length === 0) { + return; + } + setMenu({ isOpen: true, - children: getContextMenuContent(event as EnhancedMouseEvent).map( - (Content, index) => - ), + itemGroups, position: { // TODO: Fix handling offset while scrolling x: event.clientX, @@ -36,22 +57,14 @@ export function ContextMenuProvider({ }); } - function handleClosingEvent(event: Event) { - if (!event.defaultPrevented) { - setMenu({ isOpen: false }); - } - } - document.addEventListener('contextmenu', handleContextMenu); - document.addEventListener('click', handleClosingEvent); window.addEventListener('resize', handleClosingEvent); return () => { document.removeEventListener('contextmenu', handleContextMenu); - document.removeEventListener('click', handleClosingEvent); window.removeEventListener('resize', handleClosingEvent); }; - }, [setMenu]); + }, [handleClosingEvent]); const value = useMemo( () => ({ @@ -60,12 +73,12 @@ export function ContextMenuProvider({ [close] ); + const Wrapper = wrapper ?? React.Fragment; + return ( - <> - {children} - {menu.isOpen && ( - {menu.children} - )} - + + {children} + + ); } diff --git a/packages/compass-context-menu/src/index.ts b/packages/compass-context-menu/src/index.ts index d60a97e04b3..75d933ef767 100644 --- a/packages/compass-context-menu/src/index.ts +++ b/packages/compass-context-menu/src/index.ts @@ -1,2 +1,7 @@ export { useContextMenu } from './use-context-menu'; -export type { ContextMenuItem } from './types'; +export { ContextMenuProvider } from './context-menu-provider'; +export type { + ContextMenuItem, + ContextMenuItemGroup, + ContextMenuWrapperProps, +} from './types'; diff --git a/packages/compass-context-menu/src/types.ts b/packages/compass-context-menu/src/types.ts index f453930dcb3..91e8d65cdcc 100644 --- a/packages/compass-context-menu/src/types.ts +++ b/packages/compass-context-menu/src/types.ts @@ -1,15 +1,20 @@ -export type MenuState = - | { - isOpen: false; - } - | { - isOpen: true; - children: React.ReactNode; - position: { - x: number; - y: number; - }; - }; +export interface ContextMenuItemGroup { + items: ContextMenuItem[]; + originListener: (event: MouseEvent) => void; +} + +export type ContextMenuState = { + isOpen: boolean; + itemGroups: ContextMenuItemGroup[]; + position: { + x: number; + y: number; + }; +}; + +export type ContextMenuWrapperProps = { + menu: ContextMenuState & { close: () => void }; +}; export type ContextMenuContext = { close(): void; diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx index eb857db9aeb..cfe0bfc7ef4 100644 --- a/packages/compass-context-menu/src/use-context-menu.spec.tsx +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -4,27 +4,29 @@ import { expect } from 'chai'; import sinon from 'sinon'; import { useContextMenu } from './use-context-menu'; import { ContextMenuProvider } from './context-menu-provider'; -import type { ContextMenuItem } from './types'; +import type { ContextMenuItem, ContextMenuWrapperProps } from './types'; describe('useContextMenu', function () { - const TestMenu: React.FC<{ items: ContextMenuItem[] }> = ({ items }) => ( + const TestMenu: React.FC = ({ menu }) => (
- {items.map((item, idx) => ( -
item.onAction?.(event)} - onKeyDown={(event) => { - if (event.key === 'Enter') { - item.onAction?.(event); - } - }} - > - {item.label} -
- ))} + {menu.itemGroups.flatMap((group, groupIdx) => + group.items.map((item, idx) => ( +
item.onAction?.(event)} + onKeyDown={(event) => { + if (event.key === 'Enter') { + item.onAction?.(event); + } + }} + > + {item.label} +
+ )) + )}
); @@ -33,9 +35,9 @@ describe('useContextMenu', function () { onAction, }: { onRegister?: (ref: any) => void; - onAction?: (id) => void; + onAction?: (id: number) => void; }) => { - const contextMenu = useContextMenu({ Menu: TestMenu }); + const contextMenu = useContextMenu(); const items: ContextMenuItem[] = [ { label: 'Test Item', @@ -55,7 +57,6 @@ describe('useContextMenu', function () { ); }; - // Add new test components for nested context menu scenario const ParentComponent = ({ onAction, children, @@ -63,7 +64,7 @@ describe('useContextMenu', function () { onAction?: (id: number) => void; children?: React.ReactNode; }) => { - const contextMenu = useContextMenu({ Menu: TestMenu }); + const contextMenu = useContextMenu(); const parentItems: ContextMenuItem[] = [ { label: 'Parent Item 1', @@ -89,7 +90,7 @@ describe('useContextMenu', function () { }: { onAction?: (id: number) => void; }) => { - const contextMenu = useContextMenu({ Menu: TestMenu }); + const contextMenu = useContextMenu(); const childItems: ContextMenuItem[] = [ { label: 'Child Item 1', @@ -135,7 +136,7 @@ describe('useContextMenu', function () { it('renders without error', function () { render( - + ); @@ -147,7 +148,7 @@ describe('useContextMenu', function () { const onRegister = sinon.spy(); render( - + ); @@ -158,7 +159,7 @@ describe('useContextMenu', function () { it('shows context menu on right click', function () { render( - + ); @@ -173,7 +174,7 @@ describe('useContextMenu', function () { describe('with nested context menus', function () { it('shows only parent items when right clicking parent area', function () { render( - + ); @@ -192,7 +193,7 @@ describe('useContextMenu', function () { it('shows both parent and child items when right clicking child area', function () { render( - + @@ -214,7 +215,7 @@ describe('useContextMenu', function () { const childOnAction = sinon.spy(); render( - + @@ -237,7 +238,7 @@ describe('useContextMenu', function () { const childOnAction = sinon.spy(); render( - + diff --git a/packages/compass-context-menu/src/use-context-menu.tsx b/packages/compass-context-menu/src/use-context-menu.tsx index fa9e20e7537..a60aeba4c69 100644 --- a/packages/compass-context-menu/src/use-context-menu.tsx +++ b/packages/compass-context-menu/src/use-context-menu.tsx @@ -1,4 +1,5 @@ -import React, { useContext, useMemo, useRef } from 'react'; +import type { RefCallback } from 'react'; +import { useContext, useMemo, useRef } from 'react'; import { Context } from './context-menu-provider'; import { appendContextMenuContent } from './context-menu-content'; import type { ContextMenuItem } from './types'; @@ -12,17 +13,12 @@ export type ContextMenuMethods = { * Register the menu items for the context menu. * @returns a callback ref to be passed onto the element responsible for triggering the menu. */ - registerItems: (items: T[]) => React.RefCallback; + registerItems: (items: T[]) => RefCallback; }; -export function useContextMenu({ - Menu, -}: { - Menu: React.ComponentType<{ - items: T[]; - }>; -}): ContextMenuMethods { - // Get the close function from the ContextProvider +export function useContextMenu< + T extends ContextMenuItem = ContextMenuItem +>(): ContextMenuMethods { const context = useContext(Context); const previous = useRef void]>( null @@ -33,32 +29,33 @@ export function useContextMenu({ throw new Error('useContextMenu called outside of the provider'); } - const register = ( - content: React.ComponentType - ): React.RefCallback => { - function listener(event: MouseEvent) { - appendContextMenuContent(event, content); - } - return (trigger: HTMLElement | null) => { - if (previous.current) { - const [previousTrigger, previousListener] = previous.current; - previousTrigger.removeEventListener('contextmenu', previousListener); - } - if (trigger) { - trigger.addEventListener('contextmenu', listener); - previous.current = [trigger, listener]; - } - }; - }; - return { close: context.close.bind(context), /** * @returns a callback ref, passed onto the element responsible for triggering the menu. */ - registerItems(items: T[]) { - return register(() => ); + registerItems(items: ContextMenuItem[]) { + function listener(event: MouseEvent): void { + appendContextMenuContent(event, { + items, + originListener: listener, + }); + } + + return (trigger: HTMLElement | null) => { + if (previous.current) { + const [previousTrigger, previousListener] = previous.current; + previousTrigger.removeEventListener( + 'contextmenu', + previousListener + ); + } + if (trigger) { + trigger.addEventListener('contextmenu', listener); + previous.current = [trigger, listener]; + } + }; }, }; - }, [context, Menu]); + }, [context]); } From 3d14d6df00a8c1909853f878bf086fb6bcda7323 Mon Sep 17 00:00:00 2001 From: gagik Date: Fri, 23 May 2025 17:12:31 +0200 Subject: [PATCH 32/46] fix: delete redundant context menu --- .../compass-context-menu/src/context-menu.tsx | 22 ------------------- 1 file changed, 22 deletions(-) delete mode 100644 packages/compass-context-menu/src/context-menu.tsx diff --git a/packages/compass-context-menu/src/context-menu.tsx b/packages/compass-context-menu/src/context-menu.tsx deleted file mode 100644 index b053bc4963a..00000000000 --- a/packages/compass-context-menu/src/context-menu.tsx +++ /dev/null @@ -1,22 +0,0 @@ -import { createPortal } from 'react-dom'; -import React from 'react'; - -type ContextMenuProps = React.PropsWithChildren<{ - position: { - x: number; - y: number; - }; -}>; - -export function ContextMenu({ children, position }: ContextMenuProps) { - const container = document.getElementById('context-menu-container'); - if (container === null) { - throw new Error('Expected a container for the context menu in the DOM'); - } - return createPortal( -
- {children} -
, - container - ); -} From ee658e1f394f49713edb63647193dc0641bcaaeb Mon Sep 17 00:00:00 2001 From: gagik Date: Mon, 2 Jun 2025 13:51:48 +0200 Subject: [PATCH 33/46] fix: remove unused dep --- package-lock.json | 4 ++-- packages/compass-context-menu/package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index a205c4ec913..57ab1929a9d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -43216,11 +43216,11 @@ "@mongodb-js/eslint-config-compass": "^1.3.8", "@mongodb-js/mocha-config-compass": "^1.6.8", "@mongodb-js/prettier-config-compass": "^1.2.8", + "@mongodb-js/testing-library-compass": "^1.3.1", "@mongodb-js/tsconfig-compass": "^1.2.8", "@types/chai": "^4.2.21", "@types/mocha": "^9.0.0", "@types/react": "^17.0.5", - "@types/react-dom": "^17.0.10", "@types/sinon-chai": "^3.2.5", "chai": "^4.3.6", "depcheck": "^1.4.1", @@ -55712,11 +55712,11 @@ "@mongodb-js/eslint-config-compass": "^1.3.8", "@mongodb-js/mocha-config-compass": "^1.6.8", "@mongodb-js/prettier-config-compass": "^1.2.8", + "@mongodb-js/testing-library-compass": "^1.3.1", "@mongodb-js/tsconfig-compass": "^1.2.8", "@types/chai": "^4.2.21", "@types/mocha": "^9.0.0", "@types/react": "^17.0.5", - "@types/react-dom": "^17.0.10", "@types/sinon-chai": "^3.2.5", "chai": "^4.3.6", "depcheck": "^1.4.1", diff --git a/packages/compass-context-menu/package.json b/packages/compass-context-menu/package.json index 3cf186e0270..67099115f92 100644 --- a/packages/compass-context-menu/package.json +++ b/packages/compass-context-menu/package.json @@ -55,11 +55,11 @@ "@mongodb-js/eslint-config-compass": "^1.3.8", "@mongodb-js/mocha-config-compass": "^1.6.8", "@mongodb-js/prettier-config-compass": "^1.2.8", + "@mongodb-js/testing-library-compass": "^1.3.1", "@mongodb-js/tsconfig-compass": "^1.2.8", "@types/chai": "^4.2.21", "@types/mocha": "^9.0.0", "@types/react": "^17.0.5", - "@types/react-dom": "^17.0.10", "@types/sinon-chai": "^3.2.5", "chai": "^4.3.6", "depcheck": "^1.4.1", From 4730c189e9b0ba768e7b772d4d3ffd7d5bfe5818 Mon Sep 17 00:00:00 2001 From: gagik Date: Tue, 10 Jun 2025 17:15:25 +0200 Subject: [PATCH 34/46] fix: add tests and fix bug with menu auto-closing --- .../src/components/context-menu.tsx | 2 +- .../src/context-menu-provider.spec.tsx | 45 +++++++++++++++++++ .../src/context-menu-provider.tsx | 18 +++++++- 3 files changed, 63 insertions(+), 2 deletions(-) create mode 100644 packages/compass-context-menu/src/context-menu-provider.spec.tsx diff --git a/packages/compass-components/src/components/context-menu.tsx b/packages/compass-components/src/components/context-menu.tsx index c78519e399c..288db7393a5 100644 --- a/packages/compass-components/src/components/context-menu.tsx +++ b/packages/compass-components/src/components/context-menu.tsx @@ -28,7 +28,7 @@ export function ContextMenu({ menu }: ContextMenuWrapperProps) { if (!menu.isOpen) { menu.close(); } - }, [menu, menu.isOpen]); + }, [menu.isOpen]); return (
= () => ( +
Test Menu
+ ); + + const TestComponent = () => ( +
Test Content
+ ); + + describe('when nested', function () { + it('throws an error when providers are nested', function () { + expect(() => { + render( + +
+ + + +
+ + ); + }).to.throw( + 'Duplicated ContextMenuProvider found. Please remove the nested provider.' + ); + }); + }); + + describe('when not nested', function () { + it('renders without error', function () { + render( + + + + ); + + expect(document.querySelector('[data-testid="test-content"]')).to.exist; + }); + }); +}); diff --git a/packages/compass-context-menu/src/context-menu-provider.tsx b/packages/compass-context-menu/src/context-menu-provider.tsx index 5dd1cba2cff..0ae7c5f0e06 100644 --- a/packages/compass-context-menu/src/context-menu-provider.tsx +++ b/packages/compass-context-menu/src/context-menu-provider.tsx @@ -4,6 +4,7 @@ import React, { useState, useMemo, createContext, + useContext, } from 'react'; import type { ContextMenuContext, ContextMenuState } from './types'; import type { EnhancedMouseEvent } from './context-menu-content'; @@ -20,6 +21,9 @@ export function ContextMenuProvider({ menu: ContextMenuState & { close: () => void }; }>; }) { + // Check if there's already a parent context menu provider + const parentContext = useContext(Context); + const [menu, setMenu] = useState({ isOpen: false, itemGroups: [], @@ -37,6 +41,11 @@ export function ContextMenuProvider({ ); useEffect(() => { + // If there's a parent provider, don't add event listeners + if (parentContext) { + return; + } + function handleContextMenu(event: MouseEvent) { event.preventDefault(); @@ -64,7 +73,7 @@ export function ContextMenuProvider({ document.removeEventListener('contextmenu', handleContextMenu); window.removeEventListener('resize', handleClosingEvent); }; - }, [handleClosingEvent]); + }, [handleClosingEvent, parentContext]); const value = useMemo( () => ({ @@ -73,6 +82,13 @@ export function ContextMenuProvider({ [close] ); + // Prevent accidental nested providers + if (parentContext) { + throw new Error( + 'Duplicated ContextMenuProvider found. Please remove the nested provider.' + ); + } + const Wrapper = wrapper ?? React.Fragment; return ( From eb657699844edde0af7ba583f8fa153ef08f6fb2 Mon Sep 17 00:00:00 2001 From: gagik Date: Tue, 10 Jun 2025 17:13:08 +0200 Subject: [PATCH 35/46] fix: enforce no nesting, adjsut enzyme test and move setup to testing-library --- configs/testing-library-compass/src/index.tsx | 77 ++++++++++--------- .../components/document-list-view.spec.tsx | 30 ++++---- 2 files changed, 58 insertions(+), 49 deletions(-) diff --git a/configs/testing-library-compass/src/index.tsx b/configs/testing-library-compass/src/index.tsx index 6f3bf3bb79d..d01c55bba42 100644 --- a/configs/testing-library-compass/src/index.tsx +++ b/configs/testing-library-compass/src/index.tsx @@ -45,7 +45,10 @@ import { ReadOnlyPreferenceAccess, } from 'compass-preferences-model/provider'; import { TelemetryProvider } from '@mongodb-js/compass-telemetry/provider'; -import { CompassComponentsProvider } from '@mongodb-js/compass-components'; +import { + CompassComponentsProvider, + ContextMenuProvider, +} from '@mongodb-js/compass-components'; import { TestEnvCurrentConnectionContext, ConnectionInfoProvider, @@ -349,41 +352,43 @@ function createWrapper( - - - { - // noop - }) - } - onExtraConnectionDataRequest={ - options.onExtraConnectionDataRequest ?? - (() => { - return Promise.resolve([{}, null] as [any, null]); - }) - } - onAutoconnectInfoRequest={ - options.onAutoconnectInfoRequest - } - preloadStorageConnectionInfos={connections} - > - - - - {children} - - - - - - + + + + { + // noop + }) + } + onExtraConnectionDataRequest={ + options.onExtraConnectionDataRequest ?? + (() => { + return Promise.resolve([{}, null] as [any, null]); + }) + } + onAutoconnectInfoRequest={ + options.onAutoconnectInfoRequest + } + preloadStorageConnectionInfos={connections} + > + + + + {children} + + + + + + + diff --git a/packages/compass-crud/src/components/document-list-view.spec.tsx b/packages/compass-crud/src/components/document-list-view.spec.tsx index 536f48e66c2..ed257306567 100644 --- a/packages/compass-crud/src/components/document-list-view.spec.tsx +++ b/packages/compass-crud/src/components/document-list-view.spec.tsx @@ -1,28 +1,32 @@ import React from 'react'; import { mount } from 'enzyme'; +import type { ReactWrapper } from 'enzyme'; import HadronDocument from 'hadron-document'; import { expect } from 'chai'; -import sinon from 'sinon'; import DocumentListView from './document-list-view'; +import { ContextMenuProvider } from '@mongodb-js/compass-components'; describe('', function () { describe('#render', function () { context('when the documents have objects for ids', function () { const docs = [{ _id: { name: 'test-1' } }, { _id: { name: 'test-2' } }]; const hadronDocs = docs.map((doc) => new HadronDocument(doc)); - const component = mount( - - ); + let component: ReactWrapper; + beforeEach(function () { + component = mount( + , + { wrappingComponent: ContextMenuProvider } + ); + }); + + afterEach(function () { + component?.unmount(); + }); it('renders all the documents', function () { const wrapper = component.find('[data-testid="readonly-document"]'); From 220a300c065917a7ee4ef4f289dedda782e9987e Mon Sep 17 00:00:00 2001 From: gagik Date: Tue, 10 Jun 2025 18:04:57 +0200 Subject: [PATCH 36/46] fix: move to compass components provider --- configs/testing-library-compass/src/index.tsx | 82 ++++++++----------- .../compass-components-provider.tsx | 21 +++-- packages/compass-components/src/index.ts | 5 +- .../compass/src/app/components/entrypoint.tsx | 4 +- 4 files changed, 50 insertions(+), 62 deletions(-) diff --git a/configs/testing-library-compass/src/index.tsx b/configs/testing-library-compass/src/index.tsx index d01c55bba42..3739137c602 100644 --- a/configs/testing-library-compass/src/index.tsx +++ b/configs/testing-library-compass/src/index.tsx @@ -45,10 +45,7 @@ import { ReadOnlyPreferenceAccess, } from 'compass-preferences-model/provider'; import { TelemetryProvider } from '@mongodb-js/compass-telemetry/provider'; -import { - CompassComponentsProvider, - ContextMenuProvider, -} from '@mongodb-js/compass-components'; +import { CompassComponentsProvider } from '@mongodb-js/compass-components'; import { TestEnvCurrentConnectionContext, ConnectionInfoProvider, @@ -352,43 +349,41 @@ function createWrapper( - - - - { - // noop - }) - } - onExtraConnectionDataRequest={ - options.onExtraConnectionDataRequest ?? - (() => { - return Promise.resolve([{}, null] as [any, null]); - }) - } - onAutoconnectInfoRequest={ - options.onAutoconnectInfoRequest - } - preloadStorageConnectionInfos={connections} - > - - - - {children} - - - - - - - + + + { + // noop + }) + } + onExtraConnectionDataRequest={ + options.onExtraConnectionDataRequest ?? + (() => { + return Promise.resolve([{}, null] as [any, null]); + }) + } + onAutoconnectInfoRequest={ + options.onAutoconnectInfoRequest + } + preloadStorageConnectionInfos={connections} + > + + + + {children} + + + + + + @@ -756,11 +751,6 @@ function createPluginTestHelpers< */ const fireEvent = testingLibraryFireEvent; -/** - * @deprecated @testing-library/react installs these hooks automatically - */ -const cleanup = rtlCleanup; - /** * @deprecated @testing-library/react-hooks installs these hooks automatically */ diff --git a/packages/compass-components/src/components/compass-components-provider.tsx b/packages/compass-components/src/components/compass-components-provider.tsx index 18bac239d30..7a2fdf20117 100644 --- a/packages/compass-components/src/components/compass-components-provider.tsx +++ b/packages/compass-components/src/components/compass-components-provider.tsx @@ -6,6 +6,7 @@ import { GuideCueProvider } from './guide-cue/guide-cue'; import { SignalHooksProvider } from './signal-popover'; import { RequiredURLSearchParamsProvider } from './links/link'; import { StackedComponentProvider } from '../hooks/use-stacked-component'; +import { ContextMenuProvider } from './context-menu'; type GuideCueProviderProps = React.ComponentProps; @@ -135,15 +136,17 @@ export const CompassComponentsProvider = ({ > - - {typeof children === 'function' - ? children({ - darkMode, - portalContainerRef: setPortalContainer, - scrollContainerRef: setScrollContainer, - }) - : children} - + + + {typeof children === 'function' + ? children({ + darkMode, + portalContainerRef: setPortalContainer, + scrollContainerRef: setScrollContainer, + }) + : children} + + diff --git a/packages/compass-components/src/index.ts b/packages/compass-components/src/index.ts index e6f8068c130..69ce000a74f 100644 --- a/packages/compass-components/src/index.ts +++ b/packages/compass-components/src/index.ts @@ -93,10 +93,7 @@ export { ModalHeader } from './components/modals/modal-header'; export { FormModal } from './components/modals/form-modal'; export { InfoModal } from './components/modals/info-modal'; -export { - ContextMenuProvider, - useContextMenuItems, -} from './components/context-menu'; +export { useContextMenuItems } from './components/context-menu'; export type { FileInputBackend, diff --git a/packages/compass/src/app/components/entrypoint.tsx b/packages/compass/src/app/components/entrypoint.tsx index 866b3414188..28a79b35c37 100644 --- a/packages/compass/src/app/components/entrypoint.tsx +++ b/packages/compass/src/app/components/entrypoint.tsx @@ -102,9 +102,7 @@ export const CompassElectron = (props: React.ComponentProps) => { - - - + From 74f3d6ebacbc766f2987762dd8be6afe02b7f009 Mon Sep 17 00:00:00 2001 From: gagik Date: Tue, 10 Jun 2025 18:07:30 +0200 Subject: [PATCH 37/46] fix: remove unintended deletion --- configs/testing-library-compass/src/index.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/configs/testing-library-compass/src/index.tsx b/configs/testing-library-compass/src/index.tsx index 3739137c602..6f3bf3bb79d 100644 --- a/configs/testing-library-compass/src/index.tsx +++ b/configs/testing-library-compass/src/index.tsx @@ -751,6 +751,11 @@ function createPluginTestHelpers< */ const fireEvent = testingLibraryFireEvent; +/** + * @deprecated @testing-library/react installs these hooks automatically + */ +const cleanup = rtlCleanup; + /** * @deprecated @testing-library/react-hooks installs these hooks automatically */ From 4f05381b57cb2c9f62f9b224d35a5d3869cb2648 Mon Sep 17 00:00:00 2001 From: gagik Date: Tue, 10 Jun 2025 18:30:27 +0200 Subject: [PATCH 38/46] fix: adjust tests --- .../components/content-with-fallback.spec.tsx | 5 +- .../src/components/context-menu.spec.tsx | 210 +++++++++--------- .../src/components/context-menu.tsx | 1 + .../compass/src/app/components/entrypoint.tsx | 1 - 4 files changed, 104 insertions(+), 113 deletions(-) diff --git a/packages/compass-components/src/components/content-with-fallback.spec.tsx b/packages/compass-components/src/components/content-with-fallback.spec.tsx index bd4daa3861c..5b989316f5b 100644 --- a/packages/compass-components/src/components/content-with-fallback.spec.tsx +++ b/packages/compass-components/src/components/content-with-fallback.spec.tsx @@ -58,7 +58,10 @@ describe('ContentWithFallback', function () { { container } ); - expect(container).to.be.empty; + expect(container.children.length).to.equal(1); + expect(container.children[0].getAttribute('data-testid')).to.equal( + 'context-menu' + ); }); it('should render fallback when the timeout passes', async function () { diff --git a/packages/compass-components/src/components/context-menu.spec.tsx b/packages/compass-components/src/components/context-menu.spec.tsx index 14177575d9d..d2ecaa63cdf 100644 --- a/packages/compass-components/src/components/context-menu.spec.tsx +++ b/packages/compass-components/src/components/context-menu.spec.tsx @@ -28,40 +28,88 @@ describe('useContextMenuItems', function () { ); }; - describe('when used outside provider', function () { - it('throws an error', function () { - const items = [ - { - label: 'Test Item', - onAction: () => {}, - }, - ]; - - expect(() => { - render(); - }).to.throw('useContextMenu called outside of the provider'); - }); - }); - - describe('with a valid provider', function () { - it('renders without error', function () { - const items = [ - { - label: 'Test Item', - onAction: () => {}, - }, - ]; - + it('errors if the component is double wrapped', function () { + const items = [ + { + label: 'Test Item', + onAction: () => {}, + }, + ]; + + expect(() => { render( ); + }).to.throw( + 'Duplicated ContextMenuProvider found. Please remove the nested provider.' + ); + }); - expect(screen.getByTestId(menuTestTriggerId)).to.exist; - }); + it('renders without error', function () { + const items = [ + { + label: 'Test Item', + onAction: () => {}, + }, + ]; + + render(); + + expect(screen.getByTestId(menuTestTriggerId)).to.exist; + }); + + it('shows context menu with items on right click', function () { + const items = [ + { + label: 'Test Item 1', + onAction: () => {}, + }, + { + label: 'Test Item 2', + onAction: () => {}, + }, + ]; + + render(); + + const trigger = screen.getByTestId(menuTestTriggerId); + userEvent.click(trigger, { button: 2 }); + + // The menu items should be rendered + expect(screen.getByTestId('menu-group-0-item-0')).to.exist; + expect(screen.getByTestId('menu-group-0-item-1')).to.exist; + }); + + it('triggers the correct action when menu item is clicked', function () { + const onAction = sinon.spy(); + const items = [ + { + label: 'Test Item 1', + onAction: () => onAction(1), + }, + { + label: 'Test Item 2', + onAction: () => onAction(2), + }, + ]; + + render(); + + const trigger = screen.getByTestId(menuTestTriggerId); + userEvent.click(trigger, { button: 2 }); - it('shows context menu with items on right click', function () { + const menuItem = screen.getByTestId('menu-group-0-item-1'); + userEvent.click(menuItem); + + expect(onAction).to.have.been.calledOnceWithExactly(2); + }); + + describe('with nested components', function () { + const childTriggerId = 'child-trigger'; + + beforeEach(function () { const items = [ { label: 'Test Item 1', @@ -73,101 +121,41 @@ describe('useContextMenuItems', function () { }, ]; - render( - - - - ); - - const trigger = screen.getByTestId(menuTestTriggerId); - userEvent.click(trigger, { button: 2 }); - - // The menu items should be rendered - expect(screen.getByTestId('menu-group-0-item-0')).to.exist; - expect(screen.getByTestId('menu-group-0-item-1')).to.exist; - }); - - it('triggers the correct action when menu item is clicked', function () { - const onAction = sinon.spy(); - const items = [ + const childItems = [ { - label: 'Test Item 1', - onAction: () => onAction(1), - }, - { - label: 'Test Item 2', - onAction: () => onAction(2), + label: 'Child Item 1', + onAction: () => {}, }, ]; render( - - - + + + ); + }); - const trigger = screen.getByTestId(menuTestTriggerId); + it('renders menu items with separators', function () { + const trigger = screen.getByTestId(childTriggerId); userEvent.click(trigger, { button: 2 }); - const menuItem = screen.getByTestId('menu-group-0-item-1'); - userEvent.click(menuItem); + // Should find the menu item and the separator + expect(screen.getByTestId('menu-group-0').children.length).to.equal(2); + expect( + screen.getByTestId('menu-group-0').children.item(0)?.textContent + ).to.equal('Child Item 1'); - expect(onAction).to.have.been.calledOnceWithExactly(2); - }); + expect(screen.getByTestId('menu-group-0-separator')).to.exist; + + expect(screen.getByTestId('menu-group-1').children.length).to.equal(2); + expect( + screen.getByTestId('menu-group-1').children.item(0)?.textContent + ).to.equal('Test Item 1'); + expect( + screen.getByTestId('menu-group-1').children.item(1)?.textContent + ).to.equal('Test Item 2'); - describe('with nested components', function () { - const childTriggerId = 'child-trigger'; - - beforeEach(function () { - const items = [ - { - label: 'Test Item 1', - onAction: () => {}, - }, - { - label: 'Test Item 2', - onAction: () => {}, - }, - ]; - - const childItems = [ - { - label: 'Child Item 1', - onAction: () => {}, - }, - ]; - - render( - - - - - - ); - }); - - it('renders menu items with separators', function () { - const trigger = screen.getByTestId(childTriggerId); - userEvent.click(trigger, { button: 2 }); - - // Should find the menu item and the separator - expect(screen.getByTestId('menu-group-0').children.length).to.equal(2); - expect( - screen.getByTestId('menu-group-0').children.item(0)?.textContent - ).to.equal('Child Item 1'); - - expect(screen.getByTestId('menu-group-0-separator')).to.exist; - - expect(screen.getByTestId('menu-group-1').children.length).to.equal(2); - expect( - screen.getByTestId('menu-group-1').children.item(0)?.textContent - ).to.equal('Test Item 1'); - expect( - screen.getByTestId('menu-group-1').children.item(1)?.textContent - ).to.equal('Test Item 2'); - - expect(screen.queryByTestId('menu-group-1-separator')).not.to.exist; - }); + expect(screen.queryByTestId('menu-group-1-separator')).not.to.exist; }); }); }); diff --git a/packages/compass-components/src/components/context-menu.tsx b/packages/compass-components/src/components/context-menu.tsx index 288db7393a5..c12ff5fdd3e 100644 --- a/packages/compass-components/src/components/context-menu.tsx +++ b/packages/compass-components/src/components/context-menu.tsx @@ -32,6 +32,7 @@ export function ContextMenu({ menu }: ContextMenuWrapperProps) { return (
{ const loggerProviderValue = useRef({ From 6b6e3981b77d32e36ff46b319dfdae3f1babdf1f Mon Sep 17 00:00:00 2001 From: gagik Date: Wed, 18 Jun 2025 11:22:45 +0200 Subject: [PATCH 39/46] fix: throw early on --- .../src/context-menu-provider.tsx | 21 +++++++------------ 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/packages/compass-context-menu/src/context-menu-provider.tsx b/packages/compass-context-menu/src/context-menu-provider.tsx index 0ae7c5f0e06..4c90da83f63 100644 --- a/packages/compass-context-menu/src/context-menu-provider.tsx +++ b/packages/compass-context-menu/src/context-menu-provider.tsx @@ -24,6 +24,13 @@ export function ContextMenuProvider({ // Check if there's already a parent context menu provider const parentContext = useContext(Context); + // Prevent accidental nested providers + if (parentContext) { + throw new Error( + 'Duplicated ContextMenuProvider found. Please remove the nested provider.' + ); + } + const [menu, setMenu] = useState({ isOpen: false, itemGroups: [], @@ -41,11 +48,6 @@ export function ContextMenuProvider({ ); useEffect(() => { - // If there's a parent provider, don't add event listeners - if (parentContext) { - return; - } - function handleContextMenu(event: MouseEvent) { event.preventDefault(); @@ -73,7 +75,7 @@ export function ContextMenuProvider({ document.removeEventListener('contextmenu', handleContextMenu); window.removeEventListener('resize', handleClosingEvent); }; - }, [handleClosingEvent, parentContext]); + }, [handleClosingEvent]); const value = useMemo( () => ({ @@ -82,13 +84,6 @@ export function ContextMenuProvider({ [close] ); - // Prevent accidental nested providers - if (parentContext) { - throw new Error( - 'Duplicated ContextMenuProvider found. Please remove the nested provider.' - ); - } - const Wrapper = wrapper ?? React.Fragment; return ( From 6dadf25564cdfa7708ae962c16f31642e1e53124 Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 19 Jun 2025 10:12:09 +0200 Subject: [PATCH 40/46] fix: correct wrapper use --- .../src/components/context-menu.spec.tsx | 2 +- .../src/components/context-menu.tsx | 2 +- .../src/context-menu-provider.spec.tsx | 6 +++--- .../src/use-context-menu.spec.tsx | 14 +++++++------- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/compass-components/src/components/context-menu.spec.tsx b/packages/compass-components/src/components/context-menu.spec.tsx index d2ecaa63cdf..aa4aee700df 100644 --- a/packages/compass-components/src/components/context-menu.spec.tsx +++ b/packages/compass-components/src/components/context-menu.spec.tsx @@ -38,7 +38,7 @@ describe('useContextMenuItems', function () { expect(() => { render( - + ); diff --git a/packages/compass-components/src/components/context-menu.tsx b/packages/compass-components/src/components/context-menu.tsx index c12ff5fdd3e..0cb8d38b6a3 100644 --- a/packages/compass-components/src/components/context-menu.tsx +++ b/packages/compass-components/src/components/context-menu.tsx @@ -14,7 +14,7 @@ export function ContextMenuProvider({ children: React.ReactNode; }) { return ( - + {children} ); diff --git a/packages/compass-context-menu/src/context-menu-provider.spec.tsx b/packages/compass-context-menu/src/context-menu-provider.spec.tsx index 7e39fdee800..0cd6f23e492 100644 --- a/packages/compass-context-menu/src/context-menu-provider.spec.tsx +++ b/packages/compass-context-menu/src/context-menu-provider.spec.tsx @@ -17,9 +17,9 @@ describe('ContextMenuProvider', function () { it('throws an error when providers are nested', function () { expect(() => { render( - +
- +
@@ -34,7 +34,7 @@ describe('ContextMenuProvider', function () { describe('when not nested', function () { it('renders without error', function () { render( - + ); diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx index cfe0bfc7ef4..fb17fa0e66a 100644 --- a/packages/compass-context-menu/src/use-context-menu.spec.tsx +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -136,7 +136,7 @@ describe('useContextMenu', function () { it('renders without error', function () { render( - + ); @@ -148,7 +148,7 @@ describe('useContextMenu', function () { const onRegister = sinon.spy(); render( - + ); @@ -159,7 +159,7 @@ describe('useContextMenu', function () { it('shows context menu on right click', function () { render( - + ); @@ -174,7 +174,7 @@ describe('useContextMenu', function () { describe('with nested context menus', function () { it('shows only parent items when right clicking parent area', function () { render( - + ); @@ -193,7 +193,7 @@ describe('useContextMenu', function () { it('shows both parent and child items when right clicking child area', function () { render( - + @@ -215,7 +215,7 @@ describe('useContextMenu', function () { const childOnAction = sinon.spy(); render( - + @@ -238,7 +238,7 @@ describe('useContextMenu', function () { const childOnAction = sinon.spy(); render( - + From 4dafa7be32311839954ddecda6ef78b4c10e2374 Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 19 Jun 2025 17:55:34 +0200 Subject: [PATCH 41/46] fix: use render directly for compass-context-menu --- package-lock.json | 5 ++++- packages/compass-components/src/components/context-menu.tsx | 2 +- packages/compass-context-menu/package.json | 1 + .../compass-context-menu/src/context-menu-provider.spec.tsx | 2 +- packages/compass-context-menu/src/render.ts | 5 +++++ packages/compass-context-menu/src/use-context-menu.spec.tsx | 3 ++- 6 files changed, 14 insertions(+), 4 deletions(-) create mode 100644 packages/compass-context-menu/src/render.ts diff --git a/package-lock.json b/package-lock.json index 3bafa1def9b..74c7fb5a9dd 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13059,6 +13059,7 @@ "version": "12.1.5", "resolved": "https://registry.npmjs.org/@testing-library/react/-/react-12.1.5.tgz", "integrity": "sha512-OfTXCJUFgjd/digLUuPxa0+/3ZxsQmE7ub9kcbW/wi96Bh3o/p5vrETcBGfP17NWPGqeYYl5LTRpwyGoMC4ysg==", + "license": "MIT", "dependencies": { "@babel/runtime": "^7.12.5", "@testing-library/dom": "^8.0.0", @@ -43994,6 +43995,7 @@ "@mongodb-js/prettier-config-compass": "^1.2.8", "@mongodb-js/testing-library-compass": "^1.3.1", "@mongodb-js/tsconfig-compass": "^1.2.8", + "@testing-library/react": "^12.1.5", "@types/chai": "^4.2.21", "@types/mocha": "^9.0.0", "@types/react": "^17.0.5", @@ -57130,6 +57132,7 @@ "@mongodb-js/prettier-config-compass": "^1.2.8", "@mongodb-js/testing-library-compass": "^1.3.1", "@mongodb-js/tsconfig-compass": "^1.2.8", + "@testing-library/react": "^12.1.5", "@types/chai": "^4.2.21", "@types/mocha": "^9.0.0", "@types/react": "^17.0.5", @@ -64989,7 +64992,7 @@ "requires": { "@babel/runtime": "^7.12.5", "@testing-library/dom": "^8.0.0", - "@types/react-dom": "^17.0.25" + "@types/react-dom": "<18.0.0" } }, "@testing-library/react-hooks": { diff --git a/packages/compass-components/src/components/context-menu.tsx b/packages/compass-components/src/components/context-menu.tsx index 0cb8d38b6a3..b15715e128c 100644 --- a/packages/compass-components/src/components/context-menu.tsx +++ b/packages/compass-components/src/components/context-menu.tsx @@ -6,7 +6,7 @@ import { ContextMenuProvider as ContextMenuProviderBase } from '@mongodb-js/comp import type { ContextMenuItemGroup, ContextMenuWrapperProps, -} from '@mongodb-js/compass-context-menu/dist/types'; +} from '@mongodb-js/compass-context-menu'; export function ContextMenuProvider({ children, diff --git a/packages/compass-context-menu/package.json b/packages/compass-context-menu/package.json index 67099115f92..b810c1a2aff 100644 --- a/packages/compass-context-menu/package.json +++ b/packages/compass-context-menu/package.json @@ -57,6 +57,7 @@ "@mongodb-js/prettier-config-compass": "^1.2.8", "@mongodb-js/testing-library-compass": "^1.3.1", "@mongodb-js/tsconfig-compass": "^1.2.8", + "@testing-library/react": "^12.1.5", "@types/chai": "^4.2.21", "@types/mocha": "^9.0.0", "@types/react": "^17.0.5", diff --git a/packages/compass-context-menu/src/context-menu-provider.spec.tsx b/packages/compass-context-menu/src/context-menu-provider.spec.tsx index 0cd6f23e492..3173b88f7f6 100644 --- a/packages/compass-context-menu/src/context-menu-provider.spec.tsx +++ b/packages/compass-context-menu/src/context-menu-provider.spec.tsx @@ -1,5 +1,5 @@ import React from 'react'; -import { render } from '@mongodb-js/testing-library-compass'; +import { render } from './render'; import { expect } from 'chai'; import { ContextMenuProvider } from './context-menu-provider'; import type { ContextMenuWrapperProps } from './types'; diff --git a/packages/compass-context-menu/src/render.ts b/packages/compass-context-menu/src/render.ts new file mode 100644 index 00000000000..35e6c857ea9 --- /dev/null +++ b/packages/compass-context-menu/src/render.ts @@ -0,0 +1,5 @@ +// We need to import from testing-library/react directly because the wrapping done +// by testing-library-compass already sets up the context menu provider which is not +// useful for our tests. +// eslint-disable-next-line @typescript-eslint/no-restricted-imports +export { render } from '@testing-library/react'; diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx index fb17fa0e66a..fadaffd73a6 100644 --- a/packages/compass-context-menu/src/use-context-menu.spec.tsx +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -1,5 +1,6 @@ import React from 'react'; -import { render, screen, userEvent } from '@mongodb-js/testing-library-compass'; +import { render } from './render'; +import { screen, userEvent } from '@mongodb-js/testing-library-compass'; import { expect } from 'chai'; import sinon from 'sinon'; import { useContextMenu } from './use-context-menu'; From 915c59763c60398ca89103d673b78cdc039b03ff Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 19 Jun 2025 18:21:53 +0200 Subject: [PATCH 42/46] fix: use testingLibrary's render This reverts commit 4dafa7be32311839954ddecda6ef78b4c10e2374 and uses testing-library instead. --- package-lock.json | 5 +---- packages/compass-context-menu/package.json | 1 - .../src/context-menu-provider.spec.tsx | 5 ++++- packages/compass-context-menu/src/render.ts | 5 ----- .../compass-context-menu/src/use-context-menu.spec.tsx | 10 ++++++++-- 5 files changed, 13 insertions(+), 13 deletions(-) delete mode 100644 packages/compass-context-menu/src/render.ts diff --git a/package-lock.json b/package-lock.json index 74c7fb5a9dd..3bafa1def9b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13059,7 +13059,6 @@ "version": "12.1.5", "resolved": "https://registry.npmjs.org/@testing-library/react/-/react-12.1.5.tgz", "integrity": "sha512-OfTXCJUFgjd/digLUuPxa0+/3ZxsQmE7ub9kcbW/wi96Bh3o/p5vrETcBGfP17NWPGqeYYl5LTRpwyGoMC4ysg==", - "license": "MIT", "dependencies": { "@babel/runtime": "^7.12.5", "@testing-library/dom": "^8.0.0", @@ -43995,7 +43994,6 @@ "@mongodb-js/prettier-config-compass": "^1.2.8", "@mongodb-js/testing-library-compass": "^1.3.1", "@mongodb-js/tsconfig-compass": "^1.2.8", - "@testing-library/react": "^12.1.5", "@types/chai": "^4.2.21", "@types/mocha": "^9.0.0", "@types/react": "^17.0.5", @@ -57132,7 +57130,6 @@ "@mongodb-js/prettier-config-compass": "^1.2.8", "@mongodb-js/testing-library-compass": "^1.3.1", "@mongodb-js/tsconfig-compass": "^1.2.8", - "@testing-library/react": "^12.1.5", "@types/chai": "^4.2.21", "@types/mocha": "^9.0.0", "@types/react": "^17.0.5", @@ -64992,7 +64989,7 @@ "requires": { "@babel/runtime": "^7.12.5", "@testing-library/dom": "^8.0.0", - "@types/react-dom": "<18.0.0" + "@types/react-dom": "^17.0.25" } }, "@testing-library/react-hooks": { diff --git a/packages/compass-context-menu/package.json b/packages/compass-context-menu/package.json index b810c1a2aff..67099115f92 100644 --- a/packages/compass-context-menu/package.json +++ b/packages/compass-context-menu/package.json @@ -57,7 +57,6 @@ "@mongodb-js/prettier-config-compass": "^1.2.8", "@mongodb-js/testing-library-compass": "^1.3.1", "@mongodb-js/tsconfig-compass": "^1.2.8", - "@testing-library/react": "^12.1.5", "@types/chai": "^4.2.21", "@types/mocha": "^9.0.0", "@types/react": "^17.0.5", diff --git a/packages/compass-context-menu/src/context-menu-provider.spec.tsx b/packages/compass-context-menu/src/context-menu-provider.spec.tsx index 3173b88f7f6..53284272555 100644 --- a/packages/compass-context-menu/src/context-menu-provider.spec.tsx +++ b/packages/compass-context-menu/src/context-menu-provider.spec.tsx @@ -1,9 +1,12 @@ import React from 'react'; -import { render } from './render'; +import { testingLibrary } from '@mongodb-js/testing-library-compass'; import { expect } from 'chai'; import { ContextMenuProvider } from './context-menu-provider'; import type { ContextMenuWrapperProps } from './types'; +// We need to import from testing-library-compass directly to avoid the extra wrapping. +const { render } = testingLibrary; + describe('ContextMenuProvider', function () { const TestMenu: React.FC = () => (
Test Menu
diff --git a/packages/compass-context-menu/src/render.ts b/packages/compass-context-menu/src/render.ts deleted file mode 100644 index 35e6c857ea9..00000000000 --- a/packages/compass-context-menu/src/render.ts +++ /dev/null @@ -1,5 +0,0 @@ -// We need to import from testing-library/react directly because the wrapping done -// by testing-library-compass already sets up the context menu provider which is not -// useful for our tests. -// eslint-disable-next-line @typescript-eslint/no-restricted-imports -export { render } from '@testing-library/react'; diff --git a/packages/compass-context-menu/src/use-context-menu.spec.tsx b/packages/compass-context-menu/src/use-context-menu.spec.tsx index fadaffd73a6..9459f924b05 100644 --- a/packages/compass-context-menu/src/use-context-menu.spec.tsx +++ b/packages/compass-context-menu/src/use-context-menu.spec.tsx @@ -1,12 +1,18 @@ import React from 'react'; -import { render } from './render'; -import { screen, userEvent } from '@mongodb-js/testing-library-compass'; +import { + screen, + userEvent, + testingLibrary, +} from '@mongodb-js/testing-library-compass'; import { expect } from 'chai'; import sinon from 'sinon'; import { useContextMenu } from './use-context-menu'; import { ContextMenuProvider } from './context-menu-provider'; import type { ContextMenuItem, ContextMenuWrapperProps } from './types'; +// We need to import from testing-library-compass directly to avoid the extra wrapping. +const { render } = testingLibrary; + describe('useContextMenu', function () { const TestMenu: React.FC = ({ menu }) => (
From 8f306e41cb8d22e29164bcd379cf62e41069e646 Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 19 Jun 2025 18:27:56 +0200 Subject: [PATCH 43/46] feat: memoize items for context menu --- configs/eslint-config-compass/index.js | 2 +- .../src/components/context-menu.spec.tsx | 2 +- .../compass-components/src/components/context-menu.tsx | 8 +++++--- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/configs/eslint-config-compass/index.js b/configs/eslint-config-compass/index.js index a7f2479d9b6..21572f894e9 100644 --- a/configs/eslint-config-compass/index.js +++ b/configs/eslint-config-compass/index.js @@ -46,7 +46,7 @@ const tsxRules = { 'react-hooks/exhaustive-deps': [ 'warn', { - additionalHooks: 'useTrackOnChange', + additionalHooks: '(useTrackOnChange|useContextMenuItems)', }, ], }; diff --git a/packages/compass-components/src/components/context-menu.spec.tsx b/packages/compass-components/src/components/context-menu.spec.tsx index aa4aee700df..51ff62d6b6d 100644 --- a/packages/compass-components/src/components/context-menu.spec.tsx +++ b/packages/compass-components/src/components/context-menu.spec.tsx @@ -18,7 +18,7 @@ describe('useContextMenuItems', function () { children?: React.ReactNode; 'data-testid'?: string; }) => { - const ref = useContextMenuItems(items); + const ref = useContextMenuItems(() => items, [items]); return (
diff --git a/packages/compass-components/src/components/context-menu.tsx b/packages/compass-components/src/components/context-menu.tsx index b15715e128c..99abb6472c2 100644 --- a/packages/compass-components/src/components/context-menu.tsx +++ b/packages/compass-components/src/components/context-menu.tsx @@ -1,4 +1,4 @@ -import React, { useEffect } from 'react'; +import React, { useEffect, useMemo } from 'react'; import { Menu, MenuItem, MenuSeparator } from './leafygreen'; import type { ContextMenuItem } from '@mongodb-js/compass-context-menu'; import { useContextMenu } from '@mongodb-js/compass-context-menu'; @@ -88,8 +88,10 @@ export function ContextMenu({ menu }: ContextMenuWrapperProps) { } export function useContextMenuItems( - items: ContextMenuItem[] + getItems: () => ContextMenuItem[], + dependencies: React.DependencyList | undefined ): React.RefCallback { + const memoizedItems = useMemo(getItems, dependencies); const contextMenu = useContextMenu(); - return contextMenu.registerItems(items); + return contextMenu.registerItems(memoizedItems); } From aa52fb7f0153bd61a847d47b2883d2bb7a8c66ce Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 19 Jun 2025 18:42:02 +0200 Subject: [PATCH 44/46] fix: adjust dependencies --- package-lock.json | 2 ++ packages/compass-components/package.json | 1 + packages/compass-components/src/components/context-menu.tsx | 1 + 3 files changed, 4 insertions(+) diff --git a/package-lock.json b/package-lock.json index 3bafa1def9b..286d3a3f9e2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -43583,6 +43583,7 @@ "@leafygreen-ui/tokens": "^2.11.3", "@leafygreen-ui/tooltip": "^13.0.2", "@leafygreen-ui/typography": "^20.0.2", + "@mongodb-js/compass-context-menu": "^0.0.1", "@react-aria/interactions": "^3.9.1", "@react-aria/utils": "^3.13.1", "@react-aria/visually-hidden": "^3.3.1", @@ -56777,6 +56778,7 @@ "@leafygreen-ui/tokens": "^2.11.3", "@leafygreen-ui/tooltip": "^13.0.2", "@leafygreen-ui/typography": "^20.0.2", + "@mongodb-js/compass-context-menu": "^0.0.1", "@mongodb-js/eslint-config-compass": "^1.3.10", "@mongodb-js/mocha-config-compass": "^1.6.8", "@mongodb-js/prettier-config-compass": "^1.2.8", diff --git a/packages/compass-components/package.json b/packages/compass-components/package.json index 14af3a7b98b..e5f44908ba0 100644 --- a/packages/compass-components/package.json +++ b/packages/compass-components/package.json @@ -75,6 +75,7 @@ "@leafygreen-ui/tokens": "^2.11.3", "@leafygreen-ui/tooltip": "^13.0.2", "@leafygreen-ui/typography": "^20.0.2", + "@mongodb-js/compass-context-menu": "^0.0.1", "@react-aria/interactions": "^3.9.1", "@react-aria/utils": "^3.13.1", "@react-aria/visually-hidden": "^3.3.1", diff --git a/packages/compass-components/src/components/context-menu.tsx b/packages/compass-components/src/components/context-menu.tsx index 99abb6472c2..0346589e318 100644 --- a/packages/compass-components/src/components/context-menu.tsx +++ b/packages/compass-components/src/components/context-menu.tsx @@ -91,6 +91,7 @@ export function useContextMenuItems( getItems: () => ContextMenuItem[], dependencies: React.DependencyList | undefined ): React.RefCallback { + // eslint-disable-next-line react-hooks/exhaustive-deps const memoizedItems = useMemo(getItems, dependencies); const contextMenu = useContextMenu(); return contextMenu.registerItems(memoizedItems); From fb9c5996eb87cf7d539363d08a0a941faad57f3f Mon Sep 17 00:00:00 2001 From: gagik Date: Thu, 19 Jun 2025 19:01:40 +0200 Subject: [PATCH 45/46] fix: support nesting --- .../src/components/context-menu.spec.tsx | 17 +++++++---- .../src/context-menu-provider.spec.tsx | 29 ++++++++++--------- .../src/context-menu-provider.tsx | 17 ++++++----- 3 files changed, 36 insertions(+), 27 deletions(-) diff --git a/packages/compass-components/src/components/context-menu.spec.tsx b/packages/compass-components/src/components/context-menu.spec.tsx index 51ff62d6b6d..13fd851ec0c 100644 --- a/packages/compass-components/src/components/context-menu.spec.tsx +++ b/packages/compass-components/src/components/context-menu.spec.tsx @@ -28,7 +28,7 @@ describe('useContextMenuItems', function () { ); }; - it('errors if the component is double wrapped', function () { + it('works with nested providers, using the parent provider', function () { const items = [ { label: 'Test Item', @@ -36,15 +36,20 @@ describe('useContextMenuItems', function () { }, ]; - expect(() => { - render( + const { container } = render( + - ); - }).to.throw( - 'Duplicated ContextMenuProvider found. Please remove the nested provider.' + ); + + // Should only find one context menu (from the parent provider) + expect( + container.querySelectorAll('[data-testid="context-menu"]') + ).to.have.length(1); + // Should still render the trigger + expect(screen.getByTestId(menuTestTriggerId)).to.exist; }); it('renders without error', function () { diff --git a/packages/compass-context-menu/src/context-menu-provider.spec.tsx b/packages/compass-context-menu/src/context-menu-provider.spec.tsx index 53284272555..88d85c1fbcf 100644 --- a/packages/compass-context-menu/src/context-menu-provider.spec.tsx +++ b/packages/compass-context-menu/src/context-menu-provider.spec.tsx @@ -17,20 +17,23 @@ describe('ContextMenuProvider', function () { ); describe('when nested', function () { - it('throws an error when providers are nested', function () { - expect(() => { - render( - -
- - - -
- - ); - }).to.throw( - 'Duplicated ContextMenuProvider found. Please remove the nested provider.' + it('uses parent provider and does not render duplicate menu wrapper', function () { + const { container } = render( + +
+ + + +
+ ); + + // Should only find one test-menu element (from the parent provider) + expect( + container.querySelectorAll('[data-testid="test-menu"]') + ).to.have.length(1); + // Should still render the content + expect(container.querySelector('[data-testid="test-content"]')).to.exist; }); }); diff --git a/packages/compass-context-menu/src/context-menu-provider.tsx b/packages/compass-context-menu/src/context-menu-provider.tsx index aac3cd92494..0c2134f7ec4 100644 --- a/packages/compass-context-menu/src/context-menu-provider.tsx +++ b/packages/compass-context-menu/src/context-menu-provider.tsx @@ -26,13 +26,6 @@ export function ContextMenuProvider({ // Check if there's already a parent context menu provider const parentContext = useContext(ContextMenuContext); - // Prevent accidental nested providers - if (parentContext) { - throw new Error( - 'Duplicated ContextMenuProvider found. Please remove the nested provider.' - ); - } - const [menu, setMenu] = useState({ isOpen: false, itemGroups: [], @@ -50,6 +43,9 @@ export function ContextMenuProvider({ ); useEffect(() => { + // Don't set up event listeners if we have a parent context + if (parentContext) return; + function handleContextMenu(event: MouseEvent) { event.preventDefault(); @@ -77,7 +73,7 @@ export function ContextMenuProvider({ document.removeEventListener('contextmenu', handleContextMenu); window.removeEventListener('resize', handleClosingEvent); }; - }, [handleClosingEvent]); + }, [handleClosingEvent, parentContext]); const value = useMemo( () => ({ @@ -86,6 +82,11 @@ export function ContextMenuProvider({ [close] ); + // If we have a parent context, just render children without the wrapper + if (parentContext) { + return <>{children}; + } + const Wrapper = menuWrapper ?? React.Fragment; return ( From 9a8f24fda9f1be2eb4033acbb02f00a6566ba9da Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kr=C3=A6n=20Hansen?= Date: Mon, 30 Jun 2025 12:07:35 +0200 Subject: [PATCH 46/46] Fix memoization of event listeners --- .../compass-context-menu/src/context-menu-provider.tsx | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/compass-context-menu/src/context-menu-provider.tsx b/packages/compass-context-menu/src/context-menu-provider.tsx index 0c2134f7ec4..d88280d976f 100644 --- a/packages/compass-context-menu/src/context-menu-provider.tsx +++ b/packages/compass-context-menu/src/context-menu-provider.tsx @@ -31,15 +31,18 @@ export function ContextMenuProvider({ itemGroups: [], position: { x: 0, y: 0 }, }); - const close = useCallback(() => setMenu({ ...menu, isOpen: false }), [menu]); + const close = useCallback( + () => setMenu((prev) => ({ ...prev, isOpen: false })), + [setMenu] + ); const handleClosingEvent = useCallback( (event: Event) => { if (!event.defaultPrevented) { - setMenu({ ...menu, isOpen: false }); + close(); } }, - [menu] + [close] ); useEffect(() => {