token editor focus control

Author: pan-kot

src/autosuggest/options-controller.ts

@@ -63,6 +63,7 @@ export const useAutosuggestItems = ({
   const enteredItemLabel = i18n('enteredTextLabel', enteredTextLabel?.(filterValue), format =>
     format({ value: filterValue })
   );
+
   if (!enteredItemLabel) {
     warnOnce('Autosuggest', 'A value for enteredTextLabel must be provided.');
   }

src/internal/components/focus-lock/utils.ts

@@ -25,6 +25,10 @@ const tabbables = [
   '[autofocus]',
 ].join(',');
 
+export function isFocusable(element: HTMLElement): boolean {
+  return element.matches(tabbables);
+}
+
 export function getAllFocusables(container: HTMLElement): HTMLElement[] {
   return Array.prototype.slice.call(container.querySelectorAll(tabbables));
 }

src/internal/components/token-list/index.tsx

@@ -4,7 +4,6 @@
 import React, { useState } from 'react';
 import clsx from 'clsx';
 
-import { useListFocusController } from '../../hooks/use-list-focus-controller';
 import { useUniqueId } from '../../hooks/use-unique-id';
 import { TokenListProps } from './interfaces';
 import TokenLimitToggle from './token-limit-toggle';
@@ -13,9 +12,6 @@ import styles from './styles.css.js';
 
 export { TokenListProps };
 
-const listItemSelector = `.${styles['list-item']}`;
-const showMoreSelector = `.${styles.toggle}`;
-
 export default function TokenList<Item>({
   items,
   alignment,
@@ -25,11 +21,8 @@ export default function TokenList<Item>({
   i18nStrings,
   limitShowFewerAriaLabel,
   limitShowMoreAriaLabel,
-  moveFocusNextToIndex: nextFocusIndex = null,
-  onFocusMoved,
   onExpandedClick = () => undefined,
 }: TokenListProps<Item>) {
-  const tokenListRef = useListFocusController({ nextFocusIndex, onFocusMoved, listItemSelector, showMoreSelector });
   const controlId = useUniqueId();
 
   const [expanded, setExpanded] = useState(false);
@@ -59,7 +52,7 @@ export default function TokenList<Item>({
 
   if (alignment === 'inline') {
     return (
-      <div ref={tokenListRef} className={clsx(styles.root, styles.horizontal)}>
+      <div className={clsx(styles.root, styles.horizontal)}>
         {hasItems && (
           <ul id={controlId} className={styles.list}>
             {visibleItems.map((item, itemIndex) => (
@@ -82,7 +75,7 @@ export default function TokenList<Item>({
   }
 
   return (
-    <div ref={tokenListRef} className={clsx(styles.root, styles.vertical)}>
+    <div className={clsx(styles.root, styles.vertical)}>
       {hasVisibleItems && (
         <ul id={controlId} className={clsx(styles.list, styles[alignment])}>
           {visibleItems.map((item, itemIndex) => (

src/internal/components/token-list/interfaces.ts

@@ -11,7 +11,6 @@ export interface TokenListProps<Item> {
   renderItem: (item: Item, itemIndex: number) => React.ReactNode;
   i18nStrings?: I18nStrings;
   moveFocusNextToIndex?: null | number;
-  onFocusMoved?: () => void;
   onExpandedClick?: (isExpanded: boolean) => void;
   limitShowFewerAriaLabel?: string;
   limitShowMoreAriaLabel?: string;

src/internal/hooks/use-list-focus-controller.tsx

@@ -3,7 +3,7 @@
 
 import { useEffect, useRef } from 'react';
 
-import { getFirstFocusable } from '../components/focus-lock/utils.js';
+import { getFirstFocusable, isFocusable } from '../components/focus-lock/utils.js';
 
 interface UseListFocusControllerOptions {
   nextFocusIndex: null | number;
@@ -28,8 +28,8 @@ export function useListFocusController({
     }
 
     const tokenElements = tokenListRef.current.querySelectorAll(listItemSelector);
-    const outsideElement = outsideSelector ? tokenListRef.current.closest(outsideSelector) : null;
-    const toggleButton = showMoreSelector ? tokenListRef.current.querySelector(showMoreSelector) : null;
+    const outsideElement = outsideSelector ? selectElement(tokenListRef.current, outsideSelector) : null;
+    const toggleButton = showMoreSelector ? selectElement(tokenListRef.current, showMoreSelector) : null;
 
     let closestPrevIndex = Number.NEGATIVE_INFINITY;
     let closestNextIndex = Number.POSITIVE_INFINITY;
@@ -46,12 +46,7 @@ export function useListFocusController({
 
     const nextElement = tokenElements[closestNextIndex];
     const prevElement = tokenElements[closestPrevIndex];
-    const focusTarget = getFirstEligible(
-      nextElement instanceof HTMLElement ? getFirstFocusable(nextElement) : null,
-      prevElement instanceof HTMLElement ? getFirstFocusable(prevElement) : null,
-      toggleButton,
-      outsideElement
-    );
+    const focusTarget = getFirstEligible(nextElement, prevElement, toggleButton, outsideElement);
 
     if (focusTarget) {
       focusTarget.focus();
@@ -67,9 +62,27 @@ export function useListFocusController({
 
 function getFirstEligible(...elements: Array<null | Element>): null | HTMLElement {
   for (const element of elements) {
-    if (element instanceof HTMLElement) {
-      return element;
+    const focusable = element ? getFocusableElement(element) : null;
+    if (focusable) {
+      return focusable;
     }
   }
   return null;
 }
+
+function getFocusableElement(element: Element): null | HTMLElement {
+  if (!(element instanceof HTMLElement)) {
+    return null;
+  }
+  if (isFocusable(element)) {
+    return element;
+  }
+  return getFirstFocusable(element);
+}
+
+function selectElement(container: HTMLElement, selector: string): null | Element {
+  if (container.matches(selector)) {
+    return container;
+  }
+  return container.querySelector(selector);
+}

src/property-filter/__tests__/property-filter-token-editor.test.tsx

@@ -2,15 +2,21 @@
 // SPDX-License-Identifier: Apache-2.0
 
 import React from 'react';
-import { act, fireEvent, render } from '@testing-library/react';
+import { act, fireEvent, render as reactRender } from '@testing-library/react';
 
 import TestI18nProvider from '../../../lib/components/i18n/testing';
 import { useMobile } from '../../../lib/components/internal/hooks/use-mobile';
-import { FilteringOption, FilteringProperty, Ref } from '../../../lib/components/property-filter/interfaces';
+import { FilteringOption, FilteringProperty } from '../../../lib/components/property-filter/interfaces';
 import PropertyFilterInternal, { PropertyFilterInternalProps } from '../../../lib/components/property-filter/internal';
 import createWrapper from '../../../lib/components/test-utils/dom';
 import { PropertyFilterWrapperInternal } from '../../../lib/components/test-utils/dom/property-filter';
-import { createDefaultProps, i18nStrings, i18nStringsTokenGroups, providedI18nStrings } from './common';
+import {
+  createDefaultProps,
+  i18nStrings,
+  i18nStringsTokenGroups,
+  providedI18nStrings,
+  StatefulInternalPropertyFilter,
+} from './common';
 
 jest.mock('../../../lib/components/internal/hooks/use-mobile', () => ({
   ...jest.requireActual('../../../lib/components/internal/hooks/use-mobile'),
@@ -65,25 +71,22 @@ const filteringOptions: readonly FilteringOption[] = [
   { propertyKey: 'default-operator', value: 'value' },
 ];
 
-const defaultProps = {
+const defaultProps: PropertyFilterInternalProps = {
   filteringOptions: [],
   customGroupsText: [],
   disableFreeTextFiltering: false,
   i18nStringsTokenGroups,
   ...createDefaultProps(filteringProperties, filteringOptions),
 };
 
-function renderComponent(
-  props?: Partial<PropertyFilterInternalProps & { ref: React.Ref<Ref> }>,
-  withI18nProvider = false
-) {
+function renderComponent(props?: Partial<PropertyFilterInternalProps>, withI18nProvider = false) {
   return withI18nProvider
-    ? render(
+    ? reactRender(
         <TestI18nProvider messages={providedI18nStrings}>
           <PropertyFilterInternal {...defaultProps} {...props} />
         </TestI18nProvider>
       )
-    : render(<PropertyFilterInternal {...defaultProps} {...props} />);
+    : reactRender(<PropertyFilterInternal {...defaultProps} {...props} />);
 }
 
 function openEditor(tokenIndex: number, options: { expandToViewport?: boolean; isMobile?: boolean }) {
@@ -438,6 +441,10 @@ describe('token editor with groups', () => {
     return renderComponent({ enableTokenGroups: true, ...props });
   }
 
+  function renderStateful(props?: Partial<PropertyFilterInternalProps>) {
+    return reactRender(<StatefulInternalPropertyFilter {...defaultProps} {...props} enableTokenGroups={true} />);
+  }
+
   test('changes property name', () => {
     const onChange = jest.fn();
     render({
@@ -735,6 +742,55 @@ describe('token editor with groups', () => {
     }
   );
 
+  test('moves focus to adjacent property when a filter is removed', () => {
+    renderStateful({
+      query: {
+        operation: 'and',
+        tokenGroups: [{ operation: 'or', tokens: [tokenJohn, tokenJane, tokenJack] }],
+        tokens: [],
+      },
+    });
+    const editor = openEditor(0, { expandToViewport: false });
+
+    // Removing 2nd filter (Jane)
+    editor.removeActions(2).actionsMenu.openDropdown();
+    editor.removeActions(2).removeAction().click();
+
+    // Focus is on new second filter (Jack)
+    expect(editor.propertySelect(2).find('button')!.getElement()).toHaveFocus();
+
+    // Removing 2nd filter (Jack)
+    editor.removeActions(2).actionsMenu.openDropdown();
+    editor.removeActions(2).removeFromGroupAction().click();
+
+    // Focus is on new first filter (John)
+    expect(editor.propertySelect(1).find('button')!.getElement()).toHaveFocus();
+  });
+
+  test('moves focus to the new filter when it is added', () => {
+    renderStateful({
+      query: {
+        operation: 'and',
+        tokenGroups: [{ operation: 'or', tokens: [tokenJohn] }, tokenJane],
+        tokens: [],
+      },
+    });
+    const editor = openEditor(0, { expandToViewport: false });
+
+    // Adding new filter
+    editor.addActions!.findMainAction()!.click();
+
+    // Focus is on newly added 2nd filter
+    expect(editor.propertySelect(2).find('button')!.getElement()).toHaveFocus();
+
+    // Adding standalone token (Jane)
+    editor.addActions!.openDropdown();
+    editor.addActions!.findItems()[0].click();
+
+    // Focus is on newly added 3rd filter
+    expect(editor.propertySelect(3).find('button')!.getElement()).toHaveFocus();
+  });
+
   test('standalone property menu items have indices as IDs', () => {
     const onChange = jest.fn();
     render({

src/property-filter/__tests__/property-filter-token-list.test.tsx

@@ -212,7 +212,7 @@ describe('filtering tokens', () => {
       expect(handleChange).not.toHaveBeenCalled();
     });
 
-    test('moves focus to the first token in the group when the last one is removed', () => {
+    test('moves focus to the adjacent grouped token and to the single remaining token', () => {
       const wrapper = renderStatefulInternalComponent({
         query: {
           operation: 'and',
@@ -233,11 +233,11 @@ describe('filtering tokens', () => {
 
       wrapper.findTokens()[0].findGroupTokens()[3].findRemoveButton()!.click();
       expect(wrapper.findTokens()[0].findGroupTokens()).toHaveLength(3);
-      expect(wrapper.findTokens()[0].findGroupTokens()[0].findRemoveButton()!.getElement()).toHaveFocus();
+      expect(wrapper.findTokens()[0].findGroupTokens()[2]!.find('button')!.getElement()).toHaveFocus();
 
-      wrapper.findTokens()[0].findGroupTokens()[2].findRemoveButton()!.click();
+      wrapper.findTokens()[0].findGroupTokens()[0].findRemoveButton()!.click();
       expect(wrapper.findTokens()[0].findGroupTokens()).toHaveLength(2);
-      expect(wrapper.findTokens()[0].findGroupTokens()[0].findRemoveButton()!.getElement()).toHaveFocus();
+      expect(wrapper.findTokens()[0].findGroupTokens()[0]!.find('button')!.getElement()).toHaveFocus();
 
       wrapper.findTokens()[0].findGroupTokens()[1].findRemoveButton()!.click();
       expect(wrapper.findTokens()[0].findGroupTokens()).toHaveLength(0);