🐛 fix: prevent accordion from stealing focus from content

e1en0r · e1en0r · commit d005520866c9 · 2024-11-04T17:27:53.000-08:00
diff --git a/package.json b/package.json
@@ -104,17 +104,14 @@
   "lint-staged": {
     "*.{md,mdx}": [
       "yarn prettier",
-      "yarn eslint",
-      "git add"
+      "yarn eslint"
     ],
     "*.{js,json,ts,tsx}": [
       "yarn prettier",
-      "yarn eslint",
-      "git add"
+      "yarn eslint"
     ],
     "*.css": [
-      "yarn stylelint",
-      "git add"
+      "yarn stylelint"
     ]
   },
   "publishConfig": {
diff --git a/src/components/InteractiveGroup/useInteractiveGroupItem.ts b/src/components/InteractiveGroup/useInteractiveGroupItem.ts
@@ -3,7 +3,7 @@ import { useScrollIntoView, UseScrollIntoViewProps } from './../../hooks/useScro
 
 export type UseInteractiveGroupItemProps<E extends HTMLElement> = Omit<UseScrollIntoViewProps<E>, 'behavior'> & {
   scrollBehavior?: UseScrollIntoViewProps<E>['behavior'];
-  moveBrowserFocus?: boolean;
+  moveBrowserFocus?: boolean | ((props: { ref: React.MutableRefObject<E | null>; focused?: boolean }) => boolean);
 };
 
 /**
@@ -20,8 +20,13 @@ export function useInteractiveGroupItem<E extends HTMLElement>({
   useScrollIntoView<E>({ behavior: scrollBehavior, ref, focused });
 
   useEffect(() => {
-    if (moveBrowserFocus && focused && document.activeElement !== ref.current) {
-      ref.current?.focus();
+    if (focused && document.activeElement !== ref.current) {
+      const shouldMoveBrowserFocus =
+        typeof moveBrowserFocus === 'function' ? moveBrowserFocus({ ref, focused }) : moveBrowserFocus;
+
+      if (shouldMoveBrowserFocus) {
+        ref.current?.focus();
+      }
     }
   }, [focused, moveBrowserFocus, ref]);
 }
diff --git a/src/compositions/Accordion/AccordionLabel.tsx b/src/compositions/Accordion/AccordionLabel.tsx
@@ -1,5 +1,5 @@
 import { cx } from '@emotion/css';
-import React, { useRef } from 'react';
+import React, { useCallback, useRef } from 'react';
 import { MergeElementPropsWithoutRef, Orientation } from '../../types';
 import { useInteractiveGroupItem } from '../../components/InteractiveGroup/useInteractiveGroupItem';
 import { useListRegistryItem } from '../../components/ListRegistry/useListRegistryItem';
@@ -9,6 +9,7 @@ import { AccordionItemStateProps } from './types';
 export type LocalAccordionLabelProps = AccordionItemStateProps & {
   children: React.ReactNode;
   className?: string;
+  contentRef?: React.RefObject<HTMLDivElement>;
   flush?: boolean;
   iconOnly?: boolean;
   id: string;
@@ -31,6 +32,7 @@ export type AccordionLabelProps = MergeElementPropsWithoutRef<'div', LocalAccord
 export function AccordionLabel({
   children,
   className,
+  contentRef,
   disabled = false,
   flush = false,
   focused = false,
@@ -42,7 +44,16 @@ export function AccordionLabel({
   ...props
 }: AccordionLabelProps): JSX.Element | null {
   const ref = useRef<HTMLDivElement>(null!);
-  useInteractiveGroupItem<HTMLDivElement>({ focused, ref, moveBrowserFocus: true, scrollBehavior: 'smooth' });
+
+  // don't move the browser focus if the focus is within the content
+  const moveBrowserFocus = useCallback(() => {
+    if (contentRef?.current?.contains(document.activeElement)) {
+      return false;
+    }
+    return true;
+  }, [contentRef]);
+
+  useInteractiveGroupItem<HTMLDivElement>({ focused, ref, moveBrowserFocus, scrollBehavior: 'smooth' });
   useListRegistryItem({ id, ref });
 
   // wrap the content in a class so the content opacity can change without affecting the pseudo elements
diff --git a/src/compositions/Accordion/AccordionList.tsx b/src/compositions/Accordion/AccordionList.tsx
@@ -54,7 +54,7 @@ export const AccordionList = React.forwardRef<HTMLDivElement, AccordionListProps
   ): React.ReactElement<AccordionListProps> => {
     const ref = useRef<HTMLDivElement>(null);
     const accessible = useAccessibility();
-    const { focused, handleBlur, handleFocus } = useDeepFocus<HTMLDivElement>(ref);
+    const { focused, handleBlur, handleFocus, setFocused } = useDeepFocus<HTMLDivElement>(ref);
     const themeId = useThemeId(initThemeId);
     const variant = contrast ? 'contrast' : initVariant;
 
@@ -113,6 +113,7 @@ export const AccordionList = React.forwardRef<HTMLDivElement, AccordionListProps
               key={id}
               orientation={orientation}
               parentRef={ref}
+              setDeepFocus={setFocused}
               unstyled={unstyled}
               {...item}
             />
diff --git a/src/compositions/Accordion/AccordionListItem.tsx b/src/compositions/Accordion/AccordionListItem.tsx
@@ -1,5 +1,5 @@
 import { cx } from '@emotion/css';
-import React, { useCallback, useContext } from 'react';
+import React, { useCallback, useContext, useRef } from 'react';
 import { useAccessibility } from '../../context/Accessibility';
 import { useComponentId } from '../../hooks/useComponentId';
 import {
@@ -21,6 +21,8 @@ export type AccordionListItemProps = React.HTMLAttributes<HTMLDivElement> &
     index: number;
     /** The parent ref is used to get its dimensions so the content can be rendered at the right size */
     parentRef: React.RefObject<HTMLDivElement>;
+    /** Manually triggers the focused state of the useDeepFocus hook */
+    setDeepFocus: React.Dispatch<React.SetStateAction<boolean>>;
     unstyled?: boolean;
   };
 
