Merge branch 'main' into dep-bump

Author: dgreif

.changeset/good-grapes-collect.md

@@ -0,0 +1,5 @@
+---
+"@primer/components": patch
+---
+
+Restore "fix: Don’t focus first 'Item' of 'DropdownMenu' and 'SelectMenu' on open" by deferring the removal of a temporary `tabIndex` until focus moves within the container.

src/AnchoredOverlay/AnchoredOverlay.tsx

@@ -1,6 +1,6 @@
 import React, {useCallback, useMemo} from 'react'
 import Overlay, {OverlayProps} from '../Overlay'
-import {useFocusTrap} from '../hooks/useFocusTrap'
+import {FocusTrapHookSettings, useFocusTrap} from '../hooks/useFocusTrap'
 import {FocusZoneHookSettings, useFocusZone} from '../hooks/useFocusZone'
 import {useAnchoredPosition, useProvidedRefOrCreate, useRenderForcingRef} from '../hooks'
 import {uniqueId} from '../utils/uniqueId'
@@ -53,6 +53,11 @@ interface AnchoredOverlayBaseProps extends Pick<OverlayProps, 'height' | 'width'
    */
   overlayProps?: Partial<OverlayProps>
 
+  /**
+   * Settings to apply to the Focus Zone on the internal `Overlay` component.
+   */
+  focusTrapSettings?: Partial<FocusTrapHookSettings>
+
   /**
    * Settings to apply to the Focus Zone on the internal `Overlay` component.
    */
