fix(multi-select): Correct selected pill accessibility

cypress/component/MultiSelect.spec.tsx

@@ -0,0 +1,124 @@
+import {Searching} from '../../modules/preview-react/multi-select/stories/examples/Searching';
+
+describe('MultiSelect', () => {
+  context('given the Searching story is rendered', () => {
+    beforeEach(() => {
+      cy.mount(<Searching />);
+    });
+
+    // Cypress adds its own main landmark; scan the controls without the story's nested main.
+    it('should not have any axe errors before interaction', () => {
+      cy.checkA11y({include: ['form > main > *']});
+    });
+
+    context('when options have been selected', () => {
+      beforeEach(() => {
+        cy.findByRole('combobox', {name: 'Fruits'}).focus().realPress('ArrowDown');
+        cy.findByRole('option', {name: 'Red Apple 1'}).should('be.visible');
+        cy.findByRole('combobox', {name: 'Fruits'}).realType('apple');
+        cy.findByRole('option', {name: 'Red Apple 1'}).click();
+        cy.findByRole('option', {name: 'Blue Apple 7'}).click();
+        cy.findByRole('combobox', {name: 'Fruits'}).focus().realPress('Escape');
+      });
+
+      it('should expose removal buttons in a named toolbar without axe errors', () => {
+        cy.findByRole('toolbar', {name: 'Fruits'}).within(() => {
+          cy.findAllByRole('button').should('have.length', 2);
+          cy.findByRole('button', {name: 'Remove Red Apple 1'}).should($button => {
+            expect($button).not.to.have.attr('aria-selected');
+            expect($button).to.have.attr('type', 'button');
+          });
+          cy.findByRole('button', {name: 'Remove Blue Apple 7'}).should('be.visible');
+        });
+        cy.checkA11y({include: ['form > main > *']});
+      });
+
+      it('should navigate and remove pills with the keyboard, then return focus to the input', () => {
+        cy.findByRole('button', {name: 'Remove Red Apple 1'}).focus().realPress('ArrowRight');
+        cy.findByRole('button', {name: 'Remove Blue Apple 7'})
+          .should('be.focused')
+          .realPress('Backspace');
+        cy.findByRole('button', {name: 'Remove Blue Apple 7'}).should('not.exist');
+        cy.findByRole('button', {name: 'Remove Red Apple 1'})
+          .should('be.focused')
+          .realPress('Delete');
+        cy.findByRole('toolbar', {name: 'Fruits'}).should('not.exist');
+        cy.findByRole('combobox', {name: 'Fruits'}).should('be.focused');
+      });
+
+      it('should use one tab stop and navigate the toolbar with Home, End, and arrow keys', () => {
+        cy.findByRole('combobox', {name: 'Fruits'}).focus().realPress('Tab');
+        cy.findByRole('button', {name: 'Remove Red Apple 1'}).should('be.focused');
+        cy.findByRole('toolbar', {name: 'Fruits'})
+          .find('button[tabindex="0"]')
+          .should('have.length', 1);
+        cy.realPress('End');
+        cy.findByRole('button', {name: 'Remove Blue Apple 7'}).should('be.focused');
+        cy.realPress('ArrowRight');
+        cy.findByRole('button', {name: 'Remove Red Apple 1'}).should('be.focused');
+        cy.realPress('ArrowLeft');
+        cy.findByRole('button', {name: 'Remove Blue Apple 7'}).should('be.focused');
+        cy.realPress('ArrowLeft');
+        cy.findByRole('button', {name: 'Remove Red Apple 1'}).should('be.focused');
+        cy.realPress('ArrowRight');
+        cy.findByRole('button', {name: 'Remove Blue Apple 7'}).should('be.focused');
+        cy.realPress('Home');
+        cy.findByRole('button', {name: 'Remove Red Apple 1'}).should('be.focused');
+        cy.realPress('ArrowUp');
+        cy.realPress('ArrowDown');
+        cy.findByRole('button', {name: 'Remove Red Apple 1'}).should('be.focused');
+        cy.realPress(['Shift', 'Tab']);
+        cy.findByRole('combobox', {name: 'Fruits'}).should('be.focused');
+        cy.checkA11y({include: ['form > main > *']});
+      });
+
+      it('should remove a pill without submitting the form', () => {
+        const onSubmit = cy.stub().as('submit');
+        cy.get('form').then($form => $form[0].addEventListener('submit', onSubmit));
+        cy.findByRole('button', {name: 'Remove Red Apple 1'}).click();
+        cy.findByRole('button', {name: 'Remove Red Apple 1'}).should('not.exist');
+        cy.get('@submit').should('not.have.been.called');
+      });
+
+      it('should activate removal buttons with Enter and Space without submitting the form', () => {
+        const onSubmit = cy.stub().as('submit');
+        cy.get('form').then($form => $form[0].addEventListener('submit', onSubmit));
+        cy.findByRole('button', {name: 'Remove Red Apple 1'}).focus().realPress('Enter');
+        cy.findByRole('button', {name: 'Remove Red Apple 1'}).should('not.exist');
+        cy.findByRole('button', {name: 'Remove Blue Apple 7'})
+          .should('be.focused')
+          .realPress('Space');
+        cy.findByRole('toolbar', {name: 'Fruits'}).should('not.exist');
+        cy.findByRole('combobox', {name: 'Fruits'}).should('be.focused');
+        cy.get('@submit').should('not.have.been.called');
+        cy.checkA11y({include: ['form > main > *']});
+      });
+    });
+
+    context('when search text is entered using the keyboard', () => {
+      beforeEach(() => {
+        cy.findByRole('combobox', {name: 'Fruits'}).focus().realPress('ArrowDown');
+        cy.findByRole('option', {name: 'Red Apple 1'}).should('be.visible');
+        cy.findByRole('combobox', {name: 'Fruits'}).realType('apple');
+      });
+
+      it('should not have any axe errors', () => {
+        cy.findByRole('option', {name: 'Pink Apple 25'}).should('be.visible');
+        cy.checkA11y({include: ['form > main > *', '[role="listbox"]']});
+      });
+
+      it('should open the listbox with the filtered result', () => {
+        cy.findByRole('combobox', {name: 'Fruits'}).should('have.attr', 'aria-expanded', 'true');
+        cy.findByRole('option', {name: 'Pink Apple 25'}).should('be.visible');
+      });
+
+      it('should clear the search without moving focus or exposing a redundant button', () => {
+        cy.get('[data-part="input-group-clear-button"]')
+          .should('have.attr', 'aria-hidden', 'true')
+          .and('have.attr', 'tabindex', '-1')
+          .click();
+        cy.findByRole('combobox', {name: 'Fruits'}).should('have.value', '').and('be.focused');
+      });
+    });
+  });
+});

