fix: select a11y issues and more

momentum-design · Oct 11, 2023 · bfebb6f · bfebb6f
1 parent dac9870
commit bfebb6f
Show file tree

Hide file tree

Showing 7 changed files with 6,966 additions and 2,331 deletions.
diff --git a/src/components/Select/Select.constants.ts b/src/components/Select/Select.constants.ts
@@ -1,3 +1,4 @@
+import { FocusStrategy } from '@react-types/shared';
 import { SelectDirection } from '.';
 
 const CLASS_PREFIX = 'md-select';
@@ -9,7 +10,7 @@ const DIRECTIONS: Record<string, SelectDirection> = {
 
 const DEFAULTS = {
   DIRECTION: DIRECTIONS.bottom,
-  SHOULD_AUTOFOCUS: true,
+  FOCUS_STRATEGY: 'first' as FocusStrategy,
   SHOULD_SHOW_BORDER: true,
 };
 
@@ -20,8 +21,8 @@ const STYLE = {
   open: `${CLASS_PREFIX}-open`,
   iconWrapper: `${CLASS_PREFIX}-icon-wrapper`,
   selectedItemWrapper: `${CLASS_PREFIX}-selected-item-wrapper`,
-  overlay: `${CLASS_PREFIX}-overlay`,
   menuListBox: `${CLASS_PREFIX}-menu-listbox`,
+  popover: `${CLASS_PREFIX}-popover`,
   borderLess: 'borderLess',
 };
 

diff --git a/src/components/Select/Select.stories.tsx b/src/components/Select/Select.stories.tsx
@@ -41,6 +41,23 @@ const singleItems: SelectData[] = [
   { id: 3, value: 'Yellow' },
 ];
 
+const manyItems: SelectData[] = [
+  { id: 0, value: 'Red' },
+  { id: 1, value: 'Blue' },
+  { id: 2, value: 'Green' },
+  { id: 3, value: 'Yellow' },
+  { id: 4, value: 'Red' },
+  { id: 5, value: 'Blue' },
+  { id: 6, value: 'Green' },
+  { id: 7, value: 'Yellow' },
+  { id: 8, value: 'Red' },
+  { id: 9, value: 'Blue' },
+  { id: 10, value: 'Green' },
+  { id: 11, value: 'Yellow' },
+  { id: 12, value: 'Red' },
+  { id: 13, value: 'Blue' },
+];
+
 const Example = Template(Select).bind({});
 
 Example.args = {
@@ -314,6 +331,29 @@ Common.parameters = {
       showBorder: true,
       children: (item) => <Item key={item.id}>{item.value}</Item>,
     },
+    {
+      label: 'With a lot of list items',
+      items: manyItems,
+      direction: 'bottom',
+      showBorder: true,
+      children: (item) => <Item key={item.id}>{item.value}</Item>,
+    },
+    {
+      label: 'With a lot of list items and fixed height',
+      items: manyItems,
+      listboxMaxHeight: '13.5rem',
+      direction: 'bottom',
+      showBorder: true,
+      children: (item) => <Item key={item.id}>{item.value}</Item>,
+    },
+    {
+      label: 'Select with fixed width',
+      style: { width: '25rem', margin: '1rem' },
+      items: singleItems,
+      direction: 'bottom',
+      showBorder: true,
+      children: (item) => <Item key={item.id}>{item.value}</Item>,
+    },
   ],
 };
 

diff --git a/src/components/Select/Select.style.scss b/src/components/Select/Select.style.scss
@@ -11,6 +11,11 @@
     margin-left: 0.75rem;
     margin-bottom: 0.25rem;
   }
+
+  /* to make sure the popover matches the parent width, we have to override tippy css here: */
+  div[data-tippy-root=''] {
+    width: 100%;
+  }
 }
 
 .md-select-dropdown-input {
@@ -84,23 +89,24 @@
   }
 }
 
