Skip to content

Commit 57d9311

Browse files
author
Kubit
committed
Improve Desktop and Mobile Behavior for OliveMenu Component
Updated the OliveMenu component to differentiate behavior between desktop and mobile/tablet environments. On desktop, the component no longer acts as a dialog, allowing focus to move freely across the screen and ensuring it closes when losing focus, while still closing with the Escape key. For mobile and tablet, the popover now adheres to modal behavior as outlined by W3C ARIA practices.
1 parent 6fed45d commit 57d9311

File tree

9 files changed

+229
-62
lines changed

9 files changed

+229
-62
lines changed

src/components/oliveMenu/__test__/oliveMenu.test.tsx

Lines changed: 139 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,10 @@ import * as React from 'react';
33

44
import { axe } from 'jest-axe';
55

6+
import * as mediaHooks from '@/hooks/useMediaDevice/useMediaDevice';
67
import { renderProvider } from '@/tests/renderProvider/renderProvider.utility';
78
import { windowMatchMedia } from '@/tests/windowMatchMedia';
9+
import { DeviceBreakpointsType } from '@/types';
810

911
import { OliveMenu } from '../oliveMenu';
1012
import { IOliveMenu, OliveMenuListOptions } from '../types';
@@ -74,8 +76,15 @@ const mockProps: IOliveMenu = {
7476
window.matchMedia = windowMatchMedia();
7577

7678
describe('OliveMenu component', () => {
79+
afterEach(() => {
80+
window.matchMedia = windowMatchMedia();
81+
jest.clearAllMocks();
82+
jest.resetAllMocks();
83+
jest.restoreAllMocks();
84+
});
85+
7786
it('Should render OliveMenu', async () => {
78-
const { container } = renderProvider(<OliveMenu {...mockProps} />);
87+
const { container } = renderProvider(<OliveMenu {...mockProps} ref={jest.fn()} />);
7988

8089
const oliveMenu = screen.getByText(
8190
(_content, element) => element?.tagName.toLowerCase() === 'button'
@@ -94,10 +103,55 @@ describe('OliveMenu component', () => {
94103
const openButton = screen.getAllByText('Options');
95104
fireEvent.click(openButton[0]);
96105

97-
const menu = screen.getByText(
98-
(content, element) => element?.tagName.toLowerCase() === 'dialog'
99-
);
100-
expect(menu).toBeInTheDocument();
106+
// Element in the popover
107+
const popoverElement = screen.getByText('list 1');
108+
expect(popoverElement).toBeInTheDocument();
109+
110+
const results = await axe(container);
111+
expect(container).toHTMLValidate({
112+
rules: {
113+
'no-inline-style': 'off',
114+
},
115+
});
116+
expect(results).toHaveNoViolations();
117+
});
118+
119+
it('When desktop popover will be a div', async () => {
120+
window.matchMedia = windowMatchMedia('onlyDesktop');
121+
jest
122+
.spyOn(mediaHooks, 'useMediaDevice')
123+
.mockImplementation(() => DeviceBreakpointsType.DESKTOP);
124+
125+
const { container } = renderProvider(<OliveMenu {...mockProps} />);
126+
127+
const openButton = screen.getAllByText('Options');
128+
fireEvent.click(openButton[0]);
129+
130+
// Element in the popover
131+
const popover = container.querySelector('[data-popover]');
132+
expect(popover?.tagName).toBe('DIV');
133+
134+
const results = await axe(container);
135+
expect(container).toHTMLValidate({
136+
rules: {
137+
'no-inline-style': 'off',
138+
},
139+
});
140+
expect(results).toHaveNoViolations();
141+
});
142+
143+
it('When mobile or tablet, popover will be a dialog', async () => {
144+
window.matchMedia = windowMatchMedia('onlyMobile');
145+
jest.spyOn(mediaHooks, 'useMediaDevice').mockImplementation(() => DeviceBreakpointsType.MOBILE);
146+
147+
const { container } = renderProvider(<OliveMenu {...mockProps} />);
148+
149+
const openButton = screen.getAllByText('Options');
150+
fireEvent.click(openButton[0]);
151+
152+
// Element in the popover
153+
const popover = container.querySelector('[data-popover]');
154+
expect(popover?.tagName).toBe('DIALOG');
101155

102156
const results = await axe(container);
103157
expect(container).toHTMLValidate({
@@ -127,38 +181,102 @@ describe('OliveMenu component', () => {
127181
const openButton = screen.getAllByText('Options');
128182
fireEvent.click(openButton[0]);
129183

130-
const menu = screen.getByText(
131-
(_content, element) => element?.tagName.toLowerCase() === 'dialog'
132-
);
184+
// Element in the popover
185+
const popoverElement = screen.getByText('list 1');
133186

134187
const closeButton = screen.getByLabelText(
135188
mockProps.actionBottomSheetStructure?.closeIcon?.altText as string
136189
);
137190
fireEvent.click(closeButton);
138191

139-
expect(menu).not.toBeInTheDocument();
192+
expect(popoverElement).not.toBeInTheDocument();
140193
});
141194

142-
it('Should close the menu when the popover is closed automatically', async () => {
195+
it('Should close the menu when lose the focus outside', async () => {
196+
const handleOpenClose = jest.fn();
143197
renderProvider(
144-
<div>
145-
<button>external</button>
146-
<OliveMenu {...mockProps} />
147-
</div>
198+
<OliveMenu {...mockProps} dataTestId="OliveMenu" onOpenClose={handleOpenClose} />
148199
);
149200

201+
const container = screen.getByTestId('OliveMenuContainer');
202+
203+
// Lossing the focus when the popover is closed
204+
act(() => {
205+
fireEvent.blur(container);
206+
});
207+
expect(handleOpenClose).not.toHaveBeenCalled();
208+
209+
// Open the popover
150210
const openButton = screen.getAllByText('Options');
151-
fireEvent.click(openButton[0]);
211+
act(() => {
212+
fireEvent.click(openButton[0]);
213+
});
152214

153-
const menu = screen.getByText(
154-
(_content, element) => element?.tagName.toLowerCase() === 'dialog'
155-
);
215+
// Element in the popover
216+
const popoverElement = screen.getByText('list 1');
217+
expect(popoverElement).toBeInTheDocument();
218+
219+
// Move focus to an element inside the popover
220+
// Do not close the popover
221+
const option = screen.getAllByText('option 1')[0];
222+
act(() => {
223+
fireEvent.blur(container, { relatedTarget: option });
224+
});
225+
expect(popoverElement).toBeInTheDocument();
226+
227+
// Close the popover lossing the focus
228+
act(() => {
229+
fireEvent.blur(container);
230+
});
156231

157-
const external = screen.getByText('external');
232+
expect(handleOpenClose).toHaveBeenCalledWith(false, expect.anything());
233+
234+
expect(popoverElement).not.toBeInTheDocument();
235+
});
236+
237+
it('Should close the menu when pressing Escape', async () => {
238+
const handleOpenClose = jest.fn();
239+
renderProvider(<OliveMenu {...mockProps} onOpenClose={handleOpenClose} />);
240+
241+
// Open the popover
242+
const openButton = screen.getAllByText('Options');
243+
244+
// Simulate a inner element has the focus
245+
openButton[0].focus();
246+
247+
// Press escape when is already close
158248
await act(async () => {
159-
fireEvent.mouseUp(external);
249+
fireEvent.keyDown(window, {
250+
key: 'Escape',
251+
code: 'Escape',
252+
keyCode: 27,
253+
charCode: 27,
254+
});
160255
});
256+
expect(handleOpenClose).not.toHaveBeenCalled();
257+
258+
act(() => {
259+
fireEvent.click(openButton[0]);
260+
});
261+
262+
// Element in the popover
263+
const popoverElement = screen.getByText('list 1');
264+
265+
// Simulate a inner element has the focus
266+
openButton[0].focus();
267+
268+
// Press escape
269+
await act(async () => {
270+
fireEvent.keyDown(window, {
271+
key: 'Escape',
272+
code: 'Escape',
273+
keyCode: 27,
274+
charCode: 27,
275+
});
276+
});
277+
278+
expect(handleOpenClose).toHaveBeenCalledWith(false);
161279

162-
expect(menu).not.toBeInTheDocument();
280+
expect(popoverElement).not.toBeInTheDocument();
163281
});
164282
});

src/components/oliveMenu/oliveMenu.styled.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ type OliveMenuStylesProps = {
1010

1111
export const OliveMenuStyled = styled.div<OliveMenuStylesProps>`
1212
${({ styles }) => getStyles(styles.container)};
13+
*[data-popover] {
14+
${({ styles }) => getStyles(styles.popover)};
15+
}
1316
`;
1417

1518
export const ButtonContainer = styled.div<OliveMenuStylesProps>`

src/components/oliveMenu/oliveMenu.tsx

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as React from 'react';
22

3+
import { useEscPressed, useMediaDevice } from '@/hooks';
34
import { useStyles } from '@/hooks/useStyles/useStyles';
45
import { ErrorBoundary, FallbackComponent } from '@/provider/errorBoundary';
56

@@ -10,50 +11,61 @@ const OLIVE_MENU_STYLES = 'OLIVE_MENU_STYLES';
1011

1112
const OliveMenuComponent = React.forwardRef(
1213
<V extends string | unknown>(
13-
{
14-
variant,
15-
ctv,
16-
trigger,
17-
popover,
18-
actionBottomSheetStructure,
19-
onOpenClose,
20-
...props
21-
}: IOliveMenu<V>,
14+
{ variant, ctv, trigger, actionBottomSheetStructure, onOpenClose, ...props }: IOliveMenu<V>,
2215
ref: React.ForwardedRef<HTMLDivElement> | undefined | null
2316
): JSX.Element => {
2417
const [open, setOpen] = React.useState<boolean>(false);
2518
const styles = useStyles<OliveMenuGlobalStylesType, V>(OLIVE_MENU_STYLES, variant, ctv);
19+
const device = useMediaDevice();
20+
21+
const innerRef = React.useRef<HTMLDivElement>(null);
22+
React.useImperativeHandle(ref, () => innerRef.current as HTMLDivElement, []);
23+
24+
const handlePressScape = React.useCallback(() => {
25+
// Do nothing if already closed
26+
if (!open) {
27+
return;
28+
}
29+
setOpen(false);
30+
onOpenClose?.(false);
31+
}, [open]);
32+
33+
useEscPressed({ execute: handlePressScape, element: innerRef });
2634

2735
const handleTriggerClick = e => {
2836
setOpen(!open);
2937
onOpenClose?.(!open, e);
3038
trigger?.onClick?.(e);
3139
};
3240

33-
const handlePopoverCloseInternally = () => {
41+
const handleCloseIconClick = e => {
3442
setOpen(false);
35-
onOpenClose?.(false);
36-
popover?.onCloseInternally?.();
43+
onOpenClose?.(false, e);
44+
actionBottomSheetStructure?.closeIcon?.onClick?.(e);
3745
};
3846

39-
const handleCloseIconClick = e => {
47+
const handleBlur: React.FocusEventHandler<HTMLDivElement> = e => {
48+
// Do nothing if the new focus is inside the component or already closed
49+
if (e.currentTarget.contains(e.relatedTarget) || !open) {
50+
return;
51+
}
4052
setOpen(false);
4153
onOpenClose?.(false, e);
42-
actionBottomSheetStructure?.closeIcon?.onClick?.(e);
4354
};
4455

4556
return (
4657
<OliveMenuStandAlone
4758
{...props}
48-
ref={ref}
59+
ref={innerRef}
4960
actionBottomSheetStructure={{
5061
...actionBottomSheetStructure,
5162
closeIcon: { ...actionBottomSheetStructure?.closeIcon, onClick: handleCloseIconClick },
5263
}}
64+
device={device}
5365
open={open}
54-
popover={{ ...popover, onCloseInternally: handlePopoverCloseInternally }}
5566
styles={styles}
5667
trigger={trigger && { ...trigger, onClick: handleTriggerClick }}
68+
onBlur={handleBlur}
5769
/>
5870
);
5971
}

0 commit comments

Comments
 (0)