@@ -76,6 +81,7 @@ export const AnchoredOverlay: React.FC<AnchoredOverlayProps> = ({
   height,
   width,
   overlayProps,
+  focusTrapSettings,
   focusZoneSettings
 }) => {
   const anchorRef = useProvidedRefOrCreate(externalAnchorRef)
@@ -121,7 +127,7 @@ export const AnchoredOverlay: React.FC<AnchoredOverlayProps> = ({
     disabled: !open || !position,
     ...focusZoneSettings
   })
-  useFocusTrap({containerRef: overlayRef, disabled: !open || !position})
+  useFocusTrap({containerRef: overlayRef, disabled: !open || !position, ...focusTrapSettings})
 
   return (
     <>

src/FilteredActionList/FilteredActionList.tsx

@@ -11,6 +11,7 @@ import {itemActiveDescendantClass} from '../ActionList/Item'
 import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
 import styled from 'styled-components'
 import {get} from '../constants'
+import {useProvidedRefOrCreate} from '../hooks/useProvidedRefOrCreate'
 import useScrollFlash from '../hooks/useScrollFlash'
 
 export interface FilteredActionListProps extends Partial<Omit<GroupedListProps, keyof ListPropsBase>>, ListPropsBase {
@@ -19,6 +20,7 @@ export interface FilteredActionListProps extends Partial<Omit<GroupedListProps,
   filterValue?: string
   onFilterChange: (value: string, e: React.ChangeEvent<HTMLInputElement>) => void
   textInputProps?: Partial<Omit<TextInputProps, 'onChange'>>
+  inputRef?: React.RefObject<HTMLInputElement>
 }
 
 function scrollIntoViewingArea(
@@ -56,6 +58,7 @@ export function FilteredActionList({
   onFilterChange,
   items,
   textInputProps,
+  inputRef: providedInputRef,
   ...listProps
 }: FilteredActionListProps): JSX.Element {
   const [filterValue, setInternalFilterValue] = useProvidedStateOrCreate(externalFilterValue, undefined, '')
@@ -70,7 +73,7 @@ export function FilteredActionList({
 
   const containerRef = useRef<HTMLInputElement>(null)
   const scrollContainerRef = useRef<HTMLDivElement>(null)
-  const inputRef = useRef<HTMLInputElement>(null)
+  const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)
   const activeDescendantRef = useRef<HTMLElement>()
   const listId = useMemo(uniqueId, [])
   const onInputKeyPress: KeyboardEventHandler = useCallback(

src/Overlay.tsx

@@ -1,5 +1,5 @@
 import styled from 'styled-components'
-import React, {ReactElement, useRef} from 'react'
+import React, {ReactElement, useEffect, useRef} from 'react'
 import {get, COMMON, POSITION, SystemPositionProps, SystemCommonProps} from './constants'
 import {ComponentProps} from './utils/types'
 import {useOverlay, TouchOrMouseEvent} from './hooks'
@@ -10,6 +10,7 @@ import {useCombinedRefs} from './hooks/useCombinedRefs'
 type StyledOverlayProps = {
   width?: keyof typeof widthMap
   height?: keyof typeof heightMap
+  maxHeight?: keyof Omit<typeof heightMap, 'auto' | 'initial'>
   visibility?: 'visible' | 'hidden'
 }
 
@@ -19,7 +20,8 @@ const heightMap = {
   medium: '320px',
   large: '432px',
   xlarge: '600px',
-  auto: 'auto'
+  auto: 'auto',
+  initial: 'auto' // Passing 'initial' initially applies 'auto'
 }
 
 const widthMap = {
@@ -39,6 +41,7 @@ const StyledOverlay = styled.div<StyledOverlayProps & SystemCommonProps & System
   min-width: 192px;
   max-width: 640px;
   height: ${props => heightMap[props.height || 'auto']};
+  max-height: ${props => props.maxHeight && heightMap[props.maxHeight]};
   width: ${props => widthMap[props.width || 'auto']};
   border-radius: 12px;
   overflow: hidden;
@@ -54,6 +57,9 @@ const StyledOverlay = styled.div<StyledOverlayProps & SystemCommonProps & System
     }
   }
   visibility: ${props => props.visibility || 'visible'};
+  :focus {
+    outline: none;
+  }
   ${COMMON};
   ${POSITION};
   ${sx};
@@ -78,31 +84,49 @@ export type OverlayProps = {
  * @param onClickOutside  Required. Function to call when clicking outside of the `Overlay`. Typically this function sets the `Overlay` visibility state to `false`.
  * @param onEscape Required. Function to call when user presses `Escape`. Typically this function sets the `Overlay` visibility state to `false`.
  * @param width Sets the width of the `Overlay`, pick from our set list of widths, or pass `auto` to automatically set the width based on the content of the `Overlay`. `small` corresponds to `256px`, `medium` corresponds to `320px`, `large` corresponds to `480px`, `xlarge` corresponds to `640px`, `xxlarge` corresponds to `960px`.
- * @param height Sets the height of the `Overlay`, pick from our set list of heights, or pass `auto` to automatically set the height based on the content of the `Overlay`. `xsmall` corresponds to `192px`, `small` corresponds to `256px`, `medium` corresponds to `320px`, `large` corresponds to `432px`, `xlarge` corresponds to `600px`.
+ * @param height Sets the height of the `Overlay`, pick from our set list of heights, or pass `auto` to automatically set the height based on the content of the `Overlay`, or pass `initial` to set the height based on the initial content of the `Overlay` (i.e. ignoring content changes). `xsmall` corresponds to `192px`, `small` corresponds to `256px`, `medium` corresponds to `320px`, `large` corresponds to `432px`, `xlarge` corresponds to `600px`.
+ * @param maxHeight Sets the maximum height of the `Overlay`, pick from our set list of heights. `xsmall` corresponds to `192px`, `small` corresponds to `256px`, `medium` corresponds to `320px`, `large` corresponds to `432px`, `xlarge` corresponds to `600px`.
  * @param visibility Sets the visibility of the `Overlay`
  */
 const Overlay = React.forwardRef<HTMLDivElement, OverlayProps>(
   (
-    {onClickOutside, role = 'dialog', initialFocusRef, returnFocusRef, ignoreClickRefs, onEscape, visibility, ...rest},
+    {
+      onClickOutside,
+      role = 'dialog',
+      initialFocusRef,
+      returnFocusRef,
+      ignoreClickRefs,
+      onEscape,
+      visibility,
+      height,
+      ...rest
+    },
     forwardedRef
   ): ReactElement => {
     const overlayRef = useRef<HTMLDivElement>(null)
     const combinedRef = useCombinedRefs(overlayRef, forwardedRef)
 
-    const overlayProps = useOverlay({
+    useOverlay({
       overlayRef,
       returnFocusRef,
       onEscape,
       ignoreClickRefs,
       onClickOutside,
       initialFocusRef
     })
+
+    useEffect(() => {
+      if (height === 'initial' && combinedRef.current?.clientHeight) {
+        combinedRef.current.style.height = `${combinedRef.current.clientHeight}px`
+      }
+    }, [height, combinedRef])
+
     return (
       <Portal>
         <StyledOverlay
-          {...overlayProps}
           aria-modal="true"
           role={role}
+          height={height}
           {...rest}
           ref={combinedRef}
           visibility={visibility}

src/SelectPanel/SelectPanel.tsx

@@ -127,13 +127,19 @@ export function SelectPanel({
     })
   }, [onClose, onSelectedChange, items, selected])
 
+  const inputRef = React.useRef<HTMLInputElement>(null)
+  const focusTrapSettings = {
+    initialFocusRef: inputRef
+  }
+
   return (
     <AnchoredOverlay
       renderAnchor={renderMenuAnchor}
       open={open}
       onOpen={onOpen}
       onClose={onClose}
       overlayProps={overlayProps}
+      focusTrapSettings={focusTrapSettings}
       focusZoneSettings={focusZoneSettings}
     >
       <Flex flexDirection="column" width="100%" height="100%">
@@ -145,6 +151,7 @@ export function SelectPanel({
           items={itemsToRender}
           selectionVariant={isMultiSelectVariant(selected) ? 'multiple' : 'single'}
           textInputProps={textInputProps}
+          inputRef={inputRef}
         />
       </Flex>
     </AnchoredOverlay>

src/__tests__/behaviors/focusTrap.tsx

@@ -22,7 +22,7 @@ beforeAll(() => {
   }
 })
 
-it('Should initially focus the first focusable element when activated', () => {
+it('Should initially focus the container when activated', () => {
   const {container} = render(
     <div>
       <button tabIndex={0}>Bad Apple</button>
@@ -35,9 +35,8 @@ it('Should initially focus the first focusable element when activated', () => {
   )
 
   const trapContainer = container.querySelector<HTMLElement>('#trapContainer')!
-  const firstButton = trapContainer.querySelector('button')!
   const controller = focusTrap(trapContainer)
-  expect(document.activeElement).toEqual(firstButton)
+  expect(document.activeElement).toEqual(trapContainer)
 
   controller.abort()
 })
@@ -74,13 +73,12 @@ it('Should prevent focus from exiting the trap, returns focus to previously-focu
   )
 
   const trapContainer = container.querySelector<HTMLElement>('#trapContainer')!
-  const firstButton = trapContainer.querySelector('button')!
   const secondButton = trapContainer.querySelectorAll('button')[1]!
   const durianButton = container.querySelector<HTMLElement>('#durian')!
   const controller = focusTrap(trapContainer)
 
   focus(durianButton)
-  expect(document.activeElement).toEqual(firstButton)
+  expect(document.activeElement).toEqual(trapContainer)
 
   focus(secondButton)
   expect(document.activeElement).toEqual(secondButton)
@@ -157,12 +155,11 @@ it('Should should release the trap when the signal is aborted', async () => {
 
   const trapContainer = container.querySelector<HTMLElement>('#trapContainer')!
   const durianButton = container.querySelector<HTMLElement>('#durian')!
-  const firstButton = trapContainer.querySelector('button')!
 
   const controller = focusTrap(trapContainer)
 
   focus(durianButton)
-  expect(document.activeElement).toEqual(firstButton)
+  expect(document.activeElement).toEqual(trapContainer)
 
   controller.abort()
 
@@ -189,7 +186,7 @@ it('Should should release the trap when the container is removed from the DOM',
   focusTrap(trapContainer)
 
   focus(durianButton)
-  expect(document.activeElement).toEqual(firstButton)
+  expect(document.activeElement).toEqual(trapContainer)
 
   // empty trap and remove it from the DOM
   trapContainer.removeChild(firstButton)

src/behaviors/focusTrap.ts

@@ -68,8 +68,7 @@ export function focusTrap(
   // Ensure focus remains in the trap zone by checking that a given recently-focused
   // element is inside the trap zone. If it isn't, redirect focus to a suitable
   // element within the trap zone. If need to redirect focus and a suitable element
-  // is not found, blur the recently-focused element so that focus doesn't leave the
-  // trap zone.
+  // is not found, focus the container.
   function ensureTrapZoneHasFocus(focusedElement: EventTarget | null) {
     if (focusedElement instanceof HTMLElement && document.contains(container)) {
       if (container.contains(focusedElement)) {
@@ -80,16 +79,29 @@ export function focusTrap(
         if (lastFocusedChild && isTabbable(lastFocusedChild) && container.contains(lastFocusedChild)) {
           lastFocusedChild.focus()
           return
+        } else if (initialFocus && container.contains(initialFocus)) {
+          initialFocus.focus()
+          return
         } else {
-          const toFocus = initialFocus && container.contains(initialFocus) ? initialFocus : getFocusableChild(container)
-          if (toFocus) {
-            toFocus.focus()
-            return
-          } else {
-            // no element focusable within trap, blur the external element instead
-            // eslint-disable-next-line github/no-blur
-            focusedElement.blur()
+          // Ensure the container is focusable:
+          // - Either the container already has a `tabIndex`
+          // - Or provide a temporary `tabIndex`
+          const containerNeedsTemporaryTabIndex = container.getAttribute('tabindex') === null
+          if (containerNeedsTemporaryTabIndex) {
+            container.setAttribute('tabindex', '-1')
+          }
+          // Focus the container.
+          container.focus()
+          // If a temporary `tabIndex` was provided, remove it.
+          if (containerNeedsTemporaryTabIndex) {
+            // Once focus has moved from the container to a child within the FocusTrap,
+            // the container can be made un-refocusable by removing `tabIndex`.
+            container.addEventListener('blur', () => container.removeAttribute('tabindex'), {once: true})
+            // NB: If `tabIndex` was removed *before* `blur`, then certain browsers (e.g. Chrome)
+            // would consider `body` the `activeElement`, and as a result, keyboard navigation
+            // between children would break, since `body` is outside the `FocusTrap`.
           }
+          return
         }
       }
     }

src/behaviors/focusZone.ts

@@ -588,7 +588,9 @@ export function focusZone(container: HTMLElement, settings?: FocusZoneSettings):
     }
 
     const focusedIndex = focusableElements.indexOf(currentFocusedElement)
-    return focusedIndex === -1 ? 0 : focusedIndex
+    const fallbackIndex = currentFocusedElement === container ? -1 : 0
+
+    return focusedIndex !== -1 ? focusedIndex : fallbackIndex
   }
 
   // "keydown" is the event that triggers DOM focus change, so that is what we use here

src/stories/SelectPanel.stories.tsx

@@ -106,3 +106,66 @@ export function SingleSelectStory(): JSX.Element {
   )
 }
 SingleSelectStory.storyName = 'Single Select'
+
+export function SelectPanelHeightInitialWithOverflowingItemsStory(): JSX.Element {
+  const [selected, setSelected] = React.useState<ItemInput | undefined>(items[0])
+  const [filter, setFilter] = React.useState('')
+  const filteredItems = items.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))
+  const [open, setOpen] = useState(false)
+
+  return (
+    <>
+      <h1>Single Select Panel</h1>
+      <div>Please select a label that describe your issue:</div>
+      <SelectPanel
+        renderAnchor={({children, 'aria-labelledby': ariaLabelledBy, ...anchorProps}) => (
+          <DropdownButton aria-labelledby={` ${ariaLabelledBy}`} {...anchorProps}>
+            {children ?? 'Select Labels'}
+          </DropdownButton>
+        )}
+        placeholderText="Filter Labels"
+        open={open}
+        onOpenChange={setOpen}
+        items={filteredItems}
+        selected={selected}
+        onSelectedChange={setSelected}
+        onFilterChange={setFilter}
+        showItemDividers={true}
+        overlayProps={{width: 'small', height: 'initial', maxHeight: 'xsmall'}}
+      />
+    </>
+  )
+}
+SelectPanelHeightInitialWithOverflowingItemsStory.storyName = 'SelectPanel, Height: Initial, Overflowing Items'
+
+export function SelectPanelHeightInitialWithUnderflowingItemsStory(): JSX.Element {
+  const underflowingItems = [items[0], items[1]]
+  const [selected, setSelected] = React.useState<ItemInput | undefined>(underflowingItems[0])
+  const [filter, setFilter] = React.useState('')
+  const filteredItems = underflowingItems.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))
+  const [open, setOpen] = useState(false)
+
+  return (
+    <>
+      <h1>Single Select Panel</h1>
+      <div>Please select a label that describe your issue:</div>
+      <SelectPanel
+        renderAnchor={({children, 'aria-labelledby': ariaLabelledBy, ...anchorProps}) => (
+          <DropdownButton aria-labelledby={` ${ariaLabelledBy}`} {...anchorProps}>
+            {children ?? 'Select Labels'}
+          </DropdownButton>
+        )}
+        placeholderText="Filter Labels"
+        open={open}
+        onOpenChange={setOpen}
+        items={filteredItems}
+        selected={selected}
+        onSelectedChange={setSelected}
+        onFilterChange={setFilter}
+        showItemDividers={true}
+        overlayProps={{width: 'small', height: 'initial', maxHeight: 'xsmall'}}
+      />
+    </>
+  )
+}
+SelectPanelHeightInitialWithUnderflowingItemsStory.storyName = 'SelectPanel, Height: Initial, Underflowing Items'