Skip to content

Commit 0205d9d

Browse files
linuspahllinuspahl
authored
Entity data table: Open actions menu when clicking anywhere in column header. (#26796)
* Entity data table: Open actions menu when clicking anywhere in column header. * Show dropdown arrows on hover again. * Display gap again. * Create hook for new logic. * Simplify comments.
1 parent 9a307b9 commit 0205d9d

7 files changed

Lines changed: 269 additions & 41 deletions

File tree

‎graylog2-web-interface/src/components/bootstrap/useClickToOpenMenu.test.tsx‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
* Copyright (C) 2020 Graylog, Inc.
3+
*
4+
* This program is free software: you can redistribute it and/or modify
5+
* it under the terms of the Server Side Public License, version 1,
6+
* as published by MongoDB, Inc.
7+
*
8+
* This program is distributed in the hope that it will be useful,
9+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
10+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11+
* Server Side Public License for more details.
12+
*
13+
* You should have received a copy of the Server Side Public License
14+
* along with this program. If not, see
15+
* <http://www.mongodb.com/licensing/server-side-public-license>.
16+
*/
17+
import * as React from 'react';
18+
import { render, screen, waitFor } from 'wrappedTestingLibrary';
19+
import userEvent from '@testing-library/user-event';
20+
21+
import Menu from 'components/bootstrap/Menu';
22+
import { MenuItem } from 'components/bootstrap';
23+
24+
import useClickToOpenMenu, { MenuAnchor } from './useClickToOpenMenu';
25+
26+
const TestTrigger = ({ onSelect = () => {} }: { onSelect?: () => void }) => {
27+
const { triggerRef, opened, onOpenChange, anchorPosition, onClick, onKeyDown } =
28+
useClickToOpenMenu<HTMLButtonElement>();
29+
30+
return (
31+
<Menu opened={opened} onChange={onOpenChange} withinPortal position="bottom-start">
32+
<Menu.Target>
33+
<MenuAnchor style={{ left: anchorPosition?.x ?? 0, top: anchorPosition?.y ?? 0 }} />
34+
</Menu.Target>
35+
<button type="button" ref={triggerRef} onClick={onClick} onKeyDown={onKeyDown}>
36+
Trigger
37+
</button>
38+
<Menu.Dropdown>
39+
<MenuItem onClick={onSelect}>Item</MenuItem>
40+
</Menu.Dropdown>
41+
</Menu>
42+
);
43+
};
44+
45+
describe('useClickToOpenMenu', () => {
46+
it('should open the menu when clicking the trigger', async () => {
47+
render(<TestTrigger />);
48+
49+
await userEvent.click(await screen.findByRole('button', { name: /trigger/i }));
50+
51+
await screen.findByRole('menuitem', { name: /item/i });
52+
});
53+
54+
it('should open the menu with the Enter key', async () => {
55+
render(<TestTrigger />);
56+
57+
const trigger = await screen.findByRole('button', { name: /trigger/i });
58+
trigger.focus();
59+
60+
await userEvent.keyboard('{Enter}');
61+
62+
await screen.findByRole('menuitem', { name: /item/i });
63+
});
64+
65+
it('should return focus to the trigger once the menu closes', async () => {
66+
const onSelect = jest.fn();
67+
render(<TestTrigger onSelect={onSelect} />);
68+
69+
const trigger = await screen.findByRole('button', { name: /trigger/i });
70+
await userEvent.click(trigger);
71+
await userEvent.click(await screen.findByRole('menuitem', { name: /item/i }));
72+
73+
expect(onSelect).toHaveBeenCalledTimes(1);
74+
await waitFor(() => expect(trigger).toHaveFocus());
75+
});
76+
});

‎graylog2-web-interface/src/components/bootstrap/useClickToOpenMenu.tsx‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
/*
2+
* Copyright (C) 2020 Graylog, Inc.
3+
*
4+
* This program is free software: you can redistribute it and/or modify
5+
* it under the terms of the Server Side Public License, version 1,
6+
* as published by MongoDB, Inc.
7+
*
8+
* This program is distributed in the hope that it will be useful,
9+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
10+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
11+
* Server Side Public License for more details.
12+
*
13+
* You should have received a copy of the Server Side Public License
14+
* along with this program. If not, see
15+
* <http://www.mongodb.com/licensing/server-side-public-license>.
16+
*/
17+
import * as React from 'react';
18+
import { forwardRef, useRef, useState } from 'react';
19+
import { createPortal } from 'react-dom';
20+
import styled from 'styled-components';
21+
22+
type Position = { x: number; y: number };
23+
24+
const StyledMenuAnchor = styled.div`
25+
position: fixed;
26+
width: 0;
27+
height: 0;
28+
pointer-events: none;
29+
`;
30+
31+
// Invisible positioning anchor for a Menu's `Menu.Target`, moved to the click point; portaled into
32+
// <body> so a `transform` on some ancestor can't turn its `position: fixed` coordinates relative.
33+
export const MenuAnchor = forwardRef<HTMLDivElement, { style?: React.CSSProperties }>(
34+
({ style = undefined, ...rest }, ref) =>
35+
createPortal(<StyledMenuAnchor ref={ref} style={style} {...rest} />, document.body),
36+
);
37+
38+
// Makes a whole element (not just a dedicated toggle button) act as a dropdown trigger: click or
39+
// Enter opens a Mantine `Menu` at that point, a second click/Enter closes it, and closing it for
40+
// any reason returns focus to the trigger; rendering the `<Menu>` itself is left to the caller.
41+
const useClickToOpenMenu = <Trigger extends HTMLElement = HTMLElement>() => {
42+
const triggerRef = useRef<Trigger>(null);
43+
const [opened, setOpened] = useState(false);
44+
const [anchorPosition, setAnchorPosition] = useState<Position | null>(null);
45+
46+
// Mantine's own focus-return fires too late here (its FocusTrap moves focus into the dropdown
47+
// before that capture runs), so we return focus to the trigger on close ourselves.
48+
const onOpenChange = (nextOpened: boolean) => {
49+
setOpened(nextOpened);
50+
51+
if (!nextOpened) {
52+
triggerRef.current?.focus({ preventScroll: true });
53+
}
54+
};
55+
56+
const toggleAt = (position: Position) => {
57+
if (opened) {
58+
onOpenChange(false);
59+
60+
return;
61+
}
62+
63+
setAnchorPosition(position);
64+
setOpened(true);
65+
};
66+
67+
const onClick = (event: React.MouseEvent<Trigger>) => {
68+
// An assistive-tech-triggered click reports (0, 0) here, so fall back to the trigger's rect.
69+
const { clientX, clientY } = event;
70+
const hasPointerPosition = clientX !== 0 || clientY !== 0;
71+
const rect = event.currentTarget.getBoundingClientRect();
72+
73+
toggleAt(hasPointerPosition ? { x: clientX, y: clientY } : { x: rect.left, y: rect.bottom });
74+
};
75+
76+
// Ignore keydowns bubbled up from a focused descendant (e.g. a menu item), only the trigger itself.
77+
const onKeyDown = (event: React.KeyboardEvent<Trigger>) => {
78+
if (event.target !== event.currentTarget || event.key !== 'Enter') {
79+
return;
80+
}
81+
82+
event.preventDefault();
83+
const rect = event.currentTarget.getBoundingClientRect();
84+
toggleAt({ x: rect.left, y: rect.bottom });
85+
};
86+
87+
return { triggerRef, opened, onOpenChange, anchorPosition, onClick, onKeyDown };
88+
};
89+
90+
export default useClickToOpenMenu;

‎graylog2-web-interface/src/components/common/EntityDataTable/EntityDataTable.test.tsx‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -215,20 +215,33 @@ describe('<EntityDataTable />', () => {
215215

216216
render(<EntityDataTable {...defaultProps} onSortChange={onSortChange} />);
217217

218-
await userEvent.click(await screen.findByRole('button', { name: /toggle description actions/i }));
218+
await userEvent.click(await screen.findByRole('button', { name: /drag or press space to reorder description/i }));
219219
await userEvent.click(await screen.findByRole('menuitem', { name: /sort ascending/i }));
220220

221221
await waitFor(() => expect(onSortChange).toHaveBeenCalledTimes(1));
222222

223223
expect(onSortChange).toHaveBeenCalledWith({ attributeId: 'description', direction: 'asc' });
224224
});
225225

226+
it('should open header actions menu with the Enter key, leaving Space free for column drag', async () => {
227+
render(<EntityDataTable {...defaultProps} />);
228+
229+
const descriptionHeader = await screen.findByRole('button', {
230+
name: /drag or press space to reorder description/i,
231+
});
232+
descriptionHeader.focus();
233+
234+
await userEvent.keyboard('{Enter}');
235+
236+
await screen.findByRole('menuitem', { name: /sort ascending/i });
237+
});
238+
226239
it('should slice by column using header action', async () => {
227240
const onChangeSlicing = jest.fn(() => {});
228241

229242
render(<EntityDataTable {...defaultProps} columnSchemas={columnSchemas} onChangeSlicing={onChangeSlicing} />);
230243

231-
await userEvent.click(await screen.findByRole('button', { name: /toggle description actions/i }));
244+
await userEvent.click(await screen.findByRole('button', { name: /drag or press space to reorder description/i }));
232245
await userEvent.click(await screen.findByRole('menuitem', { name: /slice by values/i }));
233246

234247
expect(onChangeSlicing).toHaveBeenCalledWith('description');
@@ -246,7 +259,7 @@ describe('<EntityDataTable />', () => {
246259
/>,
247260
);
248261

249-
await userEvent.click(await screen.findByRole('button', { name: /toggle description actions/i }));
262+
await userEvent.click(await screen.findByRole('button', { name: /drag or press space to reorder description/i }));
250263
await userEvent.click(await screen.findByRole('menuitem', { name: /no slicing/i }));
251264

252265
expect(onChangeSlicing).toHaveBeenCalledWith(undefined, undefined);

‎graylog2-web-interface/src/components/common/EntityDataTable/HeaderActionsDropdown.tsx‎

Lines changed: 30 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -19,38 +19,29 @@ import * as React from 'react';
1919
import styled, { css } from 'styled-components';
2020

2121
import Menu from 'components/bootstrap/Menu';
22-
import Icon from 'components/common/Icon';
22+
import { MenuAnchor } from 'components/bootstrap/useClickToOpenMenu';
2323
import { TELEMETRY_EVENT_TYPE } from 'logic/telemetry/Constants';
2424
import useSendTelemetry from 'logic/telemetry/useSendTelemetry';
2525
import { MenuItem } from 'components/bootstrap';
26+
import Icon from 'components/common/Icon';
2627

27-
export const DropdownCaret = styled(Icon)`
28-
opacity: 0;
29-
transition: opacity 0.15s ease-in-out;
30-
`;
31-
32-
const DropdownTrigger = styled.button(
28+
// A plain (non-focusable) span: the header cell itself (ThInner, see AttributeHeader) is the one
29+
// focusable/clickable element for opening this menu, so this only renders the label -- it must
30+
// not be its own tab stop, or focusing the header would take two Tab presses instead of one.
31+
const DropdownTrigger = styled.span(
3332
({ theme }) => css`
34-
background: transparent;
35-
border: 0;
36-
padding: 0;
3733
display: inline-flex;
3834
align-items: center;
3935
gap: ${theme.spacings.xxs};
4036
line-height: inherit;
41-
42-
&:focus-visible {
43-
outline-offset: 2px;
44-
}
45-
46-
&:hover .header-action,
47-
&:focus-within .header-action,
48-
&[aria-expanded='true'] .header-action {
49-
opacity: 1;
50-
}
5137
`,
5238
);
5339

40+
export const DropdownCaret = styled(Icon)`
41+
opacity: 0;
42+
transition: opacity 0.15s ease-in-out;
43+
`;
44+
5445
const MenuItemLabel = styled.span<{ $active: boolean }>(
5546
({ $active }) => css`
5647
font-weight: ${$active ? 'bold' : 'inherit'};
@@ -67,7 +58,10 @@ type Props = {
6758
appSection?: string;
6859
onSort?: (desc: boolean) => void;
6960
onHideColumn?: () => void;
70-
textAlign?: string;
61+
opened?: boolean;
62+
onOpenChange?: (opened: boolean) => void;
63+
anchorPosition?: { x: number; y: number } | null;
64+
textAlign: 'right' | 'left';
7165
};
7266

7367
const HeaderActionsDropdown = ({
@@ -80,7 +74,10 @@ const HeaderActionsDropdown = ({
8074
appSection = undefined,
8175
onSort = undefined,
8276
onHideColumn = undefined,
83-
textAlign = undefined,
77+
opened = undefined,
78+
onOpenChange = undefined,
79+
anchorPosition = undefined,
80+
textAlign,
8481
}: Props) => {
8582
const sendTelemetry = useSendTelemetry();
8683
const hasActions = Boolean(onChangeSlicing || onSort || onHideColumn);
@@ -109,14 +106,19 @@ const HeaderActionsDropdown = ({
109106
}
110107

111108
return (
112-
<Menu shadow="md" withinPortal position="bottom-start">
109+
<Menu shadow="md" withinPortal position="bottom-start" opened={opened} onChange={onOpenChange}>
113110
<Menu.Target>
114-
<DropdownTrigger type="button" title={`Toggle ${label} actions`} aria-label={`Toggle ${label} actions`}>
115-
{textAlign === 'right' && <DropdownCaret name="arrow_drop_down" size="xs" className="header-action" />}
116-
<span>{children}</span>
117-
{textAlign !== 'right' && <DropdownCaret name="arrow_drop_down" size="xs" className="header-action" />}
118-
</DropdownTrigger>
111+
<MenuAnchor style={{ left: anchorPosition?.x ?? 0, top: anchorPosition?.y ?? 0 }} />
119112
</Menu.Target>
113+
{/* Not the Menu.Target: opening/positioning is driven by the whole header's click handler
114+
(see AttributeHeader) so this only needs to render the label -- clicking it still opens
115+
the menu, since the click bubbles up to that handler. */}
116+
<DropdownTrigger title={`Toggle ${label} actions`}>
117+
{textAlign === 'right' && <DropdownCaret name="arrow_drop_down" size="xs" className="header-action" />}
118+
{children}
119+
{textAlign !== 'right' && <DropdownCaret name="arrow_drop_down" size="xs" className="header-action" />}
120+
</DropdownTrigger>
121+
120122
<Menu.Dropdown>
121123
{onSort && (
122124
<MenuItem onClick={() => onSort(false)} icon="arrow_upward">

‎graylog2-web-interface/src/components/common/EntityDataTable/ThDragOverlay.tsx‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,20 +87,20 @@ const ThGhostInner = <Entity extends EntityBase>(
8787
// Matches AttributeHeader: for right-aligned (numeric) columns, the indicator icons and title
8888
// are mirrored (indicators first, title last) instead of title-then-indicators, and the caret
8989
// inside the title button itself moves to the other side of the label too (see textAlign below).
90-
const textAlign = columnMeta?.columnRenderer?.textAlign;
90+
const textAlign = columnMeta?.columnRenderer?.textAlign as 'left' | 'right';
9191
const isRightAligned = textAlign === 'right';
9292

9393
const titleGroup = (
9494
<LeftCol>
9595
<HeaderActionsDropdown
96+
textAlign={textAlign}
9697
label={columnLabel}
9798
activeSort={sortDirection}
9899
isSliceActive={isSliceActive}
99100
onChangeSlicing={canSlice ? () => {} : undefined}
100101
sliceColumnId={column.id}
101102
onSort={canSort ? () => {} : undefined}
102-
onHideColumn={canHideColumn ? () => {} : undefined}
103-
textAlign={textAlign}>
103+
onHideColumn={canHideColumn ? () => {} : undefined}>
104104
{columnLabel}
105105
</HeaderActionsDropdown>
106106
</LeftCol>

0 commit comments

Comments
 (0)