[Select panel] Loading states re-submission (#5139)

* Revert "Revert "Select panel loading states (#4929)" (#5136)"

This reverts commit f93eb9b.

* Use published primer/behaviors

* Revert snapshot change to TextInput

* Huh ok never mind

* Use role in test

* Prevent active descendant from resetting when items change

* Allow loading state to be managed by the user

* Encounter the loading state for the initial state announcement

* It's working

* Cleanup

* Fix linting issues

* Revert snapshots

* test(vrt): update snapshots

* Backport #5551 to this PR

* Fix auto-focus behavior

* Add test to ensure focus behavior

* Allow consumers to pass an inputRef; DRY up timeout cleanup; simplify nullish coercion

* DRY up announcement code; restore useAnnouncements hook for non-SelectPanel cases

* Fix types tests

* Remove testing ID

* Remove confusing componentManagesLoading control in async story

---------

Co-authored-by: Cameron Dutro <[email protected]>
Co-authored-by: francinelucca <[email protected]>
Co-authored-by: Marie Lucca <[email protected]>

.changeset/selfish-garlics-approve.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': minor
+---
+
+[SelectPanel] Implement loading states

packages/react/src/FilteredActionList/FilteredActionListLoaders.tsx

@@ -0,0 +1,66 @@
+import React from 'react'
+import Box from '../Box'
+import Spinner from '../Spinner'
+import {Stack} from '../Stack/Stack'
+import {SkeletonBox} from '../experimental/Skeleton/SkeletonBox'
+
+export class FilteredActionListLoadingType {
+  public name: string
+  public appearsInBody: boolean
+
+  constructor(name: string, appearsInBody: boolean) {
+    this.name = name
+    this.appearsInBody = appearsInBody
+  }
+}
+
+export const FilteredActionListLoadingTypes = {
+  bodySpinner: new FilteredActionListLoadingType('body-spinner', true),
+  bodySkeleton: new FilteredActionListLoadingType('body-skeleton', true),
+  input: new FilteredActionListLoadingType('input', false),
+}
+
+const SKELETON_ROW_HEIGHT = 24
+const SKELETON_MIN_ROWS = 3
+
+export function FilteredActionListBodyLoader({
+  loadingType,
+  height,
+}: {
+  loadingType: FilteredActionListLoadingType
+  height: number
+}): JSX.Element {
+  switch (loadingType) {
+    case FilteredActionListLoadingTypes.bodySpinner:
+      return <LoadingSpinner data-testid="filtered-action-list-spinner" />
+    case FilteredActionListLoadingTypes.bodySkeleton: {
+      const rows = height < SKELETON_ROW_HEIGHT ? SKELETON_MIN_ROWS : height / SKELETON_ROW_HEIGHT
+      return <LoadingSkeleton data-testid="filtered-action-list-skeleton" rows={rows} />
+    }
+    default:
+      return <></>
+  }
+}
+
+function LoadingSpinner({...props}): JSX.Element {
+  return (
+    <Box p={3} flexGrow={1} sx={{alignContent: 'center', textAlign: 'center', height: '100%'}}>
+      <Spinner {...props} />
+    </Box>
+  )
+}
+
+function LoadingSkeleton({rows = 10, ...props}: {rows: number}): JSX.Element {
+  return (
+    <Box p={2} display="flex" flexGrow={1} flexDirection="column">
+      <Stack direction="vertical" justify="center" gap="condensed" {...props}>
+        {Array.from({length: rows}, (_, i) => (
+          <Stack key={i} direction="horizontal" gap="condensed" align="center">
+            <SkeletonBox width="16px" height="16px" />
+            <SkeletonBox height="10px" width={`${Math.random() * 60 + 20}%`} sx={{borderRadius: '4px'}} />
+          </Stack>
+        ))}
+      </Stack>
+    </Box>
+  )
+}

packages/react/src/FilteredActionList/FilteredActionListWithDeprecatedActionList.tsx

@@ -1,10 +1,9 @@
 import type {ScrollIntoViewOptions} from '@primer/behaviors'
 import {scrollIntoView} from '@primer/behaviors'
-import type {KeyboardEventHandler} from 'react'
-import React, {useCallback, useEffect, useRef} from 'react'
+import type {KeyboardEventHandler, RefObject} from 'react'
+import React, {useCallback, useEffect, useRef, useState} from 'react'
 import styled from 'styled-components'
 import Box from '../Box'
-import Spinner from '../Spinner'
 import type {TextInputProps} from '../TextInput'
 import TextInput from '../TextInput'
 import {get} from '../constants'
@@ -17,6 +16,11 @@ import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
 import useScrollFlash from '../hooks/useScrollFlash'
 import {VisuallyHidden} from '../VisuallyHidden'
 import type {SxProp} from '../sx'
+import {
+  type FilteredActionListLoadingType,
+  FilteredActionListBodyLoader,
+  FilteredActionListLoadingTypes,
+} from './FilteredActionListLoaders'
 
 const menuScrollMargins: ScrollIntoViewOptions = {startMargin: 0, endMargin: 8}
 
@@ -25,12 +29,16 @@ export interface FilteredActionListProps
     ListPropsBase,
     SxProp {
   loading?: boolean
+  loadingType?: FilteredActionListLoadingType
   placeholderText?: string
   filterValue?: string
-  onFilterChange: (value: string, e: React.ChangeEvent<HTMLInputElement>) => void
+  onFilterChange: (value: string, e: React.ChangeEvent<HTMLInputElement> | null) => void
+  onListContainerRefChanged?: (ref: HTMLElement | null) => void
+  onInputRefChanged?: (ref: React.RefObject<HTMLInputElement>) => void
   textInputProps?: Partial<Omit<TextInputProps, 'onChange'>>
   inputRef?: React.RefObject<HTMLInputElement>
   className?: string
+  announcementsEnabled?: boolean
 }
 
 const StyledHeader = styled.div`
@@ -40,14 +48,18 @@ const StyledHeader = styled.div`
 
 export function FilteredActionList({
   loading = false,
+  loadingType = FilteredActionListLoadingTypes.bodySpinner,
   placeholderText,
   filterValue: externalFilterValue,
   onFilterChange,
+  onListContainerRefChanged,
+  onInputRefChanged,
   items,
   textInputProps,
   inputRef: providedInputRef,
   sx,
   className,
+  announcementsEnabled: _announcementsEnabled = true,
   ...listProps
 }: FilteredActionListProps): JSX.Element {
   const [filterValue, setInternalFilterValue] = useProvidedStateOrCreate(externalFilterValue, undefined, '')
@@ -61,7 +73,7 @@ export function FilteredActionList({
   )
 
   const scrollContainerRef = useRef<HTMLDivElement>(null)
-  const listContainerRef = useRef<HTMLDivElement>(null)
+  const [listContainerElement, setListContainerElement] = useState<HTMLDivElement | null>(null)
   const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)
   const activeDescendantRef = useRef<HTMLElement>()
   const listId = useId()
@@ -80,9 +92,21 @@ export function FilteredActionList({
     [activeDescendantRef],
   )
 
+  const listContainerRefCallback = useCallback(
+    (node: HTMLDivElement | null) => {
+      setListContainerElement(node)
+      onListContainerRefChanged?.(node)
+    },
+    [onListContainerRefChanged],
+  )
+
+  useEffect(() => {
+    onInputRefChanged?.(inputRef)
+  }, [inputRef, onInputRefChanged])
+
   useFocusZone(
     {
-      containerRef: listContainerRef,
+      containerRef: {current: listContainerElement},
       focusOutBehavior: 'wrap',
       focusableElementFilter: element => {
         return !(element instanceof HTMLInputElement)
@@ -97,15 +121,18 @@ export function FilteredActionList({
       },
     },
     [
-      // List ref isn't set while loading.  Need to re-bind focus zone when it changes
-      loading,
+      // List container isn't in the DOM while loading.  Need to re-bind focus zone when it changes.
+      listContainerElement,
     ],
   )
 
   useEffect(() => {
     // if items changed, we want to instantly move active descendant into view
     if (activeDescendantRef.current && scrollContainerRef.current) {
-      scrollIntoView(activeDescendantRef.current, scrollContainerRef.current, {...menuScrollMargins, behavior: 'auto'})
+      scrollIntoView(activeDescendantRef.current, scrollContainerRef.current, {
+        ...menuScrollMargins,
+        behavior: 'auto',
+      })
     }
   }, [items])
 
@@ -116,6 +143,7 @@ export function FilteredActionList({
       display="flex"
       flexDirection="column"
       overflow="hidden"
+      flexGrow={1}
       sx={sx}
       className={className}
       data-testid="filtered-action-list"
@@ -133,17 +161,17 @@ export function FilteredActionList({
           aria-label={placeholderText}
           aria-controls={listId}
           aria-describedby={inputDescriptionTextId}
+          loaderPosition={'leading'}
+          loading={loading && !loadingType.appearsInBody}
           {...textInputProps}
         />
       </StyledHeader>
       <VisuallyHidden id={inputDescriptionTextId}>Items will be filtered as you type</VisuallyHidden>
-      <Box ref={scrollContainerRef} overflow="auto">
-        {loading ? (
-          <Box width="100%" display="flex" flexDirection="row" justifyContent="center" pt={6} pb={7}>
-            <Spinner />
-          </Box>
+      <Box ref={scrollContainerRef} overflow="auto" flexGrow={1}>
+        {loading && scrollContainerRef.current && loadingType.appearsInBody ? (
+          <FilteredActionListBodyLoader loadingType={loadingType} height={scrollContainerRef.current.clientHeight} />
         ) : (
-          <ActionList ref={listContainerRef} items={items} {...listProps} role="listbox" id={listId} />
+          <ActionList ref={listContainerRefCallback} items={items} {...listProps} role="listbox" id={listId} />
         )}
       </Box>
     </Box>

packages/react/src/FilteredActionList/FilteredActionListWithModernActionList.tsx

@@ -1,10 +1,9 @@
 import type {ScrollIntoViewOptions} from '@primer/behaviors'
 import {scrollIntoView, FocusKeys} from '@primer/behaviors'
 import type {KeyboardEventHandler} from 'react'
-import React, {useCallback, useEffect, useRef} from 'react'
+import React, {useCallback, useEffect, useRef, useState} from 'react'
 import styled from 'styled-components'
 import Box from '../Box'
-import Spinner from '../Spinner'
 import type {TextInputProps} from '../TextInput'
 import TextInput from '../TextInput'
 import {get} from '../constants'
@@ -17,6 +16,8 @@ import {useProvidedStateOrCreate} from '../hooks/useProvidedStateOrCreate'
 import useScrollFlash from '../hooks/useScrollFlash'
 import {VisuallyHidden} from '../VisuallyHidden'
 import type {SxProp} from '../sx'
+import type {FilteredActionListLoadingType} from './FilteredActionListLoaders'
+import {FilteredActionListLoadingTypes, FilteredActionListBodyLoader} from './FilteredActionListLoaders'
 
 import {isValidElementType} from 'react-is'
 import type {RenderItemFn} from '../deprecated/ActionList/List'
@@ -29,12 +30,16 @@ export interface FilteredActionListProps
     ListPropsBase,
     SxProp {
   loading?: boolean
+  loadingType?: FilteredActionListLoadingType
   placeholderText?: string
   filterValue?: string
   onFilterChange: (value: string, e: React.ChangeEvent<HTMLInputElement>) => void
+  onListContainerRefChanged?: (ref: HTMLElement | null) => void
+  onInputRefChanged?: (ref: React.RefObject<HTMLInputElement>) => void
   textInputProps?: Partial<Omit<TextInputProps, 'onChange'>>
   inputRef?: React.RefObject<HTMLInputElement>
   className?: string
+  announcementsEnabled?: boolean
 }
 
 const StyledHeader = styled.div`
@@ -46,14 +51,18 @@ export function FilteredActionList({
   loading = false,
   placeholderText,
   filterValue: externalFilterValue,
+  loadingType = FilteredActionListLoadingTypes.bodySpinner,
   onFilterChange,
+  onListContainerRefChanged,
+  onInputRefChanged,
   items,
   textInputProps,
   inputRef: providedInputRef,
   sx,
   groupMetadata,
   showItemDividers,
   className,
+  announcementsEnabled = true,
   ...listProps
 }: FilteredActionListProps): JSX.Element {
   const [filterValue, setInternalFilterValue] = useProvidedStateOrCreate(externalFilterValue, undefined, '')
@@ -67,8 +76,8 @@ export function FilteredActionList({
   )
 
   const scrollContainerRef = useRef<HTMLDivElement>(null)
-  const listContainerRef = useRef<HTMLUListElement>(null)
   const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)
+  const [listContainerElement, setListContainerElement] = useState<HTMLUListElement | null>(null)
   const activeDescendantRef = useRef<HTMLElement>()
   const listId = useId()
   const inputDescriptionTextId = useId()
@@ -86,9 +95,21 @@ export function FilteredActionList({
     [activeDescendantRef],
   )
 
+  const listContainerRefCallback = useCallback(
+    (node: HTMLUListElement | null) => {
+      setListContainerElement(node)
+      onListContainerRefChanged?.(node)
+    },
+    [onListContainerRefChanged],
+  )
+
+  useEffect(() => {
+    onInputRefChanged?.(inputRef)
+  }, [inputRef, onInputRefChanged])
+
   useFocusZone(
     {
-      containerRef: listContainerRef,
+      containerRef: {current: listContainerElement},
       bindKeys: FocusKeys.ArrowVertical | FocusKeys.PageUpDown,
       focusOutBehavior: 'wrap',
       focusableElementFilter: element => {
@@ -104,20 +125,23 @@ export function FilteredActionList({
       },
     },
     [
-      // List ref isn't set while loading.  Need to re-bind focus zone when it changes
-      loading,
+      // List container isn't in the DOM while loading.  Need to re-bind focus zone when it changes.
+      listContainerElement,
     ],
   )
 
   useEffect(() => {
     // if items changed, we want to instantly move active descendant into view
     if (activeDescendantRef.current && scrollContainerRef.current) {
-      scrollIntoView(activeDescendantRef.current, scrollContainerRef.current, {...menuScrollMargins, behavior: 'auto'})
+      scrollIntoView(activeDescendantRef.current, scrollContainerRef.current, {
+        ...menuScrollMargins,
+        behavior: 'auto',
+      })
     }
   }, [items])
 
+  useAnnouncements(items, {current: listContainerElement}, inputRef, announcementsEnabled)
   useScrollFlash(scrollContainerRef)
-  useAnnouncements(items, listContainerRef, inputRef)
 
   function getItemListForEachGroup(groupId: string) {
     const itemsInGroup = []
@@ -155,17 +179,24 @@ export function FilteredActionList({
           aria-controls={listId}
           aria-label={placeholderText}
           aria-describedby={inputDescriptionTextId}
+          loaderPosition={'leading'}
+          loading={loading && !loadingType.appearsInBody}
           {...textInputProps}
         />
       </StyledHeader>
       <VisuallyHidden id={inputDescriptionTextId}>Items will be filtered as you type</VisuallyHidden>
-      <Box ref={scrollContainerRef} overflow="auto">
-        {loading ? (
-          <Box width="100%" display="flex" flexDirection="row" justifyContent="center" pt={6} pb={7}>
-            <Spinner />
-          </Box>
+      <Box ref={scrollContainerRef} overflow="auto" display="flex" flexGrow={1}>
+        {loading && scrollContainerRef.current && loadingType.appearsInBody ? (
+          <FilteredActionListBodyLoader loadingType={loadingType} height={scrollContainerRef.current.clientHeight} />
         ) : (
-          <ActionList ref={listContainerRef} showDividers={showItemDividers} {...listProps} role="listbox" id={listId}>
+          <ActionList
+            ref={listContainerRefCallback}
+            showDividers={showItemDividers}
+            {...listProps}
+            role="listbox"
+            id={listId}
+            sx={{flexGrow: 1}}
+          >
             {groupMetadata?.length
               ? groupMetadata.map((group, index) => {
                   return (
@@ -174,13 +205,15 @@ export function FilteredActionList({
                         {group.header?.title ? group.header.title : `Group ${group.groupId}`}
                       </ActionList.GroupHeading>
                       {getItemListForEachGroup(group.groupId).map((item, index) => {
-                        return <MappedActionListItem key={index} {...item} renderItem={listProps.renderItem} />
+                        const key = item.key ?? item.id?.toString() ?? index.toString()
+                        return <MappedActionListItem key={key} {...item} renderItem={listProps.renderItem} />
                       })}
                     </ActionList.Group>
                   )
                 })
               : items.map((item, index) => {
-                  return <MappedActionListItem key={index} {...item} renderItem={listProps.renderItem} />
+                  const key = item.key ?? item.id?.toString() ?? index.toString()
+                  return <MappedActionListItem key={key} {...item} renderItem={listProps.renderItem} />
                 })}
           </ActionList>
         )}