-
Notifications
You must be signed in to change notification settings - Fork 228
feat(compass-components): add context menu COMPASS-9386 #6956
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
297a943
6838513
8e30a83
7d17984
6004593
43023f4
971b0fd
0ad7a63
aa9f0cf
58df56a
a54c738
731ac13
56d11b6
743f068
ab14b78
1d279ca
1efdb2e
0f5303b
33b2a41
4a2f032
8e1feb3
1f55000
9618e8e
ca1fb86
a285d63
aacdf1d
cb99706
da22b51
78ff94c
a24e89c
f3869ea
c2d9ac1
1c6ef03
3d14d6d
ee658e1
a173aae
4730c18
eb65769
220a300
74f3d6e
4f05381
4ad7231
cbb0656
c6d7b03
6b6e398
17c05f0
6dadf25
a62690b
4dafa7b
915c597
8f306e4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,161 @@ | ||
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 menuTestTriggerId = 'test-trigger'; | ||
|
||
const TestComponent = ({ | ||
items, | ||
children, | ||
'data-testid': dataTestId = menuTestTriggerId, | ||
}: { | ||
items: ContextMenuItem[]; | ||
children?: React.ReactNode; | ||
'data-testid'?: string; | ||
}) => { | ||
const ref = useContextMenuItems(() => items, [items]); | ||
|
||
return ( | ||
<div data-testid={dataTestId} ref={ref}> | ||
Test Component | ||
{children} | ||
</div> | ||
); | ||
}; | ||
|
||
it('errors if the component is double wrapped', function () { | ||
const items = [ | ||
{ | ||
label: 'Test Item', | ||
onAction: () => {}, | ||
}, | ||
]; | ||
|
||
expect(() => { | ||
render( | ||
<ContextMenuProvider menuWrapper={ContextMenu}> | ||
<TestComponent items={items} /> | ||
</ContextMenuProvider> | ||
); | ||
}).to.throw( | ||
'Duplicated ContextMenuProvider found. Please remove the nested provider.' | ||
); | ||
}); | ||
|
||
it('renders without error', function () { | ||
const items = [ | ||
{ | ||
label: 'Test Item', | ||
onAction: () => {}, | ||
}, | ||
]; | ||
|
||
render(<TestComponent items={items} />); | ||
|
||
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(<TestComponent items={items} />); | ||
|
||
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(<TestComponent items={items} />); | ||
|
||
const trigger = screen.getByTestId(menuTestTriggerId); | ||
userEvent.click(trigger, { button: 2 }); | ||
|
||
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', | ||
onAction: () => {}, | ||
}, | ||
{ | ||
label: 'Test Item 2', | ||
onAction: () => {}, | ||
}, | ||
]; | ||
|
||
const childItems = [ | ||
{ | ||
label: 'Child Item 1', | ||
onAction: () => {}, | ||
}, | ||
]; | ||
|
||
render( | ||
<TestComponent items={items}> | ||
<TestComponent items={childItems} data-testid={childTriggerId} /> | ||
</TestComponent> | ||
); | ||
}); | ||
|
||
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; | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,97 @@ | ||
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'; | ||
import { ContextMenuProvider as ContextMenuProviderBase } from '@mongodb-js/compass-context-menu'; | ||
import type { | ||
ContextMenuItemGroup, | ||
ContextMenuWrapperProps, | ||
} from '@mongodb-js/compass-context-menu'; | ||
|
||
export function ContextMenuProvider({ | ||
children, | ||
}: { | ||
children: React.ReactNode; | ||
}) { | ||
return ( | ||
<ContextMenuProviderBase menuWrapper={ContextMenu}> | ||
{children} | ||
</ContextMenuProviderBase> | ||
); | ||
} | ||
|
||
export function ContextMenu({ menu }: ContextMenuWrapperProps) { | ||
const position = menu.position; | ||
const itemGroups = menu.itemGroups; | ||
|
||
useEffect(() => { | ||
if (!menu.isOpen) { | ||
menu.close(); | ||
} | ||
}, [menu.isOpen]); | ||
|
||
return ( | ||
<div | ||
data-testid="context-menu" | ||
style={{ | ||
position: 'absolute', | ||
pointerEvents: 'all', | ||
left: position.x, | ||
top: position.y, | ||
}} | ||
> | ||
<Menu | ||
renderMode="inline" | ||
open={menu.isOpen} | ||
setOpen={menu.close} | ||
justify="start" | ||
> | ||
{itemGroups.map( | ||
(itemGroup: ContextMenuItemGroup, groupIndex: number) => { | ||
return ( | ||
<div | ||
key={`menu-group-${groupIndex}`} | ||
data-testid={`menu-group-${groupIndex}`} | ||
> | ||
{itemGroup.items.map( | ||
(item: ContextMenuItem, itemIndex: number) => { | ||
return ( | ||
<MenuItem | ||
key={`menu-group-${groupIndex}-item-${itemIndex}`} | ||
data-text={item.label} | ||
data-testid={`menu-group-${groupIndex}-item-${itemIndex}`} | ||
onClick={(evt: React.MouseEvent) => { | ||
item.onAction?.(evt); | ||
menu.close(); | ||
}} | ||
> | ||
{item.label} | ||
</MenuItem> | ||
); | ||
} | ||
)} | ||
{groupIndex < itemGroups.length - 1 && ( | ||
<div | ||
key={`menu-group-${groupIndex}-separator`} | ||
data-testid={`menu-group-${groupIndex}-separator`} | ||
> | ||
<MenuSeparator /> | ||
</div> | ||
)} | ||
</div> | ||
); | ||
} | ||
)} | ||
</Menu> | ||
</div> | ||
); | ||
} | ||
|
||
export function useContextMenuItems( | ||
getItems: () => ContextMenuItem[], | ||
dependencies: React.DependencyList | undefined | ||
): React.RefCallback<HTMLElement> { | ||
const memoizedItems = useMemo(getItems, dependencies); | ||
const contextMenu = useContextMenu(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there could be reasons for us to memoize these items? maybe we should support that at this point There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. alternatively we leave that to outside of this hook There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we don't memoize these items it looks like it will recreate all of the listeners and the ref function every time it renders. I'm not sure how new functions passed to ref perform work or cause other items to render, but it sounds like its something that would be nice to avoid by memoizing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good point. made it memoizable by default |
||
return contextMenu.registerItems(memoizedItems); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one might want to keep this empty... the reason the context menu always exists even when closed is for the sake of animations