Skip to content

Commit

Permalink
fix(list,menu): list items left right keyboard navigation
Browse files Browse the repository at this point in the history 
PiperOrigin-RevId: 565526741
  • Loading branch information
@copybara-github
Elliott Marquez authored and copybara-github committed Sep 15, 2023
1 parent e444de3 commit fad6104
Show file tree
Hide file tree
Showing 3 changed files with 189 additions and 22 deletions.
39 changes: 29 additions & 10 deletions list/internal/list.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,16 @@ import {requestUpdateOnAriaChange} from '../../internal/aria/delegate.js';

import {ListItem} from './listitem/list-item.js';

const NAVIGABLE_KEYS = {
ArrowDown: 'ArrowDown',
ArrowUp: 'ArrowUp',
Home: 'Home',
End: 'End',
/**
* Default keys that trigger navigation.
*/
export const NAVIGABLE_KEYS = {
ARROW_DOWN: 'ArrowDown',
ARROW_LEFT: 'ArrowLeft',
ARROW_UP: 'ArrowUp',
ARROW_RIGHT: 'ArrowRight',
HOME: 'Home',
END: 'End',
} as const;

/**
Expand Down Expand Up @@ -100,7 +105,13 @@ export class List extends LitElement {
*
* @param event {KeyboardEvent} The keyboard event that triggers this handler.
*/
private handleKeydown(event: KeyboardEvent) {
private async handleKeydown(event: KeyboardEvent) {
// Allow event to bubble to check for defaultPrevented
// TODO(b/293323995): clean up when we find out why await 0 doesn't work.
await new Promise(res => {
setTimeout(res, 0);
});

const key = event.key;
if (event.defaultPrevented || !isNavigableKey(key)) {
return;
Expand All @@ -120,24 +131,32 @@ export class List extends LitElement {

event.preventDefault();

const isLTR = getComputedStyle(this).direction === 'ltr';
const inlinePrevious =
isLTR ? NAVIGABLE_KEYS.ARROW_LEFT : NAVIGABLE_KEYS.ARROW_RIGHT;
const inlineNext =
isLTR ? NAVIGABLE_KEYS.ARROW_RIGHT : NAVIGABLE_KEYS.ARROW_LEFT;

switch (key) {
// Activate the next item
case NAVIGABLE_KEYS.ArrowDown:
case NAVIGABLE_KEYS.ARROW_DOWN:
case inlineNext:
this.activateNextItemInternal(items, activeItemRecord);
break;

// Activate the previous item
case NAVIGABLE_KEYS.ArrowUp:
case NAVIGABLE_KEYS.ARROW_UP:
case inlinePrevious:
this.activatePreviousItemInternal(items, activeItemRecord);
break;

// Activate the first item
case NAVIGABLE_KEYS.Home:
case NAVIGABLE_KEYS.HOME:
List.activateFirstItem(items);
break;

// Activate the last item
case NAVIGABLE_KEYS.End:
case NAVIGABLE_KEYS.END:
List.activateLastItem(items);
break;

Expand Down
143 changes: 139 additions & 4 deletions list/list_test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,89 @@ describe('<md-list>', () => {
expect(third.active).toBeTrue();
});

it('preventDefault on keydown prevents navigation', async () => {
const root = env.render(html`
<md-list @keydown=${(e: KeyboardEvent) => {
e.preventDefault();
}}>
<md-list-item></md-list-item>
<md-list-item active></md-list-item>
<md-list-item></md-list-item>
</md-list>`);

const listEl = root.querySelector('md-list')!;
const listHarness = new ListHarness(listEl);
const [first, second, third] =
Array.from(root.querySelectorAll('md-list-item'));

await env.waitForStability();

expect(first.active).toBeFalse();
expect(second.active).toBeTrue();
expect(third.active).toBeFalse();

await listHarness.pressHandledKey('ArrowDown');
await env.waitForStability();

expect(first.active).toBeFalse();
expect(second.active).toBeTrue();
expect(third.active).toBeFalse();
});

it('ArrowRight in LTR activates the next item', async () => {
const root = env.render(html`
<md-list>
<md-list-item></md-list-item>
<md-list-item active></md-list-item>
<md-list-item></md-list-item>
</md-list>`);

const listEl = root.querySelector('md-list')!;
const listHarness = new ListHarness(listEl);
const [first, second, third] =
Array.from(root.querySelectorAll('md-list-item'));

await env.waitForStability();

expect(first.active).toBeFalse();
expect(second.active).toBeTrue();
expect(third.active).toBeFalse();

await listHarness.pressHandledKey('ArrowRight');
await env.waitForStability();

expect(first.active).toBeFalse();
expect(second.active).toBeFalse();
expect(third.active).toBeTrue();
});

it('ArrowLeft in RTL activates the next item', async () => {
const root = env.render(html`
<md-list dir="rtl">
<md-list-item></md-list-item>
<md-list-item active></md-list-item>
<md-list-item></md-list-item>
</md-list>`);

const listEl = root.querySelector('md-list')!;
const listHarness = new ListHarness(listEl);
const [first, second, third] =
Array.from(root.querySelectorAll('md-list-item'));

await env.waitForStability();

expect(first.active).toBeFalse();
expect(second.active).toBeTrue();
expect(third.active).toBeFalse();

await listHarness.pressHandledKey('ArrowLeft');
await env.waitForStability();

expect(first.active).toBeFalse();
expect(second.active).toBeFalse();
expect(third.active).toBeTrue();
});

it('ArrowDown will loop around', async () => {
const root = env.render(html`
<md-list>
Expand Down Expand Up @@ -224,6 +307,58 @@ describe('<md-list>', () => {
expect(third.active).toBeFalse();
});

it('ArrowLeft in LTR activates the previous item', async () => {
const root = env.render(html`
<md-list>
<md-list-item></md-list-item>
<md-list-item active></md-list-item>
<md-list-item></md-list-item>
</md-list>`);
const listEl = root.querySelector('md-list')!;
const listHarness = new ListHarness(listEl);
const [first, second, third] =
Array.from(root.querySelectorAll('md-list-item'));

await env.waitForStability();

expect(first.active).toBeFalse();
expect(second.active).toBeTrue();
expect(third.active).toBeFalse();

await listHarness.pressHandledKey('ArrowLeft');
await env.waitForStability();

expect(first.active).toBeTrue();
expect(second.active).toBeFalse();
expect(third.active).toBeFalse();
});

it('ArrowRight in RTL activates the previous item', async () => {
const root = env.render(html`
<md-list dir="rtl">
<md-list-item></md-list-item>
<md-list-item active></md-list-item>
<md-list-item></md-list-item>
</md-list>`);
const listEl = root.querySelector('md-list')!;
const listHarness = new ListHarness(listEl);
const [first, second, third] =
Array.from(root.querySelectorAll('md-list-item'));

await env.waitForStability();

expect(first.active).toBeFalse();
expect(second.active).toBeTrue();
expect(third.active).toBeFalse();

await listHarness.pressHandledKey('ArrowRight');
await env.waitForStability();

expect(first.active).toBeTrue();
expect(second.active).toBeFalse();
expect(third.active).toBeFalse();
});

it('activatePreviousItem will loop around', async () => {
const root = env.render(html`
<md-list>
Expand Down Expand Up @@ -1120,7 +1255,7 @@ describe('<md-list-item> link', () => {

expect(internalRoot.tagName).toBe('A');
});

it('setting type and href does not render a role', async () => {
const root = env.render(
html`<md-list-item type="menuitem" href="https://google.com"></md-list-item>`);
Expand All @@ -1134,10 +1269,10 @@ describe('<md-list-item> link', () => {

expect(internalRoot.hasAttribute('role')).toBe(false);
});

it('setting target without href renders nothing', async () => {
const root = env.render(
html`<md-list-item target="_blank"></md-list-item>`);
const root =
env.render(html`<md-list-item target="_blank"></md-list-item>`);

const listItem = root.querySelector('md-list-item')!;

Expand Down
29 changes: 21 additions & 8 deletions menu/internal/submenuitem/sub-menu-item.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,20 @@ export class SubMenuItem extends MenuItemEl {
protected override onKeydown(event: KeyboardEvent) {
const shouldOpenSubmenu = this.isSubmenuOpenKey(event.code);

if (event.defaultPrevented) return;
if (event.defaultPrevented || event.target !== this.listItemRoot) return;

const openedWithLR = shouldOpenSubmenu &&
(NAVIGABLE_KEY.LEFT === event.code ||
NAVIGABLE_KEY.RIGHT === event.code);

if (event.code === SELECTION_KEY.SPACE) {
// prevent space from scrolling. Only open the submenu.
if (event.code === SELECTION_KEY.SPACE || openedWithLR) {
// prevent space from scrolling and Left + Right from selecting previous /
// next items or opening / closing parent menus. Only open the submenu.
event.preventDefault();

if (openedWithLR) {
event.stopPropagation();
}
}

if (!shouldOpenSubmenu) {
Expand Down Expand Up @@ -204,17 +213,21 @@ export class SubMenuItem extends MenuItemEl {
this.selected = false;
}

private async onSubMenuKeydown(event: KeyboardEvent) {
private onSubMenuKeydown(event: KeyboardEvent) {
if (event.defaultPrevented) return;
const shouldClose = this.isSubmenuCloseKey(event.code);

const {close: shouldClose, keyCode} = this.isSubmenuCloseKey(event.code);
if (!shouldClose) return;

// Communicate that it's handled so that we don't accidentally close every
// parent menu. Additionally, we want to isolate things like the typeahead
// keydowns from bubbling up to the parent menu and confounding things.
event.preventDefault();

if (keyCode === NAVIGABLE_KEY.LEFT || keyCode === NAVIGABLE_KEY.RIGHT) {
// Prevent this from bubbling to parents
event.stopPropagation();
}

this.close(() => {
List.deactivateActiveItem(this.submenuEl!.items);
this.listItemRoot?.focus();
Expand Down Expand Up @@ -320,9 +333,9 @@ export class SubMenuItem extends MenuItemEl {
switch (code) {
case arrowEnterKey:
case KEYDOWN_CLOSE_KEYS.ESCAPE:
return true;
return {close: true, keyCode: code} as const;
default:
return false;
return {close: false} as const;
}
}

Expand Down

0 comments on commit fad6104

Please sign in to comment.