Skip to content

Commit 58df56a

Browse files
gagikgagik
committed
fix: cleanup and switch to menu prop
1 parent aa9f0cf commit 58df56a

File tree

8 files changed

+186
-159
lines changed

8 files changed

+186
-159
lines changed

‎packages/compass-components/src/components/context-menu.spec.tsx‎

Lines changed: 72 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,23 @@ import { useContextMenuItems, ContextMenu } from './context-menu';
77
import type { ContextMenuItem } from '@mongodb-js/compass-context-menu';
88

99
describe('useContextMenuItems', function () {
10-
const TestComponent = ({ items }: { items: ContextMenuItem[] }) => {
10+
const menuTestTriggerId = 'test-trigger';
11+
12+
const TestComponent = ({
13+
items,
14+
children,
15+
'data-testid': dataTestId = menuTestTriggerId,
16+
}: {
17+
items: ContextMenuItem[];
18+
children?: React.ReactNode;
19+
'data-testid'?: string;
20+
}) => {
1121
const ref = useContextMenuItems(items);
1222

1323
return (
14-
<div data-testid="test-trigger" ref={ref}>
24+
<div data-testid={dataTestId} ref={ref}>
1525
Test Component
26+
{children}
1627
</div>
1728
);
1829
};
@@ -33,21 +44,6 @@ describe('useContextMenuItems', function () {
3344
});
3445

3546
describe('with a valid provider', function () {
36-
beforeEach(() => {
37-
// Create the container for the context menu portal
38-
const container = document.createElement('div');
39-
container.id = 'context-menu-container';
40-
document.body.appendChild(container);
41-
});
42-
43-
afterEach(() => {
44-
// Clean up the container
45-
const container = document.getElementById('context-menu-container');
46-
if (container) {
47-
document.body.removeChild(container);
48-
}
49-
});
50-
5147
it('renders without error', function () {
5248
const items = [
5349
{
@@ -62,7 +58,7 @@ describe('useContextMenuItems', function () {
6258
</ContextMenuProvider>
6359
);
6460

65-
expect(screen.getByTestId('test-trigger')).to.exist;
61+
expect(screen.getByTestId(menuTestTriggerId)).to.exist;
6662
});
6763

6864
it('shows context menu with items on right click', function () {
@@ -83,12 +79,12 @@ describe('useContextMenuItems', function () {
8379
</ContextMenuProvider>
8480
);
8581

86-
const trigger = screen.getByTestId('test-trigger');
82+
const trigger = screen.getByTestId(menuTestTriggerId);
8783
userEvent.click(trigger, { button: 2 });
8884

8985
// The menu items should be rendered
90-
expect(screen.getByTestId('context-menu-item-Test Item 1')).to.exist;
91-
expect(screen.getByTestId('context-menu-item-Test Item 2')).to.exist;
86+
expect(screen.getByTestId('menu-group-0-item-0')).to.exist;
87+
expect(screen.getByTestId('menu-group-0-item-1')).to.exist;
9288
});
9389

9490
it('triggers the correct action when menu item is clicked', function () {
@@ -110,40 +106,68 @@ describe('useContextMenuItems', function () {
110106
</ContextMenuProvider>
111107
);
112108

113-
const trigger = screen.getByTestId('test-trigger');
109+
const trigger = screen.getByTestId(menuTestTriggerId);
114110
userEvent.click(trigger, { button: 2 });
115111

116-
const menuItem = screen.getByTestId('context-menu-item-Test Item 2');
112+
const menuItem = screen.getByTestId('menu-group-0-item-1');
117113
userEvent.click(menuItem);
118114

119115
expect(onAction).to.have.been.calledOnceWithExactly(2);
120116
});
121117

122-
it('renders menu items with separators', function () {
123-
const items = [
124-
{
125-
label: 'Test Item 1',
126-
onAction: () => {},
127-
},
128-
{
129-
label: 'Test Item 2',
130-
onAction: () => {},
131-
},
132-
];
133-
134-
render(
135-
<ContextMenuProvider wrapper={ContextMenu}>
136-
<TestComponent items={items} />
137-
</ContextMenuProvider>
138-
);
139-
140-
const trigger = screen.getByTestId('test-trigger');
141-
userEvent.click(trigger, { button: 2 });
142-
143-
// Should find both menu items and a separator between them
144-
expect(screen.getByTestId('context-menu-item-Test Item 1')).to.exist;
145-
expect(screen.getByRole('separator')).to.exist;
146-
expect(screen.getByTestId('context-menu-item-Test Item 2')).to.exist;
118+
describe('with nested components', function () {
119+
const childTriggerId = 'child-trigger';
120+
121+
beforeEach(function () {
122+
const items = [
123+
{
124+
label: 'Test Item 1',
125+
onAction: () => {},
126+
},
127+
{
128+
label: 'Test Item 2',
129+
onAction: () => {},
130+
},
131+
];
132+
133+
const childItems = [
134+
{
135+
label: 'Child Item 1',
136+
onAction: () => {},
137+
},
138+
];
139+
140+
render(
141+
<ContextMenuProvider wrapper={ContextMenu}>
142+
<TestComponent items={items}>
143+
<TestComponent items={childItems} data-testid={childTriggerId} />
144+
</TestComponent>
145+
</ContextMenuProvider>
146+
);
147+
});
148+
149+
it('renders menu items with separators', function () {
150+
const trigger = screen.getByTestId(childTriggerId);
151+
userEvent.click(trigger, { button: 2 });
152+
153+
// Should find the menu item and the separator
154+
expect(screen.getByTestId('menu-group-0').children.length).to.equal(2);
155+
expect(
156+
screen.getByTestId('menu-group-0').children.item(0)?.textContent
157+
).to.equal('Child Item 1');
158+
159+
expect(screen.getByTestId('menu-group-0-separator')).to.exist;
160+
161+
expect(screen.getByTestId('menu-group-1').children.length).to.equal(2);
162+
expect(
163+
screen.getByTestId('menu-group-1').children.item(0)?.textContent
164+
).to.equal('Test Item 1');
165+
expect(
166+
screen.getByTestId('menu-group-1').children.item(1)?.textContent
167+
).to.equal('Test Item 2');
168+
169+
expect(screen.queryByTestId('menu-group-1-separator')).not.to.exist;
170+
});
147171
});
148172
});
149173
});

‎packages/compass-components/src/components/context-menu.tsx‎

Lines changed: 62 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,12 @@
1-
import React from 'react';
1+
import React, { useEffect } from 'react';
22
import { Menu, MenuItem, MenuSeparator } from './leafygreen';
33
import type { ContextMenuItem } from '@mongodb-js/compass-context-menu';
44
import { useContextMenu } from '@mongodb-js/compass-context-menu';
55
import { ContextMenuProvider as ContextMenuProviderBase } from '@mongodb-js/compass-context-menu';
6-
import type { ContextMenuItemGroup } from '@mongodb-js/compass-context-menu/dist/types';
6+
import type {
7+
ContextMenuItemGroup,
8+
ContextMenuWrapperProps,
9+
} from '@mongodb-js/compass-context-menu/dist/types';
710

811
export function ContextMenuProvider({
912
children,
@@ -17,40 +20,65 @@ export function ContextMenuProvider({
1720
);
1821
}
1922

20-
export type ContextMenuProps = {
21-
itemGroups: ContextMenuItemGroup[];
22-
className?: string;
23-
'data-testid'?: string;
24-
};
23+
export function ContextMenu({ menu }: ContextMenuWrapperProps) {
24+
const position = menu.position;
25+
const itemGroups = menu.itemGroups;
26+
27+
useEffect(() => {
28+
if (!menu.isOpen) {
29+
menu.close();
30+
}
31+
// eslint-disable-next-line react-hooks/exhaustive-deps
32+
}, [menu.isOpen]);
2533

26-
export function ContextMenu({ itemGroups }: ContextMenuProps) {
2734
return (
28-
<Menu open={true} renderMode="inline">
29-
{itemGroups.map((itemGroup: ContextMenuItemGroup, groupIndex: number) => {
30-
return (
31-
<div key={`menu-group-${groupIndex}`}>
32-
{itemGroup.items.map((item: ContextMenuItem, itemIndex: number) => {
33-
return (
34-
<MenuItem
35-
key={`menu-group-${groupIndex}-item-${itemIndex}`}
36-
data-text={item.label}
37-
data-testid={`context-menu-item-${item.label}`}
38-
onClick={(evt: React.MouseEvent) => {
39-
console.log('clicked', evt);
40-
item.onAction?.(evt);
41-
}}
42-
>
43-
{item.label} {itemIndex} {groupIndex}
44-
</MenuItem>
45-
);
46-
})}
47-
{groupIndex < itemGroups.length - 1 && (
48-
<MenuSeparator key={`${groupIndex}-separator`} />
49-
)}
50-
</div>
51-
);
52-
})}
53-
</Menu>
35+
<div
36+
style={{
37+
position: 'absolute',
38+
pointerEvents: 'all',
39+
left: position.x,
40+
top: position.y,
41+
}}
42+
>
43+
<Menu renderMode="inline" open={menu.isOpen} setOpen={menu.close}>
44+
{itemGroups.map(
45+
(itemGroup: ContextMenuItemGroup, groupIndex: number) => {
46+
return (
47+
<div
48+
key={`menu-group-${groupIndex}`}
49+
data-testid={`menu-group-${groupIndex}`}
50+
>
51+
{itemGroup.items.map(
52+
(item: ContextMenuItem, itemIndex: number) => {
53+
return (
54+
<MenuItem
55+
key={`menu-group-${groupIndex}-item-${itemIndex}`}
56+
data-text={item.label}
57+
data-testid={`menu-group-${groupIndex}-item-${itemIndex}`}
58+
onClick={(evt: React.MouseEvent) => {
59+
item.onAction?.(evt);
60+
menu.close();
61+
}}
62+
>
63+
{item.label}
64+
</MenuItem>
65+
);
66+
}
67+
)}
68+
{groupIndex < itemGroups.length - 1 && (
69+
<div
70+
key={`menu-group-${groupIndex}-separator`}
71+
data-testid={`menu-group-${groupIndex}-separator`}
72+
>
73+
<MenuSeparator />
74+
</div>
75+
)}
76+
</div>
77+
);
78+
}
79+
)}
80+
</Menu>
81+
</div>
5482
);
5583
}
5684

‎packages/compass-context-menu/src/compass-context-menu.tsx‎

Lines changed: 0 additions & 24 deletions
This file was deleted.

0 commit comments

Comments
 (0)