modules/preview-react/multi-select/lib/MultiSelectInput.tsx

@@ -187,7 +187,7 @@ export interface MultiSelectInputProps
   extends CSProps,
     Pick<
       React.InputHTMLAttributes<HTMLInputElement>,
-      'disabled' | 'className' | 'style' | 'aria-labelledby'
+      'disabled' | 'className' | 'style' | 'aria-label' | 'aria-labelledby'
     >,
     Pick<MultiSelectedItemProps, 'removeLabel'> {
   error?: ErrorType;
@@ -203,6 +203,7 @@ export const MultiSelectInput = createSubcomponent(TextInput)({
       cs,
       style,
       error,
+      'aria-label': ariaLabel,
       'aria-labelledby': ariaLabelledBy,
       removeLabel,
       disabled,
@@ -218,6 +219,7 @@ export const MultiSelectInput = createSubcomponent(TextInput)({
           <InputGroup.Input
             data-part="user-input"
             as={Element}
+            aria-label={ariaLabel}
             aria-labelledby={ariaLabelledBy}
             readOnly
             disabled={disabled}
@@ -228,7 +230,12 @@ export const MultiSelectInput = createSubcomponent(TextInput)({
             <SystemIcon icon={caretDownSmallIcon} size="md" />
           </InputGroup.InnerEnd>
         </InputGroup>
-        <MultiSelectedList disabled={disabled} removeLabel={removeLabel} />
+        <MultiSelectedList
+          disabled={disabled}
+          removeLabel={removeLabel}
+          aria-label={ariaLabel}
+          aria-labelledby={ariaLabelledBy}
+        />
       </div>
     );
   }
@@ -243,6 +250,7 @@ export const MultiSelectSearchInput = createSubcomponent(TextInput)({
       className,
       cs,
       style,
+      'aria-label': ariaLabel,
       'aria-labelledby': ariaLabelledBy,
       removeLabel,
       formInputProps,
@@ -251,7 +259,8 @@ export const MultiSelectSearchInput = createSubcomponent(TextInput)({
       error,
       ...elemProps
     },
-    Element
+    Element,
+    model
   ) => {
     return (
       <div {...handleCsProp({className, cs, style}, multiSelectInputStencil({}))}>
@@ -267,9 +276,11 @@ export const MultiSelectSearchInput = createSubcomponent(TextInput)({
           <InputGroup.Input
             data-part="user-input"
             as={Element}
+            aria-label={ariaLabel}
             aria-labelledby={ariaLabelledBy}
             disabled={disabled}
             error={error}
+            ref={model.state.searchInputRef}
             {...elemProps}
           />
           <InputGroup.InnerEnd width={system.legacy.size.xxxs}>
@@ -279,7 +290,12 @@ export const MultiSelectSearchInput = createSubcomponent(TextInput)({
             <SystemIcon icon={caretDownSmallIcon} size="md" />
           </InputGroup.InnerEnd>
         </InputGroup>
-        <MultiSelectedList removeLabel={removeLabel} disabled={disabled} />
+        <MultiSelectedList
+          removeLabel={removeLabel}
+          disabled={disabled}
+          aria-label={ariaLabel}
+          aria-labelledby={ariaLabelledBy}
+        />
       </div>
     );
   }

modules/preview-react/multi-select/lib/MultiSelectedItem.tsx

@@ -1,7 +1,6 @@
 import {useListItemRegister, useListItemRovingFocus} from '@workday/canvas-kit-react/collection';
 import {
   composeHooks,
-  createElemPropsHook,
   createSubModelElemPropsHook,
   createSubcomponent,
 } from '@workday/canvas-kit-react/common';
@@ -25,11 +24,6 @@ export interface MultiSelectedItemProps {
 }
 
 export const useMultiSelectedItem = composeHooks(
-  createElemPropsHook(useMultiSelectModel)(model => {
-    return {
-      'aria-selected': true,
-    };
-  }),
   useMultiSelectItemRemove,
   createSubModelElemPropsHook(useMultiSelectModel)(m => m.selected, useListItemRovingFocus),
   createSubModelElemPropsHook(useMultiSelectModel)(m => m.selected, useListItemRegister)
@@ -42,7 +36,7 @@ export const MultiSelectedItem = createSubcomponent('span')({
   return (
     <Pill as={Element} disabled={disabled} variant="removable">
       <Pill.Label>{children}</Pill.Label>
-      <Pill.IconButton aria-label={removeLabel} ref={ref} {...(elemProps as any)} />
+      <Pill.IconButton type="button" aria-label={removeLabel} ref={ref} {...(elemProps as any)} />
     </Pill>
   );
 });

modules/preview-react/multi-select/lib/MultiSelectedList.tsx

@@ -19,15 +19,19 @@ export interface MultiSelectedListProps
 export const MultiSelectedList = createSubcomponent('div')({
   modelHook: useMultiSelectModel,
 })<MultiSelectedListProps>(
-  ({'aria-labelledby': ariaLabelledBy, disabled, removeLabel}, Element, model) => {
+  (
+    {'aria-label': ariaLabel, 'aria-labelledby': ariaLabelledBy, disabled, removeLabel},
+    Element,
+    model
+  ) => {
     return model.selected.state.items.length ? (
       <>
         <div data-part="separator" />
         <ListBox
           model={model.selected}
           as={Element}
-          role="listbox"
-          aria-orientation="horizontal"
+          role="toolbar"
+          aria-label={ariaLabel}
           aria-labelledby={ariaLabelledBy}
         >
           {item => (

modules/preview-react/multi-select/lib/useMultiSelectItemRemove.ts

@@ -23,15 +23,19 @@ import {useMultiSelectModel} from './useMultiSelectModel';
  */
 export const useMultiSelectItemRemove = createElemPropsHook(useMultiSelectModel)((model, _ref) => {
   return {
+    onFocus(event: React.FocusEvent<HTMLElement>) {
+      model.selected.events.goTo({id: event.currentTarget.dataset.id || ''});
+    },
     onKeyDown(event: React.KeyboardEvent<HTMLElement>) {
       if (event.key === 'Backspace' || event.key === 'Delete') {
+        event.preventDefault();
         const id = event.currentTarget.dataset.id || '';
         const nextId = listItemRemove(id, model.selected);
         model.selected.events.remove({id, event});
         if (nextId) {
           focusOnCurrentCursor(model.selected, nextId, event.currentTarget);
         } else {
-          model.state.inputRef.current?.focus();
+          (model.state.searchInputRef.current || model.state.inputRef.current)?.focus();
         }
       }
     },
@@ -42,7 +46,7 @@ export const useMultiSelectItemRemove = createElemPropsHook(useMultiSelectModel)
       if (nextId) {
         focusOnCurrentCursor(model.selected, nextId, event.currentTarget);
       } else {
-        model.state.inputRef.current?.focus();
+        (model.state.searchInputRef.current || model.state.inputRef.current)?.focus();
       }
     },
   };

modules/preview-react/multi-select/lib/useMultiSelectModel.ts

@@ -30,6 +30,8 @@ export const useMultiSelectModel = createModelHook({
   contextOverride: useComboboxModel.Context,
 })(config => {
   const cachedSelectedRef = React.useRef<Item<any>[]>([]);
+  // Search text is independent of the constrained form value, but still needs a focus target.
+  const searchInputRef = React.useRef<HTMLInputElement>(null);
   const model = useComboboxModel(
     useComboboxModel.mergeConfig(config, {
       onHide() {
@@ -61,15 +63,16 @@ export const useMultiSelectModel = createModelHook({
   );
   cachedSelectedRef.current = cachedSelected;
 
-  // The `listbox` of pills under the MultiSelect combobox input.
+  const visibleSelectedItems = model.state.visibility === 'hidden' ? cachedSelected : selectedItems;
+
   const selected = useListModel({
     orientation: 'horizontal',
     onRemove({id}) {
       model.events.select({id});
       setSelectedItems(cachedSelected.filter(item => item.id !== id));
     },
     shouldVirtualize: false,
-    items: model.state.visibility === 'hidden' ? cachedSelected : selectedItems,
+    items: visibleSelectedItems,
   });
 
   return {
@@ -87,5 +90,6 @@ export const useMultiSelectModel = createModelHook({
       },
     },
     ...model,
+    state: {...model.state, searchInputRef},
   };
 });

modules/preview-react/multi-select/spec/MultiSelectInput.spec.tsx

@@ -0,0 +1,30 @@
+import {render, screen, within} from '@testing-library/react';
+
+import {MultiSelect} from '../lib/MultiSelect';
+
+describe.each([
+  ['Input', MultiSelect.Input],
+  ['SearchInput', MultiSelect.SearchInput],
+] as const)('MultiSelect.%s selected items', (_, Input) => {
+  it.each(['aria-label', 'aria-labelledby'] as const)(
+    'should use %s to label the removal toolbar',
+    labelProp => {
+      render(
+        <MultiSelect items={['Apple']} initialSelectedIds={['Apple']}>
+          <span id="fruits-label">Fruits</span>
+          <Input
+            {...{[labelProp]: labelProp === 'aria-label' ? 'Fruits' : 'fruits-label'}}
+            removeLabel="Remove"
+          />
+        </MultiSelect>
+      );
+
+      expect(screen.getByRole('combobox', {name: 'Fruits'})).toBeInTheDocument();
+      const toolbar = screen.getByRole('toolbar', {name: 'Fruits'});
+      const button = within(toolbar).getByRole('button', {name: 'Remove Apple'});
+      expect(button).not.toHaveAttribute('aria-selected');
+      expect(button).toHaveAttribute('type', 'button');
+      expect(screen.queryByRole('grid')).not.toBeInTheDocument();
+    }
+  );
+});

modules/react/text-input/lib/InputGroup.tsx

@@ -158,7 +158,7 @@ export const useClearButton = createElemPropsHook(useInputGroupModel)(model => {
   return {
     // This element does not need to be accessible via screen reader. The user can already clear
     // an input
-    role: 'presentation',
+    'aria-hidden': true,
     // A clear input button doesn't need focus. There's already keyboard keys to clear an input
     tabIndex: -1,
     icon: xSmallIcon as CanvasSystemIcon,