Skip to content

Commit 35da22b

Browse files
fix(overflow-menu): return focus to trigger on esc (#20994)
* fix(overflow-menu): return focus to trigger on esc * test(overflow-menu): write test cases * test(overflow-menu): remove comments * fix(overflow-menu): revert change * test(overflow-menu): update tests * test(overflow-menu): update tests --------- Co-authored-by: Nikhil Tomar <63502271+2nikhiltom@users.noreply.github.com>
1 parent a1df9be commit 35da22b

File tree

3 files changed

+86
-11
lines changed

3 files changed

+86
-11
lines changed

packages/web-components/src/components/menu/__tests__/menu-test.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77

88
import '@carbon/web-components/es/components/menu/index.js';
9+
910
import { expect, fixture, html } from '@open-wc/testing';
1011

1112
describe('cds-menu', () => {
@@ -106,8 +107,8 @@ describe('cds-menu', () => {
106107

107108
expect(el.position[0]).to.be.a('number');
108109
expect(el.position[1]).to.be.a('number');
109-
expect(el.position[0]).to.be.lessThan(nearMaxX);
110-
expect(el.position[1]).to.be.lessThan(nearMaxY);
110+
expect(el.position[0]).to.be.at.most(nearMaxX);
111+
expect(el.position[1]).to.be.at.most(nearMaxY);
111112
});
112113
});
113114
});

packages/web-components/src/components/overflow-menu/__tests__/overflow-menu-test.js

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,21 @@
66
*/
77

88
import '@carbon/web-components/es/components/overflow-menu/index.js';
9+
910
import { expect, fixture, html } from '@open-wc/testing';
1011

1112
describe('cds-overflow-menu', () => {
13+
let originalBodyPosition;
14+
15+
beforeEach(() => {
16+
originalBodyPosition = document.body.style.position;
17+
document.body.style.position = 'relative';
18+
});
19+
20+
afterEach(() => {
21+
document.body.style.position = originalBodyPosition;
22+
});
23+
1224
const basicOverflowMenu = html`<cds-overflow-menu>
1325
<span slot="tooltip-content">Options</span>
1426
<cds-overflow-menu-body>
@@ -45,4 +57,44 @@ describe('cds-overflow-menu', () => {
4557
});
4658
});
4759
});
60+
61+
it('should handle Escape key to close menu', async () => {
62+
const el = await fixture(basicOverflowMenu);
63+
const menuBody = el.querySelector('cds-overflow-menu-body');
64+
65+
menuBody.open = true;
66+
67+
const event = new KeyboardEvent('keydown', {
68+
key: 'Escape',
69+
bubbles: true,
70+
cancelable: true,
71+
});
72+
73+
menuBody.dispatchEvent(event);
74+
75+
expect(menuBody.open).to.be.false;
76+
});
77+
78+
it('should close menu when a menu item is clicked', async () => {
79+
const el = await fixture(basicOverflowMenu);
80+
const menuBody = el.querySelector('cds-overflow-menu-body');
81+
82+
el.open = true;
83+
await el.updateComplete;
84+
await menuBody.updateComplete;
85+
86+
expect(el.open).to.be.true;
87+
expect(menuBody.open).to.be.true;
88+
89+
const items = menuBody.querySelectorAll('cds-overflow-menu-item');
90+
91+
items[0].click();
92+
93+
await new Promise((r) => setTimeout(r, 0));
94+
await el.updateComplete;
95+
await menuBody.updateComplete;
96+
97+
expect(el.open).to.be.false;
98+
expect(menuBody.open).to.be.false;
99+
});
48100
});

packages/web-components/src/components/overflow-menu/overflow-menu-body.ts

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,17 +5,18 @@
55
* LICENSE file in the root directory of this source tree.
66
*/
77

8-
import { html } from 'lit';
9-
import { property } from 'lit/decorators.js';
10-
import { prefix } from '../../globals/settings';
118
import CDSFloatingMenu, {
129
FLOATING_MENU_DIRECTION,
1310
} from '../floating-menu/floating-menu';
1411
import { NAVIGATION_DIRECTION, OVERFLOW_MENU_SIZE } from './defs';
12+
1513
import CDSOverflowMenuItem from './overflow-menu-item';
1614
import HostListener from '../../globals/decorators/host-listener';
17-
import { indexOf } from '../../globals/internal/collection-helpers';
1815
import { carbonElement as customElement } from '../../globals/decorators/carbon-element';
16+
import { html } from 'lit';
17+
import { indexOf } from '../../globals/internal/collection-helpers';
18+
import { prefix } from '../../globals/settings';
19+
import { property } from 'lit/decorators.js';
1920
import styles from './overflow-menu.scss?lit';
2021

2122
/**
@@ -118,7 +119,26 @@ class CDSOverflowMenuBody extends CDSFloatingMenu {
118119
}
119120

120121
if (key === 'Escape') {
122+
event.preventDefault();
123+
124+
const menuTrigger = this.parent as HTMLElement | null;
121125
this.open = false;
126+
127+
if (menuTrigger && 'open' in menuTrigger) {
128+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
129+
(menuTrigger as any).open = false;
130+
}
131+
132+
requestAnimationFrame(() => {
133+
const triggerButton =
134+
menuTrigger?.shadowRoot?.querySelector(
135+
`button.${prefix}--overflow-menu`
136+
) || menuTrigger?.querySelector(`button.${prefix}--overflow-menu`);
137+
138+
if (triggerButton) {
139+
(triggerButton as HTMLElement).focus();
140+
}
141+
});
122142
return;
123143
}
124144

@@ -131,21 +151,23 @@ class CDSOverflowMenuBody extends CDSFloatingMenu {
131151

132152
if (isInsideMenu) {
133153
event.preventDefault();
154+
155+
const menuTrigger = this.parent as HTMLElement | null;
134156
this.open = false;
135157

158+
if (menuTrigger && 'open' in menuTrigger) {
159+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
160+
(menuTrigger as any).open = false;
161+
}
162+
136163
requestAnimationFrame(() => {
137-
const menuTrigger = this.parent as HTMLElement | null;
138164
const triggerButton =
139165
menuTrigger?.shadowRoot?.querySelector(
140166
`button.${prefix}--overflow-menu`
141167
) || menuTrigger?.querySelector(`button.${prefix}--overflow-menu`);
142168

143169
if (triggerButton) {
144170
(triggerButton as HTMLElement).focus();
145-
} else {
146-
// eslint-disable-next-line no-console -- https://github.com/carbon-design-system/carbon/issues/20452
147-
console.warn('Could not find trigger button.');
148-
(document.body as HTMLElement).focus();
149171
}
150172
});
151173
}

0 commit comments

Comments
 (0)