@@ -48,12 +50,14 @@ export const AccordionListItem = React.forwardRef<HTMLDivElement, AccordionListI
       labelProps,
       orientation,
       parentRef,
+      setDeepFocus,
       unstyled,
     },
     forwardedRef,
   ): React.ReactElement<AccordionListItemProps> => {
     const accessible = useAccessibility();
     const { generateComponentId } = useComponentId(componentId);
+    const contentRef = useRef<HTMLDivElement | null>(null);
 
     const { handleItemClick, isSelected, focusedIndex, setFocused } =
       useContext<InteractiveGroupContextValue<string, HTMLDivElement, HTMLDivElement>>(InteractiveGroupContext);
@@ -78,6 +82,30 @@ export const AccordionListItem = React.forwardRef<HTMLDivElement, AccordionListI
       [id, setFocused],
     );
 
+    const handleContentFocus = useCallback<React.FocusEventHandler<HTMLDivElement>>(
+      event => {
+        event.stopPropagation();
+
+        // set this item as focused
+        setFocused(id, { event });
+
+        /**
+         * Set the entire accordion as having a focused state, which is
+         * necessary because the event.stopPropagation() call here stops
+         * the natural setting of this state by the useDeepFocus hook.
+         * The reason we need to do all this manually is that if you
+         * click into the expanded content item of an accordion while the
+         * accordion itself doesn't have focus, if we don't stopPropagation
+         * then the accordion steals focus from the item that was clicked on.
+         * And if we do stop propagation without manually reconciling the
+         * focused state then the accordion will not appear to be focused
+         * when in fact it is.
+         */
+        setDeepFocus?.(true);
+      },
+      [id, setFocused, setDeepFocus],
+    );
+
     const itemFocused = focusedIndex === index;
     const itemSelected = isSelected(id);
     const stateProps = { disabled, focused: focused && itemFocused, selected: itemSelected };
@@ -98,6 +126,7 @@ export const AccordionListItem = React.forwardRef<HTMLDivElement, AccordionListI
           aria-controls={generateComponentId(id, 'panel')}
           aria-disabled={disabled}
           aria-expanded={!!stateProps.selected}
+          contentRef={contentRef}
           flush={flush}
           iconOnly={iconOnly}
           id={generateComponentId(id)}
@@ -117,8 +146,11 @@ export const AccordionListItem = React.forwardRef<HTMLDivElement, AccordionListI
           duration={duration}
           easing={easing}
           id={generateComponentId(id, 'panel')}
+          // if something within the content gets the focus this prevents the accordion from stealing it
+          onFocus={handleContentFocus}
           orientation={orientation}
           parentRef={parentRef}
+          ref={contentRef}
           role="tabpanel"
           {...contentProps}
           {...stateProps}
diff --git a/src/compositions/Accordion/stories/Accordion.docs.mdx b/src/compositions/Accordion/stories/Accordion.docs.mdx
@@ -166,7 +166,7 @@ import { Accordion, AccordionContainer, AccordionList } from '../index';
           <Rhythm pl={5} pr={3} py={3}>
             <Paper bordered full color={focused ? 'accent' : 'primary'}>
               <Flex full alignItems="center" direction="row" justifyContent="space-between">
-                <span>First</span>
+                <span>First{focused ? ' is focused' : ''}</span>
                 <IconButton
                   color={!focused && 'neutral'}
                   contrast={focused}
