diff --git a/.changeset/filtered-action-list-aria-controls.md b/.changeset/filtered-action-list-aria-controls.md new file mode 100644 index 00000000000..3461ccd3456 --- /dev/null +++ b/.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 diff --git a/packages/react/src/FilteredActionList/FilteredActionList.test.tsx b/packages/react/src/FilteredActionList/FilteredActionList.test.tsx index 7d46fc52cb6..ced58428160 100644 --- a/packages/react/src/FilteredActionList/FilteredActionList.test.tsx +++ b/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() + + 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( + No matching items} />, + ) + + 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() + // The body loader needs the mounted scroll container, so it appears on the next render + rerender() + + 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() @@ -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 = ( + + {}} + loading + loadingType={FilteredActionListLoadingTypes.bodySpinner} + /> + + ) + 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) + }) }) } }) diff --git a/packages/react/src/FilteredActionList/FilteredActionList.tsx b/packages/react/src/FilteredActionList/FilteredActionList.tsx index f946fe8200a..50788f5c9e1 100644 --- a/packages/react/src/FilteredActionList/FilteredActionList.tsx +++ b/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 ( - - ) + if (showBodyLoader) { + return } 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} diff --git a/packages/react/src/FilteredActionList/FilteredActionListInput.tsx b/packages/react/src/FilteredActionList/FilteredActionListInput.tsx index a6ca0cdfa5c..8c3ffe8db03 100644 --- a/packages/react/src/FilteredActionList/FilteredActionListInput.tsx +++ b/packages/react/src/FilteredActionList/FilteredActionListInput.tsx @@ -11,7 +11,8 @@ export interface FilteredActionListInputProps extends Partial onInputKeyDown?: React.KeyboardEventHandler 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}