Skip to content

Commit e265fb5

Browse files
authored
Dismiss tooltip/context menus on Escape (#2354)
* dismiss tooltip/context menus on Escape * add header context menu tests and fix issues
1 parent e0feb25 commit e265fb5

File tree

4 files changed

+101
-4
lines changed

4 files changed

+101
-4
lines changed

webview/src/experiments/components/App.test.tsx

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -671,6 +671,46 @@ describe('App', () => {
671671
})
672672
})
673673

674+
describe('Header Context Menu', () => {
675+
beforeAll(() => {
676+
jest.useFakeTimers()
677+
})
678+
afterAll(() => {
679+
jest.useRealTimers()
680+
})
681+
682+
it('should open on left click', () => {
683+
renderTableWithoutRunningExperiments()
684+
685+
const paramsFileHeader = screen.getByText('params.yaml')
686+
fireEvent.click(paramsFileHeader, { bubbles: true })
687+
688+
jest.advanceTimersByTime(100)
689+
const menuitems = screen.getAllByRole('menuitem')
690+
const itemLabels = menuitems.map(item => item.textContent)
691+
expect(itemLabels).toStrictEqual([
692+
'Open to the Side',
693+
'Sort Ascending',
694+
'Sort Descending'
695+
])
696+
})
697+
698+
it('should open on right click and close on esc', () => {
699+
renderTableWithoutRunningExperiments()
700+
701+
const paramsFileHeader = screen.getByText('params.yaml')
702+
fireEvent.contextMenu(paramsFileHeader, { bubbles: true })
703+
704+
jest.advanceTimersByTime(100)
705+
706+
const menuitems = screen.getAllByRole('menuitem')
707+
expect(menuitems).toHaveLength(3)
708+
709+
fireEvent.keyDown(paramsFileHeader, { bubbles: true, key: 'Escape' })
710+
expect(screen.queryAllByRole('menuitem')).toHaveLength(0)
711+
})
712+
})
713+
674714
describe('Row Context Menu', () => {
675715
beforeAll(() => {
676716
jest.useFakeTimers()
@@ -719,7 +759,7 @@ describe('App', () => {
719759
])
720760
})
721761

722-
it('should present the correct option for an experiment with checkpoints', () => {
762+
it('should present the correct option for an experiment with checkpoints and close on esc', () => {
723763
renderTableWithoutRunningExperiments()
724764

725765
const target = screen.getByText('[exp-e7a67]')
@@ -739,6 +779,9 @@ describe('App', () => {
739779
'Star',
740780
'Remove'
741781
])
782+
783+
fireEvent.keyDown(menuitems[0], { bubbles: true, key: 'Escape' })
784+
expect(screen.queryAllByRole('menuitem')).toHaveLength(0)
742785
})
743786

744787
it('should be removed with a left click', () => {

webview/src/experiments/components/table/TableHeaderCellContents.tsx

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,6 @@ export const ColumnDragHandle: React.FC<{
4141
<span
4242
data-testid="rendered-header"
4343
className={cx(styles.cellContents)}
44-
onKeyDown={e => {
45-
e.stopPropagation()
46-
}}
4744
role={'columnheader'}
4845
tabIndex={0}
4946
>

webview/src/plots/components/App.test.tsx

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1318,6 +1318,41 @@ describe('App', () => {
13181318
).toBeInTheDocument()
13191319
})
13201320

1321+
it('should dismiss a tooltip by pressing esc', () => {
1322+
renderAppWithOptionalData({
1323+
checkpoint: checkpointPlotsFixture,
1324+
comparison: comparisonTableFixture,
1325+
template: complexTemplatePlotsFixture
1326+
})
1327+
1328+
const [templateInfo] = screen.getAllByTestId('info-tooltip-toggle')
1329+
1330+
const getSectionText = (sectionNode: ReactElement) =>
1331+
// eslint-disable-next-line testing-library/no-node-access
1332+
sectionNode.props.children || ''
1333+
1334+
fireEvent.mouseEnter(templateInfo, { bubbles: true })
1335+
expect(
1336+
screen.getByText(
1337+
getSectionText(SectionDescription[Section.TEMPLATE_PLOTS])
1338+
)
1339+
).toBeInTheDocument()
1340+
1341+
fireEvent.keyDown(templateInfo, { bubbles: true, key: 'Space' })
1342+
expect(
1343+
screen.getByText(
1344+
getSectionText(SectionDescription[Section.TEMPLATE_PLOTS])
1345+
)
1346+
).toBeInTheDocument()
1347+
1348+
fireEvent.keyDown(templateInfo, { bubbles: true, key: 'Escape' })
1349+
expect(
1350+
screen.queryByText(
1351+
getSectionText(SectionDescription[Section.TEMPLATE_PLOTS])
1352+
)
1353+
).not.toBeInTheDocument()
1354+
})
1355+
13211356
describe('Virtualization', () => {
13221357
const createCheckpointPlots = (nbOfPlots: number) => {
13231358
const plots = []

webview/src/shared/components/tooltip/Tooltip.tsx

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,27 @@ import 'tippy.js/dist/tippy.css'
66
export const HEADER_TOOLTIP_DELAY = 100
77
export const CELL_TOOLTIP_DELAY = 1000
88

9+
const hideOnEsc = {
10+
defaultValue: true,
11+
fn({ hide }: { hide: () => void }) {
12+
function onKeyDown(event: KeyboardEvent) {
13+
if (event.key === 'Escape') {
14+
hide()
15+
}
16+
}
17+
18+
return {
19+
onHide() {
20+
document.removeEventListener('keydown', onKeyDown)
21+
},
22+
onShow() {
23+
document.addEventListener('keydown', onKeyDown)
24+
}
25+
}
26+
},
27+
name: 'hideOnEsc'
28+
}
29+
930
const TooltipRenderFunction: React.ForwardRefRenderFunction<
1031
unknown,
1132
TippyProps & { isContextMenu?: boolean }
@@ -57,6 +78,7 @@ const TooltipRenderFunction: React.ForwardRefRenderFunction<
5778
onShow={onShow}
5879
onHide={onHide}
5980
ref={ref as Ref<Element>}
81+
plugins={[hideOnEsc]}
6082
>
6183
{children}
6284
</Tippy>

0 commit comments

Comments
 (0)