Skip to content

Commit fb9c599

Browse files
committed
fix: support nesting
1 parent aa52fb7 commit fb9c599

File tree

3 files changed

+36
-27
lines changed

3 files changed

+36
-27
lines changed

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

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,23 +28,28 @@ describe('useContextMenuItems', function () {
2828
);
2929
};
3030

31-
it('errors if the component is double wrapped', function () {
31+
it('works with nested providers, using the parent provider', function () {
3232
const items = [
3333
{
3434
label: 'Test Item',
3535
onAction: () => {},
3636
},
3737
];
3838

39-
expect(() => {
40-
render(
39+
const { container } = render(
40+
<ContextMenuProvider menuWrapper={ContextMenu}>
4141
<ContextMenuProvider menuWrapper={ContextMenu}>
4242
<TestComponent items={items} />
4343
</ContextMenuProvider>
44-
);
45-
}).to.throw(
46-
'Duplicated ContextMenuProvider found. Please remove the nested provider.'
44+
</ContextMenuProvider>
4745
);
46+
47+
// Should only find one context menu (from the parent provider)
48+
expect(
49+
container.querySelectorAll('[data-testid="context-menu"]')
50+
).to.have.length(1);
51+
// Should still render the trigger
52+
expect(screen.getByTestId(menuTestTriggerId)).to.exist;
4853
});
4954

5055
it('renders without error', function () {

packages/compass-context-menu/src/context-menu-provider.spec.tsx

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,23 @@ describe('ContextMenuProvider', function () {
1717
);
1818

1919
describe('when nested', function () {
20-
it('throws an error when providers are nested', function () {
21-
expect(() => {
22-
render(
23-
<ContextMenuProvider menuWrapper={TestMenu}>
24-
<div>
25-
<ContextMenuProvider menuWrapper={TestMenu}>
26-
<TestComponent />
27-
</ContextMenuProvider>
28-
</div>
29-
</ContextMenuProvider>
30-
);
31-
}).to.throw(
32-
'Duplicated ContextMenuProvider found. Please remove the nested provider.'
20+
it('uses parent provider and does not render duplicate menu wrapper', function () {
21+
const { container } = render(
22+
<ContextMenuProvider menuWrapper={TestMenu}>
23+
<div>
24+
<ContextMenuProvider menuWrapper={TestMenu}>
25+
<TestComponent />
26+
</ContextMenuProvider>
27+
</div>
28+
</ContextMenuProvider>
3329
);
30+
31+
// Should only find one test-menu element (from the parent provider)
32+
expect(
33+
container.querySelectorAll('[data-testid="test-menu"]')
34+
).to.have.length(1);
35+
// Should still render the content
36+
expect(container.querySelector('[data-testid="test-content"]')).to.exist;
3437
});
3538
});
3639

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

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,6 @@ export function ContextMenuProvider({
2626
// Check if there's already a parent context menu provider
2727
const parentContext = useContext(ContextMenuContext);
2828

29-
// Prevent accidental nested providers
30-
if (parentContext) {
31-
throw new Error(
32-
'Duplicated ContextMenuProvider found. Please remove the nested provider.'
33-
);
34-
}
35-
3629
const [menu, setMenu] = useState<ContextMenuState>({
3730
isOpen: false,
3831
itemGroups: [],
@@ -50,6 +43,9 @@ export function ContextMenuProvider({
5043
);
5144

5245
useEffect(() => {
46+
// Don't set up event listeners if we have a parent context
47+
if (parentContext) return;
48+
5349
function handleContextMenu(event: MouseEvent) {
5450
event.preventDefault();
5551

@@ -77,7 +73,7 @@ export function ContextMenuProvider({
7773
document.removeEventListener('contextmenu', handleContextMenu);
7874
window.removeEventListener('resize', handleClosingEvent);
7975
};
80-
}, [handleClosingEvent]);
76+
}, [handleClosingEvent, parentContext]);
8177

8278
const value = useMemo(
8379
() => ({
@@ -86,6 +82,11 @@ export function ContextMenuProvider({
8682
[close]
8783
);
8884

85+
// If we have a parent context, just render children without the wrapper
86+
if (parentContext) {
87+
return <>{children}</>;
88+
}
89+
8990
const Wrapper = menuWrapper ?? React.Fragment;
9091

9192
return (

0 commit comments

Comments
 (0)