From c6ffd70fc82060d894e4f4ef7fc43a1fb15e2a65 Mon Sep 17 00:00:00 2001 From: Elizabeth Mitchell Date: Mon, 26 Feb 2024 13:46:12 -0800 Subject: [PATCH] feat(menu): add `no-navigation-wrap` to fix select accessibility PiperOrigin-RevId: 610514684 --- list/internal/list-controller.ts | 55 +++++++++++++++++++----- list/internal/list-navigation-helpers.ts | 32 +++++++++++++- menu/internal/menu.ts | 9 ++++ select/internal/select.ts | 1 + 4 files changed, 85 insertions(+), 12 deletions(-) diff --git a/list/internal/list-controller.ts b/list/internal/list-controller.ts index fe1dc8265..707ab2fb0 100644 --- a/list/internal/list-controller.ts +++ b/list/internal/list-controller.ts @@ -71,6 +71,11 @@ export interface ListControllerConfig { * disabled. */ isActivatable?: (item: Item) => boolean; + /** + * Whether or not navigating past the end of the list wraps to the beginning + * and vice versa. Defaults to true. + */ + wrapNavigation?: () => boolean; } /** @@ -84,6 +89,7 @@ export class ListController { private readonly activateItem: (item: Item) => void; private readonly isNavigableKey: (key: string) => boolean; private readonly isActivatable?: (item: Item) => boolean; + private readonly wrapNavigation: () => boolean; constructor(config: ListControllerConfig) { const { @@ -94,6 +100,7 @@ export class ListController { activateItem, isNavigableKey, isActivatable, + wrapNavigation, } = config; this.isItem = isItem; this.getPossibleItems = getPossibleItems; @@ -102,6 +109,7 @@ export class ListController { this.activateItem = activateItem; this.isNavigableKey = isNavigableKey; this.isActivatable = isActivatable; + this.wrapNavigation = wrapNavigation ?? (() => true); } /** @@ -149,10 +157,6 @@ export class ListController { const activeItemRecord = getActiveItem(items, this.isActivatable); - if (activeItemRecord) { - activeItemRecord.item.tabIndex = -1; - } - event.preventDefault(); const isRtl = this.isRtl(); @@ -163,32 +167,53 @@ export class ListController { ? NavigableKeys.ArrowLeft : NavigableKeys.ArrowRight; + let nextActiveItem: Item | null = null; switch (key) { // Activate the next item case NavigableKeys.ArrowDown: case inlineNext: - activateNextItem(items, activeItemRecord, this.isActivatable); + nextActiveItem = activateNextItem( + items, + activeItemRecord, + this.isActivatable, + this.wrapNavigation(), + ); break; // Activate the previous item case NavigableKeys.ArrowUp: case inlinePrevious: - activatePreviousItem(items, activeItemRecord, this.isActivatable); + nextActiveItem = activatePreviousItem( + items, + activeItemRecord, + this.isActivatable, + this.wrapNavigation(), + ); break; // Activate the first item case NavigableKeys.Home: - activateFirstItem(items, this.isActivatable); + nextActiveItem = activateFirstItem(items, this.isActivatable); break; // Activate the last item case NavigableKeys.End: - activateLastItem(items, this.isActivatable); + nextActiveItem = activateLastItem(items, this.isActivatable); break; default: break; } + + if ( + nextActiveItem && + activeItemRecord && + activeItemRecord.item !== nextActiveItem + ) { + // If a new item was activated, remove the tabindex of the previous + // activated item. + activeItemRecord.item.tabIndex = -1; + } }; /** @@ -203,7 +228,12 @@ export class ListController { if (activeItemRecord) { activeItemRecord.item.tabIndex = -1; } - return activateNextItem(items, activeItemRecord, this.isActivatable); + return activateNextItem( + items, + activeItemRecord, + this.isActivatable, + this.wrapNavigation(), + ); } /** @@ -218,7 +248,12 @@ export class ListController { if (activeItemRecord) { activeItemRecord.item.tabIndex = -1; } - return activatePreviousItem(items, activeItemRecord, this.isActivatable); + return activatePreviousItem( + items, + activeItemRecord, + this.isActivatable, + this.wrapNavigation(), + ); } /** diff --git a/list/internal/list-navigation-helpers.ts b/list/internal/list-navigation-helpers.ts index bed2a5532..f099b15f2 100644 --- a/list/internal/list-navigation-helpers.ts +++ b/list/internal/list-navigation-helpers.ts @@ -162,15 +162,23 @@ export function getLastActivatableItem( * @param index {{index: number}} The index to search from. * @param isActivatable Function to determine if an item can be activated. * Defaults to non-disabled items. + * @param wrap If true, then the next item at the end of the list is the first + * item. Defaults to true. * @return The next activatable item or `null` if none are activatable. */ export function getNextItem( items: Item[], index: number, isActivatable = isItemNotDisabled, + wrap = true, ) { for (let i = 1; i < items.length; i++) { const nextIndex = (i + index) % items.length; + if (nextIndex < index && !wrap) { + // Return if the index loops back to the beginning and not wrapping. + return null; + } + const item = items[nextIndex]; if (isActivatable(item)) { return item; @@ -187,15 +195,23 @@ export function getNextItem( * @param index {{index: number}} The index to search from. * @param isActivatable Function to determine if an item can be activated. * Defaults to non-disabled items. + * @param wrap If true, then the previous item at the beginning of the list is + * the last item. Defaults to true. * @return The previous activatable item or `null` if none are activatable. */ export function getPrevItem( items: Item[], index: number, isActivatable = isItemNotDisabled, + wrap = true, ) { for (let i = 1; i < items.length; i++) { const prevIndex = (index - i + items.length) % items.length; + if (prevIndex > index && !wrap) { + // Return if the index loops back to the end and not wrapping. + return null; + } + const item = items[prevIndex]; if (isActivatable(item)) { @@ -214,9 +230,15 @@ export function activateNextItem( items: Item[], activeItemRecord: null | ItemRecord, isActivatable = isItemNotDisabled, + wrap = true, ): Item | null { if (activeItemRecord) { - const next = getNextItem(items, activeItemRecord.index, isActivatable); + const next = getNextItem( + items, + activeItemRecord.index, + isActivatable, + wrap, + ); if (next) { next.tabIndex = 0; @@ -237,9 +259,15 @@ export function activatePreviousItem( items: Item[], activeItemRecord: null | ItemRecord, isActivatable = isItemNotDisabled, + wrap = true, ): Item | null { if (activeItemRecord) { - const prev = getPrevItem(items, activeItemRecord.index, isActivatable); + const prev = getPrevItem( + items, + activeItemRecord.index, + isActivatable, + wrap, + ); if (prev) { prev.tabIndex = 0; prev.focus(); diff --git a/menu/internal/menu.ts b/menu/internal/menu.ts index a98db077f..4be04447e 100644 --- a/menu/internal/menu.ts +++ b/menu/internal/menu.ts @@ -226,6 +226,14 @@ export abstract class Menu extends LitElement { @property({attribute: 'default-focus'}) defaultFocus: FocusState = FocusState.FIRST_ITEM; + /** + * Turns off navigation wrapping. By default, navigating past the end of the + * menu items will wrap focus back to the beginning and vice versa. Use this + * for ARIA patterns that do not wrap focus, like combobox. + */ + @property({type: Boolean, attribute: 'no-navigation-wrap'}) + noNavigationWrap = false; + @queryAssignedElements({flatten: true}) protected slotItems!: HTMLElement[]; @state() private typeaheadActive = true; @@ -282,6 +290,7 @@ export abstract class Menu extends LitElement { return submenuNavKeys.has(key); }, + wrapNavigation: () => !this.noNavigationWrap, }); /** diff --git a/select/internal/select.ts b/select/internal/select.ts index d0e5b87fa..d0825c7f2 100644 --- a/select/internal/select.ts +++ b/select/internal/select.ts @@ -474,6 +474,7 @@ export abstract class Select extends selectBaseClass { ? `${this.selectWidth}px` : undefined, })} + no-navigation-wrap .open=${this.open} .quick=${this.quick} .positioning=${this.menuPositioning}