[Select panel] Loading states re-submission

.changeset/selfish-garlics-approve.md

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

packages/react/package.json

@@ -82,7 +82,7 @@
     "@github/tab-container-element": "^4.8.0",
     "@lit-labs/react": "1.2.1",
     "@oddbird/popover-polyfill": "^0.4.4",
-    "@primer/behaviors": "^1.7.2",
+    "@primer/behaviors": "^1.8.0",
     "@primer/live-region-element": "^0.7.1",
     "@primer/octicons-react": "^19.9.0",
     "@primer/primitives": "9.x || 10.x",

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 id="foobarbaz" 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 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,9 +29,10 @@ 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
   textInputProps?: Partial<Omit<TextInputProps, 'onChange'>>
   inputRef?: React.RefObject<HTMLInputElement>
 }
@@ -39,6 +44,7 @@ const StyledHeader = styled.div`
 
 export function FilteredActionList({
   loading = false,
+  loadingType = FilteredActionListLoadingTypes.bodySpinner,
   placeholderText,
   filterValue: externalFilterValue,
   onFilterChange,
@@ -59,7 +65,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()
@@ -78,9 +84,13 @@ export function FilteredActionList({
     [activeDescendantRef],
   )
 
+  const listContainerRefCallback = useCallback((node: HTMLDivElement | null) => {
+    setListContainerElement(node)
+  }, [])
+
   useFocusZone(
     {
-      containerRef: listContainerRef,
+      containerRef: {current: listContainerElement},
       focusOutBehavior: 'wrap',
       focusableElementFilter: element => {
         return !(element instanceof HTMLInputElement)
@@ -95,8 +105,9 @@ 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,
+      listContainerElement,
     ],
   )
 
@@ -110,7 +121,7 @@ export function FilteredActionList({
   useScrollFlash(scrollContainerRef)
 
   return (
-    <Box display="flex" flexDirection="column" overflow="hidden" sx={sx}>
+    <Box display="flex" flexDirection="column" overflow="hidden" flexGrow={1} sx={sx}>
       <StyledHeader>
         <TextInput
           ref={inputRef}
@@ -124,17 +135,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,6 +30,7 @@ export interface FilteredActionListProps
     ListPropsBase,
     SxProp {
   loading?: boolean
+  loadingType?: FilteredActionListLoadingType
   placeholderText?: string
   filterValue?: string
   onFilterChange: (value: string, e: React.ChangeEvent<HTMLInputElement>) => void
@@ -43,6 +45,7 @@ const StyledHeader = styled.div`
 
 export function FilteredActionList({
   loading = false,
+  loadingType = FilteredActionListLoadingTypes.bodySpinner,
   placeholderText,
   filterValue: externalFilterValue,
   onFilterChange,
@@ -65,7 +68,7 @@ export function FilteredActionList({
   )
 
   const scrollContainerRef = useRef<HTMLDivElement>(null)
-  const listContainerRef = useRef<HTMLUListElement>(null)
+  const [listContainerElement, setListContainerElement] = useState<HTMLUListElement | null>(null)
   const inputRef = useProvidedRefOrCreate<HTMLInputElement>(providedInputRef)
   const activeDescendantRef = useRef<HTMLElement>()
   const listId = useId()
@@ -84,9 +87,13 @@ export function FilteredActionList({
     [activeDescendantRef],
   )
 
+  const listContainerRefCallback = useCallback((node: HTMLUListElement | null) => {
+    setListContainerElement(node)
+  }, [])
+
   useFocusZone(
     {
-      containerRef: listContainerRef,
+      containerRef: {current: listContainerElement},
       bindKeys: FocusKeys.ArrowVertical | FocusKeys.PageUpDown,
       focusOutBehavior: 'wrap',
       focusableElementFilter: element => {
@@ -102,8 +109,9 @@ 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,
+      listContainerElement,
     ],
   )
 
@@ -115,7 +123,7 @@ export function FilteredActionList({
   }, [items])
 
   useScrollFlash(scrollContainerRef)
-  useAnnouncements(items, listContainerRef, inputRef)
+  useAnnouncements(items, {current: listContainerElement}, inputRef)
 
   function getItemListForEachGroup(groupId: string) {
     const itemsInGroup = []
@@ -146,17 +154,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 (
@@ -165,13 +180,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 = ('key' in item ? item.key : undefined) ?? 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 = ('key' in item ? item.key : undefined) ?? item.id?.toString() ?? index.toString()
+                  return <MappedActionListItem key={key} {...item} renderItem={listProps.renderItem} />
                 })}
           </ActionList>
         )}

packages/react/src/FilteredActionList/useAnnouncements.tsx

@@ -2,20 +2,12 @@
 // on https://github.com/github/multi-select-user-testing
 
 import {announce} from '@primer/live-region-element'
-import {useEffect, useRef} from 'react'
+import {useEffect, useState} from 'react'
 import type {FilteredActionListProps} from './FilteredActionListEntry'
 
 // we add a delay so that it does not interrupt default screen reader announcement and queues after it
 const delayMs = 500
 
-const useFirstRender = () => {
-  const firstRender = useRef(true)
-  useEffect(() => {
-    firstRender.current = false
-  }, [])
-  return firstRender.current
-}
-
 const getItemWithActiveDescendant = (
   listRef: React.RefObject<HTMLUListElement>,
   items: FilteredActionListProps['items'],
@@ -71,11 +63,17 @@ export const useAnnouncements = (
     },
     [listContainerRef, inputRef, items, liveRegion],
   )
+  const [inputEmptyFirstLoad, setInputEmptyFirstLoad] = useState(true)
 
-  const isFirstRender = useFirstRender()
   useEffect(
     function announceListUpdates() {
-      if (isFirstRender) return // ignore on first render as announceInitialFocus will also announce
+      const inputElement = inputRef.current
+
+      // Does the input element has no value for the first time? Then it is the initial state.
+      if (inputEmptyFirstLoad) {
+        setInputEmptyFirstLoad(!inputElement?.value)
+        return
+      }
 
       liveRegion?.clear() // clear previous announcements
 
@@ -103,6 +101,6 @@ export const useAnnouncements = (
         })
       })
     },
-    [isFirstRender, items, listContainerRef, liveRegion],
+    [items, listContainerRef, liveRegion, inputRef, inputEmptyFirstLoad],
   )
 }