@@ -213,7 +213,7 @@ import { Accordion, AccordionContainer, AccordionList } from '../index';
           <Rhythm full pl={5} pr={3} py={3}>
             <Paper bordered full color={focused ? 'accent' : 'primary'}>
               <Flex full alignItems="center" direction="row" justifyContent="space-between">
-                <span>Second</span>
+                <span>Second{focused ? ' is focused' : ''}</span>
                 <IconButton
                   color={!focused && 'neutral'}
                   contrast={focused}
@@ -260,7 +260,7 @@ import { Accordion, AccordionContainer, AccordionList } from '../index';
           <Rhythm pl={5} pr={3} py={3}>
             <Paper bordered full color={focused ? 'accent' : 'primary'}>
               <Flex full alignItems="center" direction="row" justifyContent="space-between">
-                <span>Third</span>
+                <span>Third{focused ? ' is focused' : ''}</span>
                 <IconButton
                   color={!focused && 'neutral'}
                   contrast={focused}
@@ -307,7 +307,7 @@ import { Accordion, AccordionContainer, AccordionList } from '../index';
           <Rhythm pl={5} pr={3} py={3}>
             <Paper bordered full color={focused ? 'accent' : 'primary'}>
               <Flex full alignItems="center" direction="row" justifyContent="space-between">
-                <span>Fourth</span>
+                <span>Fourth{focused ? ' is focused' : ''}</span>
                 <IconButton
                   color={!focused && 'neutral'}
                   contrast={focused}
@@ -354,7 +354,7 @@ import { Accordion, AccordionContainer, AccordionList } from '../index';
                     ipsum rhoncus augue, ac luctus lacus lorem vel risus. Integer a ex interdum, egestas dui vitae,
                     maximus sem. Praesent id egestas.
                   </p>
-                  <input type="text" />
+                  <input style={{ height: 28 }} type="text" />
                 </Rhythm>
                 <IconButton
                   as="div"
@@ -375,6 +375,7 @@ import { Accordion, AccordionContainer, AccordionList } from '../index';
         labelProps: { style: { outline: 'none' } },
       },
     ]}
+    maxSelect={-1}
     minSelect={0}
   >
     {({ componentId, items, orientation, ref, themeId, style, unstyled }) => (
diff --git a/src/compositions/Dropdown/DropdownWithTags.tsx b/src/compositions/Dropdown/DropdownWithTags.tsx
@@ -8,14 +8,17 @@ import { Flex } from '../../components/Flex';
 import { IconText } from '../../components/IconText';
 import { generateInteractiveGroupActions } from '../../components/InteractiveGroup/generateInteractiveGroupActions';
 import { InteractiveGroupStateAction } from '../../components/InteractiveGroup/interactiveGroupActions';
-import { getInteractiveGroupInitialState, interactiveGroupReducer, InteractiveGroupState } from '../../components/InteractiveGroup/interactiveGroupReducer';
+import {
+  getInteractiveGroupInitialState,
+  interactiveGroupReducer,
+  InteractiveGroupState,
+} from '../../components/InteractiveGroup/interactiveGroupReducer';
 import { Rhythm } from '../../components/Rhythm/Rhythm';
 import { Tag, TagGroup, TagGroupProps, TagProps, TagShape, TagSize, TagWeight } from '../../components/Tag';
 import { TypographyWithSvg } from '../../components/Typography';
 import { PartialDropdown, PartialDropdownHandles, PartialDropdownProps } from './PartialDropdown';
 import { DropdownOption } from './types';
 
-
 export type DropdownWithTagsOption = DropdownOption & {
   tagProps?: TagProps<'button'>;
 };
diff --git a/src/hooks/useDeepFocus.ts b/src/hooks/useDeepFocus.ts
@@ -24,6 +24,8 @@ export type UseDeepFocusResponse<E> = {
   focused: boolean;
   handleBlur: React.FocusEventHandler<E>;
   handleFocus: React.FocusEventHandler<E>;
+  /** This should only be used if the state is in fact focused but a child stopped the focus propagation before it could be set */
+  setFocused: React.Dispatch<React.SetStateAction<boolean>>;
 };
 
 /**
@@ -128,6 +130,7 @@ export function useDeepFocus<E extends HTMLElement>(
     draftState.focused = focused;
     draftState.handleBlur = handleBlur;
     draftState.handleFocus = handleFocus;
+    draftState.setFocused = setFocused;
   });
   return previousResponse.current;
 }
diff --git a/src/utils/measureDomNode.ts b/src/utils/measureDomNode.ts
@@ -46,8 +46,8 @@ export const measureDomNode = (
   container.appendChild(content);
   appendTo.appendChild(container);
 
-  const height = container.clientHeight;
-  const width = container.clientWidth;
+  const height = Math.ceil(container.clientHeight);
+  const width = Math.ceil(container.clientWidth);
 
   container.parentNode?.removeChild(container);
   return { height, width };
