FilteredActionList: only reference the list from the input while it is rendered

.changeset/filtered-action-list-aria-controls.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+FilteredActionList: Only reference the list from the filter input while the list is rendered, so `aria-controls` never points at a missing element during body loading or when a message is shown

packages/react/src/FilteredActionList/FilteredActionList.test.tsx

@@ -25,6 +25,48 @@ describe('FilteredActionList', () => {
     expect(onFocus).toHaveBeenCalledOnce()
   })
 
+  describe('combobox list reference', () => {
+    it('references the rendered list when items are shown', () => {
+      const {getByRole} = render(<FilteredActionList items={items} onFilterChange={vi.fn()} />)
+
+      const combobox = getByRole('combobox')
+      const listId = combobox.getAttribute('aria-controls')
+
+      expect(combobox).toHaveAttribute('aria-expanded', 'true')
+      expect(listId).toBeTruthy()
+      expect(document.getElementById(listId!)).toBeInTheDocument()
+    })
+
+    it('does not reference a list while a message is shown', () => {
+      const {getByRole} = render(
+        <FilteredActionList items={[]} onFilterChange={vi.fn()} message={<div>No matching items</div>} />,
+      )
+
+      const combobox = getByRole('combobox')
+
+      expect(combobox).toHaveAttribute('aria-expanded', 'false')
+      expect(combobox).not.toHaveAttribute('aria-controls')
+    })
+
+    it('does not reference a list while the body shows a loader', () => {
+      const props = {
+        items,
+        onFilterChange: vi.fn(),
+        loading: true,
+        loadingType: FilteredActionListLoadingTypes.bodySpinner,
+      }
+      const {getByRole, container, rerender} = render(<FilteredActionList {...props} />)
+      // The body loader needs the mounted scroll container, so it appears on the next render
+      rerender(<FilteredActionList {...props} />)
+
+      const combobox = getByRole('combobox')
+
+      expect(container.querySelector('[data-component="FilteredActionList.Spinner"]')).toBeInTheDocument()
+      expect(combobox).toHaveAttribute('aria-expanded', 'false')
+      expect(combobox).not.toHaveAttribute('aria-controls')
+    })
+  })
+
   describe('data-component attributes', () => {
     it('renders FilteredActionList with data-component attribute', () => {
       const {container} = render(<FilteredActionList items={items} onFilterChange={vi.fn()} />)
@@ -163,6 +205,28 @@ describe('FilteredActionList forwarded refs (primer_react_merged_forwarded_refs)
         )
         expect(refCallback.mock.calls.some(([el]) => el instanceof HTMLDivElement)).toBe(true)
       })
+
+      it('handles a scroll container callback ref while loading', () => {
+        const ui = (
+          <FeatureFlags flags={{primer_react_merged_forwarded_refs: enabled}}>
+            <FilteredActionList
+              items={items}
+              onFilterChange={vi.fn()}
+              scrollContainerRef={() => {}}
+              loading
+              loadingType={FilteredActionListLoadingTypes.bodySpinner}
+            />
+          </FeatureFlags>
+        )
+        const {getByRole, container, rerender} = render(ui)
+
+        expect(() => rerender(ui)).not.toThrow()
+        // With the flag off, a callback ref leaves no readable scroll container, so the loader stays hidden as before
+        expect(container.querySelector('[data-component="FilteredActionList.Spinner"]') !== null).toBe(enabled)
+
+        const listId = getByRole('combobox').getAttribute('aria-controls')
+        expect(listId === null || document.getElementById(listId) !== null).toBe(true)
+      })
     })
   }
 })

packages/react/src/FilteredActionList/FilteredActionList.tsx

@@ -438,11 +438,17 @@ export function FilteredActionList({
     [onSelectAllChange],
   )
 
+  // `current` is undefined rather than null for a callback ref while the merged refs flag is off,
+  // so the body loader is only shown once the scroll container has been read
+  const bodyLoaderHeight =
+    loading && loadingType.appearsInBody ? readScrollContainerRef.current?.clientHeight : undefined
+  const showBodyLoader = bodyLoaderHeight !== undefined
+  // The list is not rendered while the body shows a loader or a message, so the input must not reference it
+  const isListRendered = !showBodyLoader && !message
+
   function getBodyContent() {
-    if (loading && readScrollContainerRef.current && loadingType.appearsInBody) {
-      return (
-        <FilteredActionListBodyLoader loadingType={loadingType} height={readScrollContainerRef.current.clientHeight} />
-      )
+    if (showBodyLoader) {
+      return <FilteredActionListBodyLoader loadingType={loadingType} height={bodyLoaderHeight} />
     }
     if (message) {
       return message
@@ -586,7 +592,7 @@ export function FilteredActionList({
         onInputKeyPress={onInputKeyPress}
         onInputKeyDown={usingRovingTabindex ? onInputKeyDown : undefined}
         placeholderText={placeholderText}
-        listId={listId}
+        listId={isListRendered ? listId : undefined}
         inputDescriptionTextId={inputDescriptionTextId}
         loading={loading && !loadingType.appearsInBody}
         fullScreenOnNarrow={fullScreenOnNarrow}

packages/react/src/FilteredActionList/FilteredActionListInput.tsx

@@ -11,7 +11,8 @@ export interface FilteredActionListInputProps extends Partial<Omit<TextInputProp
   onInputKeyPress?: React.KeyboardEventHandler<HTMLInputElement>
   onInputKeyDown?: React.KeyboardEventHandler<HTMLInputElement>
   placeholderText?: string
-  listId: string
+  /** ID of the rendered list. Omit it while the list is not rendered. */
+  listId?: string
   inputDescriptionTextId: string
   loading: boolean
   fullScreenOnNarrow?: boolean
@@ -51,7 +52,7 @@ export function FilteredActionListInput({
         onKeyDown={onInputKeyDown}
         placeholder={placeholderText}
         role="combobox"
-        aria-expanded="true"
+        aria-expanded={listId !== undefined}
         aria-autocomplete="list"
         aria-controls={listId}
         aria-label={placeholderText}