-.md-select-overlay {
+.md-select-menu-listbox {
+  border: none !important;
+  background: none !important;
+  padding: 0 !important;
   width: 100%;
-  position: absolute;
-  padding: 0;
-  margin: 0;
-  z-index: 9999;
-
-  &[data-direction='top'] {
-    // internal token set in React code
-    bottom: calc(var(--md-globals-select-dropdown-input-height) + 0.25rem);
-  }
 
-  &[data-direction='bottom'] {
-    margin-top: 0.25rem;
+  li {
+    width: calc(100% - 0.5rem);
+    margin: 0.25rem;
   }
 }
 
+.md-select-popover {
+  width: 100%;
+  padding: 0.25rem !important;
+  border-radius: 0.75rem !important;
+}
+
 .borderLess {
   border: none;
 }
diff --git a/src/components/Select/Select.tsx b/src/components/Select/Select.tsx
@@ -1,17 +1,26 @@
-import React, { ReactElement, RefObject, useRef, forwardRef, useState, useEffect } from 'react';
+import React, {
+  ReactElement,
+  useCallback,
+  RefObject,
+  useRef,
+  forwardRef,
+  useState,
+  useEffect,
+} from 'react';
 import classnames from 'classnames';
 
 import './Select.style.scss';
 import { Props } from './Select.types';
-import { DEFAULTS, SELECT_HEIGHT_ADJUST_BORDER, STYLE } from './Select.constants';
+import { DEFAULTS, STYLE } from './Select.constants';
 import { useSelectState } from '@react-stately/select';
 import { useButton } from '@react-aria/button';
-import { DismissButton, useOverlay } from '@react-aria/overlays';
-import { FocusScope } from '@react-aria/focus';
 import { useSelect, HiddenSelect } from '@react-aria/select';
 import Icon from '../Icon';
+import { useKeyboard } from '@react-aria/interactions';
 import ListBoxBase from '../ListBoxBase';
-import FocusRing from '../FocusRing';
+import Popover, { PopoverInstance } from '../Popover';
+import Text from '../Text';
+import { FocusScope } from 'react-aria';
 
 // eslint-disable-next-line @typescript-eslint/ban-types
 function Select<T extends object>(props: Props<T>, ref: RefObject<HTMLDivElement>): ReactElement {
@@ -26,111 +35,118 @@ function Select<T extends object>(props: Props<T>, ref: RefObject<HTMLDivElement
     direction = DEFAULTS.DIRECTION,
     title,
     showBorder = DEFAULTS.SHOULD_SHOW_BORDER,
+    listboxMaxHeight,
   } = props;
-  const state = useSelectState(props);
-
-  // used to calculate position of top direction dropdown
-  const [inputHeight, setInputHeight] = useState(0);
+  const [popoverInstance, setPopoverInstance] = useState<PopoverInstance>();
 
   const selectRef = useRef<HTMLButtonElement>(null);
-
   const boxRef = useRef<HTMLUListElement>(null);
 
+  const state = useSelectState(props);
   const { labelProps, triggerProps, valueProps, menuProps } = useSelect(props, state, selectRef);
-
   const { buttonProps } = useButton({ ...triggerProps, isDisabled }, selectRef);
   delete buttonProps.color;
 
-  const overlayRef = useRef<HTMLDivElement>(null);
-  const { overlayProps } = useOverlay(
-    {
-      onClose: () => state.close(),
-      shouldCloseOnBlur: true,
-      isOpen: state.isOpen,
-      isDismissable: true,
-    },
-    overlayRef
-  );
-
   const getArrowIcon = (isOpen: boolean) => (isOpen ? 'arrow-up' : 'arrow-down');
 
-  // used to calculate position of top direction dropdown
   useEffect(() => {
-    if (selectRef && selectRef.current && selectRef.current.clientHeight)
-      setInputHeight(selectRef.current.clientHeight + SELECT_HEIGHT_ADJUST_BORDER);
+    if (popoverInstance) {
+      if (state.isOpen) {
+        // show popover once state changes to isOpen = true
+        popoverInstance.show();
+      } else {
+        // hide popover once state changes to isOpen = false
+        popoverInstance.hide();
+        handleFocusBackOnTrigger();
+      }
+    }
+  }, [state.isOpen, popoverInstance]);
+
+  const handleFocusBackOnTrigger = useCallback(() => {
+    selectRef.current?.focus();
   }, []);
 
-  const listBox = (
-    <FocusScope restoreFocus>
-      {/*
-        //TODO:
-        This div should really be a Popover component but I will refrain from creating another
-        component as this PR is already big. I have created a workaround, so the Select can work meanwhile
-      */}
-      <div
-        {...overlayProps}
-        ref={overlayRef}
-        className={STYLE.overlay}
-        data-direction={direction}
-        style={{
-          // eslint-disable-next-line @typescript-eslint/no-explicit-any
-          ['--md-globals-select-dropdown-input-height' as any]: `${(inputHeight / 16).toFixed(
-            1
-          )}rem`,
-        }}
+  /**
+   * Handle closeOnSelect from @react-aria manually
+   */
+  const closePopover = () => {
+    state.close();
+  };
+
+  const triggerComponent = (
+    <button
+      id={name}
+      {...buttonProps}
+      className={classnames(
+        STYLE.dropdownInput,
+        { [STYLE.selected]: state.selectedItem },
+        { [STYLE.open]: state.isOpen },
+        { [STYLE.borderLess]: !showBorder }
+      )}
+      title={title}
+    >
+      <span
+        title={state.selectedItem?.textValue}
+        {...valueProps}
+        className={STYLE.selectedItemWrapper}
       >
-        {/* Invisible button for accessibility */}
-        <DismissButton onDismiss={() => state.close()} />
-        <ListBoxBase
-          {...menuProps}
-          ref={boxRef}
-          state={state}
-          disallowEmptySelection
-          // eslint-disable-next-line jsx-a11y/no-autofocus
-          autoFocus={state.focusStrategy || DEFAULTS.SHOULD_AUTOFOCUS}
-          className={STYLE.menuListBox}
-        />
-        {/* Invisible button for accessibility */}
-        <DismissButton onDismiss={() => state.close()} />
-      </div>
-    </FocusScope>
+        {state.selectedItem ? state.selectedItem.rendered : placeholder}
+      </span>
+      <span aria-hidden="true" className={STYLE.iconWrapper}>
+        <Icon name={getArrowIcon(state.isOpen)} weight="bold" scale={16} />
+      </span>
+    </button>
   );
 
+  const { keyboardProps } = useKeyboard({
+    onKeyDown: (event) => {
+      if (event.key === 'Escape') {
+        closePopover();
+      }
+    },
+  });
+
+  // delete color prop which is passed down and used in the ModalContainer
+  // because it conflicts with the HTML color property
+  delete keyboardProps.color;
+
   return (
     <div className={classnames(className, STYLE.wrapper)} ref={ref} style={style} id={id}>
       {label && (
         <label htmlFor={name} {...labelProps}>
-          {/* //TODO: change with <Text /> when available */}
-          {label}
+          <Text>{label}</Text>
         </label>
       )}
       <HiddenSelect state={state} triggerRef={selectRef} label={label} name={name} />
-      <FocusRing>
-        <button
-          id={name}
-          {...buttonProps}
-          className={classnames(
-            STYLE.dropdownInput,
-            { [STYLE.selected]: state.selectedItem },
-            { [STYLE.open]: state.isOpen },
-            { [STYLE.borderLess]: !showBorder }
-          )}
-          ref={selectRef}
-          title={title}
-        >
-          <span
-            title={state.selectedItem?.textValue}
-            {...valueProps}
-            className={STYLE.selectedItemWrapper}
-          >
-            {state.selectedItem ? state.selectedItem.rendered : placeholder}
-          </span>
-          <span aria-hidden="true" className={STYLE.iconWrapper}>
-            <Icon name={getArrowIcon(state.isOpen)} weight="bold" scale={16} />
-          </span>
-        </button>
-      </FocusRing>
-      {state.isOpen && !isDisabled && listBox}
+
+      <Popover
+        interactive={true}
+        showArrow={false}
+        triggerComponent={React.cloneElement(triggerComponent, {
+          ref: selectRef,
+        })}
+        trigger="manual"
+        setInstance={setPopoverInstance}
+        placement={direction}
+        onClickOutside={closePopover}
+        addBackdrop
+        hideOnEsc={false}
+        {...(keyboardProps as Omit<React.HTMLAttributes<HTMLElement>, 'color'>)}
+        style={{ maxHeight: listboxMaxHeight || 'none' }}
+        className={STYLE.popover}
+      >
+        <FocusScope contain>
+          <ListBoxBase
+            {...menuProps}
+            ref={boxRef}
+            state={state}
+            disallowEmptySelection
+            // eslint-disable-next-line jsx-a11y/no-autofocus
+            autoFocus={state.focusStrategy || DEFAULTS.FOCUS_STRATEGY}
+            className={STYLE.menuListBox}
+          />
+        </FocusScope>
+      </Popover>
     </div>
   );
 }

diff --git a/src/components/Select/Select.types.ts b/src/components/Select/Select.types.ts
@@ -34,8 +34,14 @@ export interface Props<T> extends AriaSelectProps<T> {
    * title to use for this component.
    */
   title?: string;
+
   /**
    * showBorder for the component
    */
   showBorder?: boolean;
+
+  /**
+   * To override the list box max height
+   */
+  listboxMaxHeight?: string;
 }