Skip to content

Commit 5c449ad

Browse files
fix: mouse/keyboard interactions of menu/menu-item (#2747)
## Description: Closes #2372. Previously this PR: #2658 The problem is related to activation of focus-visible. When we click the dropdown menu with the mouse, the focus is put on the menu-item correctly with "focus", however, the focus-visible that puts the border around the item is not activated. This property only activates after a keyboard event. So for example in Safari, if you do "arrowdown" and then click "arrowright" (or any other key we are not catching), the outline suddenly appears because an event key was propagated. My solution was to force this css to appear, and also remove it when not needed. Please let me know if you know a better way to handle this. ## Definition of Reviewable: - [x] Stories (features, a11y) are created/updated - [X] relevant tickets are linked
1 parent d4c0cd2 commit 5c449ad

File tree

5 files changed

+118
-2
lines changed

5 files changed

+118
-2
lines changed

.changeset/shiny-coins-lay.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@solid-design-system/components': minor
3+
---
4+
5+
Fixed `sd-menu` keyboard interaction issue after enabling it with a mouse click.

packages/components/src/components/dropdown/dropdown.test.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import '../../../dist/solid-components';
22
import { expect, fixture, html, waitUntil } from '@open-wc/testing';
33
import { sendKeys, sendMouse } from '@web/test-runner-commands';
44
import sinon from 'sinon';
5+
import type { SdButton, SdMenu, SdMenuItem } from 'src/solid-components';
56
import type SdDropdown from './dropdown';
67

78
describe('<sd-dropdown>', () => {
@@ -361,6 +362,81 @@ describe('<sd-dropdown>', () => {
361362
});
362363
}
363364

365+
it('adds focus to the first menu item when opened via keyboard', async () => {
366+
const el = await fixture<SdDropdown>(html`
367+
<sd-dropdown distance="4" rounded>
368+
<sd-button variant="secondary" slot="trigger">
369+
Menu
370+
<sd-icon library="_internal" name="chevron-down" slot="icon-right"></sd-icon>
371+
</sd-button>
372+
373+
<sd-menu>
374+
<sd-menu-item>Menu item 1</sd-menu-item>
375+
<sd-menu-item>Menu item 2</sd-menu-item>
376+
<sd-menu-item>Menu item 3</sd-menu-item>
377+
</sd-menu>
378+
</sd-dropdown>
379+
`);
380+
381+
const button = el.querySelector<SdButton>('sd-button')!;
382+
const menu = el.querySelector<SdMenu>('sd-menu')!;
383+
const items = menu.querySelectorAll<SdMenuItem>('sd-menu-item');
384+
const firstItem = items[0];
385+
386+
// click button to open dropdown
387+
button.focus();
388+
button.click();
389+
await el.updateComplete;
390+
391+
// navigate down
392+
await sendKeys({ press: 'ArrowDown' });
393+
await firstItem.updateComplete;
394+
395+
expect(firstItem.classList.contains('menu-item-focus')).to.be.true;
396+
});
397+
398+
it('removes focus from menu items when clicking outside the dropdown', async () => {
399+
const el = await fixture<SdDropdown>(html`
400+
<sd-dropdown distance="4" rounded>
401+
<sd-button variant="secondary" slot="trigger">
402+
Menu
403+
<sd-icon library="_internal" name="chevron-down" slot="icon-right"></sd-icon>
404+
</sd-button>
405+
406+
<sd-menu>
407+
<sd-menu-item>Menu item 1</sd-menu-item>
408+
<sd-menu-item>Menu item 2</sd-menu-item>
409+
<sd-menu-item>Menu item 3</sd-menu-item>
410+
</sd-menu>
411+
</sd-dropdown>
412+
`);
413+
414+
const button = el.querySelector<SdButton>('sd-button')!;
415+
const menu = el.querySelector<SdMenu>('sd-menu')!;
416+
const items = menu.querySelectorAll<SdMenuItem>('sd-menu-item');
417+
const firstItem = items[0];
418+
419+
// click button to open dropdown
420+
button.focus();
421+
button.click();
422+
await el.updateComplete;
423+
424+
// navigate down
425+
await sendKeys({ press: 'ArrowDown' });
426+
await firstItem.updateComplete;
427+
428+
expect(firstItem.classList.contains('menu-item-focus')).to.be.true;
429+
430+
// click outside
431+
document.body.dispatchEvent(new MouseEvent('click', { bubbles: true, composed: true }));
432+
await el.updateComplete;
433+
434+
// check all menu items have lost focus
435+
items.forEach(item => {
436+
expect(item.classList.contains('menu-item-focus')).to.be.false;
437+
});
438+
});
439+
364440
// it('should close and stop propagating the keydown event when Escape is pressed and the dropdown is open ', async () => {
365441
// const el = await fixture<SdDropdown>(html`
366442
// <sd-dropdown open>

packages/components/src/components/dropdown/dropdown.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,12 @@ export default class SdDropdown extends SolidElement {
269269

270270
handleTriggerClick() {
271271
if (this.open) {
272+
const menu = this.getMenu();
273+
if (menu) {
274+
const current = menu.getCurrentItem();
275+
current?.classList.remove('menu-item-focus');
276+
}
277+
272278
this.hide();
273279
} else {
274280
this.show();
@@ -280,7 +286,7 @@ export default class SdDropdown extends SolidElement {
280286
if (event.key === 'Escape' && this.open) {
281287
event.stopPropagation();
282288
this.focusOnTrigger();
283-
this.hide();
289+
this.handleTriggerClick();
284290
return;
285291
}
286292

@@ -316,11 +322,13 @@ export default class SdDropdown extends SolidElement {
316322
if (event.key === 'ArrowDown' || event.key === 'Home') {
317323
menu.setCurrentItem(firstMenuItem);
318324
firstMenuItem.focus();
325+
firstMenuItem.classList.add('menu-item-focus');
319326
}
320327

321328
if (event.key === 'ArrowUp' || event.key === 'End') {
322329
menu.setCurrentItem(lastMenuItem);
323330
lastMenuItem.focus();
331+
lastMenuItem.classList.add('menu-item-focus');
324332
}
325333
});
326334
}

packages/components/src/components/menu-item/menu-item.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,10 @@ export default class SdMenuItem extends SolidElement {
229229
@apply focus-outline outline-offset-[-2px];
230230
}
231231
232+
:host(.menu-item-focus) [part='base'] {
233+
@apply focus-outline outline-offset-[-2px];
234+
}
235+
232236
:host(:hover) [part='base'] {
233237
@apply outline-none;
234238
}

packages/components/src/components/menu/menu.ts

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,21 @@ export default class SdMenu extends SolidElement {
3434
connectedCallback() {
3535
super.connectedCallback();
3636
this.setAttribute('role', 'menu');
37+
38+
document.addEventListener('click', this.handleOutsideClick);
39+
}
40+
41+
disconnectedCallback() {
42+
super.disconnectedCallback();
43+
document.removeEventListener('click', this.handleOutsideClick);
3744
}
3845

46+
private handleOutsideClick = (event: MouseEvent) => {
47+
if (!this.contains(event.target as Node)) {
48+
this.getAllItems().forEach(item => item.classList.remove('menu-item-focus'));
49+
}
50+
};
51+
3952
private handleClick(event: MouseEvent) {
4053
const menuItemTypes = ['menuitem', 'menuitemcheckbox'];
4154

@@ -61,12 +74,20 @@ export default class SdMenu extends SolidElement {
6174
}
6275

6376
private handleKeyDown(event: KeyboardEvent) {
77+
// Remove focus css class on escape
78+
if (event.key === 'Escape') {
79+
const item = this.getCurrentItem();
80+
item?.classList.remove('menu-item-focus');
81+
}
82+
6483
// Make a selection when pressing enter or space
65-
if (event.key === 'Enter' || event.key === ' ') {
84+
else if (event.key === 'Enter' || event.key === ' ') {
6685
const item = this.getCurrentItem();
6786
event.preventDefault();
6887
event.stopPropagation();
6988

89+
item?.classList.remove('menu-item-focus');
90+
7091
// Simulate a click to support @click handlers on menu items that also work with the keyboard
7192
item?.click();
7293
}
@@ -76,6 +97,7 @@ export default class SdMenu extends SolidElement {
7697
const items = this.getAllItems();
7798
const activeItem = this.getCurrentItem();
7899
let index = activeItem ? items.indexOf(activeItem) : 0;
100+
activeItem?.classList.remove('menu-item-focus');
79101

80102
if (items.length > 0) {
81103
event.preventDefault();
@@ -110,6 +132,7 @@ export default class SdMenu extends SolidElement {
110132

111133
this.setCurrentItem(items[index]);
112134
items[index].focus();
135+
items[index].classList.add('menu-item-focus');
113136
}
114137
}
115138
}

0 commit comments

Comments
 (0)