Skip to content

Commit c01bb30

Browse files
authored
Rename Popover to ActionMenu (#2914)
* Rename the Popover component to ActionMenu * create eslint rule to replace renamed components * code review
1 parent 975294d commit c01bb30

File tree

17 files changed

+399
-148
lines changed

17 files changed

+399
-148
lines changed

.changeset/metal-sloths-ring.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@sumup-oss/circuit-ui": major
3+
---
4+
5+
Renamed the Popover component to ActionMenu to better reflect its purpose.

.changeset/silent-moles-guess.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@sumup-oss/eslint-plugin-circuit-ui": minor
3+
---
4+
5+
Added a `no-renamed-components` new ESLint rule to flag and replace renamed components.
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
import { Meta, Status, Props, Story } from '../../../../.storybook/components';
2+
import * as Stories from './ActionMenu.stories';
3+
4+
<Meta of={Stories} />
5+
6+
# ActionMenu
7+
8+
An ActionMenu displays a list of subsequent actions, when interacting with an actionable component.
9+
10+
<Story of={Stories.Base} />
11+
<Props />
12+
13+
- The reference element which triggers the ActionMenu must be a `button` element or a component that renders a `button` such as the Button or IconButton components.
14+
- Each ActionMenu action item is represented by an appropriate HTML element. Elements provided with an `href` will render as anchor elements while elements provided with an `onClick` will render as buttons.
15+
- If needed, the dividing line can be used to separate ActionMenu action items.
16+
- The leading icon is optional.
17+
- The ActionMenu is powered by [Floating UI](https://floating-ui.com/docs/react-dom). You can change the position of the ActionMenu relative to the reference element by passing in the `placement` prop. If you want to offset the ActionMenu in the x and y directions, use the `offset` prop.
18+
19+
<Story of={Stories.Offset} />
20+
21+
## Related components
22+
23+
This component is built on top of the low level [Dialog](Components/Dialog/Docs) component. If this component does not meet your requirements, you can use the Dialog component directly to build your own custom popover component.
24+
25+
## Usage guidelines
26+
27+
- **Do** use clear, concise and actionable labels for ActionMenu items.
28+
- **Do** always think about the priority of the actions to be taken and put the options in logical order.

packages/circuit-ui/components/Popover/Popover.spec.tsx renamed to packages/circuit-ui/components/ActionMenu/ActionMenu.spec.tsx

Lines changed: 45 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@ import { waitFor } from '@testing-library/react';
2020

2121
import { act, axe, render, userEvent, screen } from '../../util/test-utils.js';
2222

23-
import { Popover, type PopoverProps } from './Popover.js';
23+
import { ActionMenu, type ActionMenuProps } from './ActionMenu.js';
2424

25-
describe('Popover', () => {
25+
describe('ActionMenu', () => {
2626
beforeEach(() => {
2727
vi.useFakeTimers({ shouldAdvanceTime: true });
2828
});
@@ -31,8 +31,8 @@ describe('Popover', () => {
3131
vi.clearAllMocks();
3232
});
3333

34-
function renderPopover(props: PopoverProps) {
35-
return render(<Popover {...props} />);
34+
function renderActionMenu(props: ActionMenuProps) {
35+
return render(<ActionMenu {...props} />);
3636
}
3737

3838
function createStateSetter(initialState: boolean) {
@@ -55,7 +55,7 @@ describe('Popover', () => {
5555
await act(async () => {});
5656
}
5757

58-
const baseProps: PopoverProps = {
58+
const baseProps: ActionMenuProps = {
5959
component: (triggerProps) => <button {...triggerProps}>Button</button>,
6060
actions: [
6161
{
@@ -76,19 +76,19 @@ describe('Popover', () => {
7676
};
7777
it('should forward a ref', () => {
7878
const ref = createRef<HTMLDialogElement>();
79-
render(<Popover {...baseProps} ref={ref} />);
79+
render(<ActionMenu {...baseProps} ref={ref} />);
8080
const dialog = screen.getByRole('dialog', { hidden: true });
8181
expect(ref.current).toBe(dialog);
8282
});
8383

84-
it('should open the popover when clicking the trigger element', async () => {
84+
it('should open the action menu when clicking the trigger element', async () => {
8585
const isOpen = false;
8686
const onToggle = vi.fn(createStateSetter(isOpen));
87-
renderPopover({ ...baseProps, isOpen, onToggle });
87+
renderActionMenu({ ...baseProps, isOpen, onToggle });
8888

89-
const popoverTrigger = screen.getByRole('button');
89+
const trigger = screen.getByRole('button');
9090

91-
await userEvent.click(popoverTrigger);
91+
await userEvent.click(trigger);
9292

9393
expect(onToggle).toHaveBeenCalledTimes(1);
9494
});
@@ -99,23 +99,23 @@ describe('Popover', () => {
9999
['arrow down', '{ArrowDown}'],
100100
['arrow up', '{ArrowUp}'],
101101
])(
102-
'should open the popover when pressing the %s key on the trigger element',
102+
'should open the action menu when pressing the %s key on the trigger element',
103103
async (_, key) => {
104104
const isOpen = false;
105105
const onToggle = vi.fn(createStateSetter(isOpen));
106-
renderPopover({ ...baseProps, isOpen, onToggle });
106+
renderActionMenu({ ...baseProps, isOpen, onToggle });
107107

108-
const popoverTrigger = screen.getByRole('button');
108+
const trigger = screen.getByRole('button');
109109

110-
popoverTrigger.focus();
110+
trigger.focus();
111111
await userEvent.keyboard(key);
112112

113113
expect(onToggle).toHaveBeenCalledTimes(1);
114114
},
115115
);
116116

117-
it('should close the popover when clicking outside', async () => {
118-
renderPopover(baseProps);
117+
it('should close the action menu when clicking outside', async () => {
118+
renderActionMenu(baseProps);
119119

120120
await userEvent.click(document.body);
121121

@@ -124,12 +124,12 @@ describe('Popover', () => {
124124
});
125125
});
126126

127-
it('should close the popover when clicking the trigger element', async () => {
128-
renderPopover(baseProps);
127+
it('should close the action menu when clicking the trigger element', async () => {
128+
renderActionMenu(baseProps);
129129

130-
const popoverTrigger = screen.getByRole('button');
130+
const trigger = screen.getByRole('button');
131131

132-
await userEvent.click(popoverTrigger);
132+
await userEvent.click(trigger);
133133

134134
// TODO Find a better way to test this as toHaveBeenCalled is not reliable here.
135135
expect(baseProps.onToggle).toHaveBeenCalled();
@@ -140,82 +140,82 @@ describe('Popover', () => {
140140
['enter', '{Enter}'],
141141
['arrow up', '{ArrowUp}'],
142142
])(
143-
'should close the popover when pressing the %s key on the trigger element',
143+
'should close the action menu when pressing the %s key on the trigger element',
144144
async (_, key) => {
145-
renderPopover(baseProps);
145+
renderActionMenu(baseProps);
146146
vi.runAllTimers();
147147

148-
const popoverTrigger = screen.getByRole('button');
148+
const trigger = screen.getByRole('button');
149149

150-
popoverTrigger.focus();
150+
trigger.focus();
151151
await userEvent.keyboard(key);
152152

153153
expect(baseProps.onToggle).toHaveBeenCalledTimes(1);
154154
},
155155
);
156156

157-
it('should close the popover when clicking the escape key', async () => {
158-
renderPopover(baseProps);
157+
it('should close the action menu when clicking the escape key', async () => {
158+
renderActionMenu(baseProps);
159159

160160
await userEvent.keyboard('{Escape}');
161161

162162
await waitFor(() => expect(baseProps.onToggle).toHaveBeenCalledTimes(1));
163163
});
164164

165-
it('should close the popover when clicking a popover item', async () => {
166-
renderPopover(baseProps);
165+
it('should close the action menu when clicking a action menu item', async () => {
166+
renderActionMenu(baseProps);
167167

168-
const popoverItems = screen.getAllByRole('menuitem');
168+
const actionMenuItems = screen.getAllByRole('menuitem');
169169

170-
await userEvent.click(popoverItems[0]);
170+
await userEvent.click(actionMenuItems[0]);
171171

172172
expect(baseProps.onToggle).toHaveBeenCalledTimes(1);
173173
});
174174

175-
it('should move focus to the first popover item after opening', async () => {
175+
it('should move focus to the first action menu item after opening', async () => {
176176
const isOpen = false;
177177
const onToggle = vi.fn(createStateSetter(isOpen));
178178

179-
const { rerender } = renderPopover({
179+
const { rerender } = renderActionMenu({
180180
...baseProps,
181181
isOpen,
182182
onToggle,
183183
});
184184

185-
rerender(<Popover {...baseProps} isOpen />);
185+
rerender(<ActionMenu {...baseProps} isOpen />);
186186

187-
const popoverItems = screen.getAllByRole('menuitem');
187+
const actionMenuItems = screen.getAllByRole('menuitem');
188188

189-
expect(popoverItems[0]).toHaveFocus();
189+
expect(actionMenuItems[0]).toHaveFocus();
190190

191191
await flushMicrotasks();
192192
});
193193

194194
it('should move focus to the trigger element after closing', async () => {
195-
const { rerender } = renderPopover(baseProps);
195+
const { rerender } = renderActionMenu(baseProps);
196196

197-
rerender(<Popover {...baseProps} isOpen={false} />);
197+
rerender(<ActionMenu {...baseProps} isOpen={false} />);
198198

199-
const popoverTrigger = screen.getByRole('button');
199+
const trigger = screen.getByRole('button');
200200

201201
await waitFor(() => {
202-
expect(popoverTrigger).toHaveFocus();
202+
expect(trigger).toHaveFocus();
203203
});
204204

205205
await flushMicrotasks();
206206
});
207207

208208
it('should have no accessibility violations', async () => {
209-
const { container } = renderPopover(baseProps);
209+
const { container } = renderActionMenu(baseProps);
210210

211211
await act(async () => {
212212
const actual = await axe(container);
213213
expect(actual).toHaveNoViolations();
214214
});
215215
});
216216

217-
it('should render the popover with menu semantics by default ', async () => {
218-
renderPopover(baseProps);
217+
it('should render the action menu with menu semantics by default ', async () => {
218+
renderActionMenu(baseProps);
219219

220220
const menu = screen.getByRole('menu');
221221
expect(menu).toBeVisible();
@@ -225,8 +225,8 @@ describe('Popover', () => {
225225
await flushMicrotasks();
226226
});
227227

228-
it('should render the popover without menu semantics ', async () => {
229-
renderPopover({ ...baseProps, role: null });
228+
it('should render the action menu without menu semantics ', async () => {
229+
renderActionMenu({ ...baseProps, role: null });
230230

231231
const menu = screen.queryByRole('menu');
232232
expect(menu).toBeNull();
@@ -237,7 +237,7 @@ describe('Popover', () => {
237237
});
238238

239239
it('should hide dividers from the accessibility tree', async () => {
240-
const { baseElement } = renderPopover(baseProps);
240+
const { baseElement } = renderActionMenu(baseProps);
241241

242242
// eslint-disable-next-line testing-library/no-node-access
243243
const dividers = baseElement.querySelectorAll('hr[aria-hidden="true"');

packages/circuit-ui/components/Popover/Popover.stories.tsx renamed to packages/circuit-ui/components/ActionMenu/ActionMenu.stories.tsx

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,11 @@ import { useState } from 'react';
1919

2020
import { Button } from '../Button/index.js';
2121

22-
import { Popover, type PopoverProps } from './Popover.js';
22+
import { ActionMenu, type ActionMenuProps } from './ActionMenu.js';
2323

2424
export default {
25-
title: 'Components/Popover',
26-
component: Popover,
25+
title: 'Components/ActionMenu',
26+
component: ActionMenu,
2727
tags: ['status:stable'],
2828
argTypes: {
2929
children: { control: 'text' },
@@ -50,17 +50,17 @@ const actions = [
5050
},
5151
];
5252

53-
export const Base = (args: PopoverProps) => {
53+
export const Base = (args: ActionMenuProps) => {
5454
const [isOpen, setOpen] = useState(true);
5555

5656
return (
57-
<Popover
57+
<ActionMenu
5858
{...args}
5959
isOpen={isOpen}
6060
onToggle={setOpen}
6161
component={(props) => (
6262
<Button size="s" variant="secondary" {...props}>
63-
Open popover
63+
Open action menu
6464
</Button>
6565
)}
6666
/>
@@ -71,17 +71,17 @@ Base.args = {
7171
actions,
7272
};
7373

74-
export const Offset = (args: PopoverProps) => {
74+
export const Offset = (args: ActionMenuProps) => {
7575
const [isOpen, setOpen] = useState(true);
7676

7777
return (
78-
<Popover
78+
<ActionMenu
7979
{...args}
8080
isOpen={isOpen}
8181
onToggle={setOpen}
8282
component={(props) => (
8383
<Button size="s" variant="secondary" {...props}>
84-
Open popover
84+
Open action menu
8585
</Button>
8686
)}
8787
/>

0 commit comments

Comments
 (0)