From a0f57593a1f20ac122921fbf617be1771740e482 Mon Sep 17 00:00:00 2001 From: HadrielWonda Date: Wed, 28 Oct 2026 20:26:25 +0100 Subject: [PATCH 1/2] FilteredActionList: only reference the list from the input while it is rendered --- .../filtered-action-list-aria-controls.md | 5 +++ .../FilteredActionList.test.tsx | 42 +++++++++++++++++++ .../FilteredActionList/FilteredActionList.tsx | 8 +++- .../FilteredActionListInput.tsx | 5 ++- 4 files changed, 56 insertions(+), 4 deletions(-) create mode 100644 .changeset/filtered-action-list-aria-controls.md 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..f124668b099 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() diff --git a/packages/react/src/FilteredActionList/FilteredActionList.tsx b/packages/react/src/FilteredActionList/FilteredActionList.tsx index f946fe8200a..5ac16a0415d 100644 --- a/packages/react/src/FilteredActionList/FilteredActionList.tsx +++ b/packages/react/src/FilteredActionList/FilteredActionList.tsx @@ -438,8 +438,12 @@ export function FilteredActionList({ [onSelectAllChange], ) + const showBodyLoader = loading && readScrollContainerRef.current !== null && loadingType.appearsInBody + // 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) { + if (showBodyLoader) { return ( ) @@ -586,7 +590,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} From 60a7e03822ec0a77b433d445d76245555469fb88 Mon Sep 17 00:00:00 2001 From: HadrielWonda Date: Thu, 29 Oct 2026 20:06:33 +0100 Subject: [PATCH 2/2] FilteredActionList: treat an unread scroll container ref as absent With primer_react_merged_forwarded_refs off, a callback scrollContainerRef leaves current undefined, so the null check let the body loader read clientHeight from undefined. Read the height with optional chaining and only show the loader when it was read, matching main. --- .../FilteredActionList.test.tsx | 22 +++++++++++++++++++ .../FilteredActionList/FilteredActionList.tsx | 10 +++++---- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/packages/react/src/FilteredActionList/FilteredActionList.test.tsx b/packages/react/src/FilteredActionList/FilteredActionList.test.tsx index f124668b099..ced58428160 100644 --- a/packages/react/src/FilteredActionList/FilteredActionList.test.tsx +++ b/packages/react/src/FilteredActionList/FilteredActionList.test.tsx @@ -205,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 5ac16a0415d..50788f5c9e1 100644 --- a/packages/react/src/FilteredActionList/FilteredActionList.tsx +++ b/packages/react/src/FilteredActionList/FilteredActionList.tsx @@ -438,15 +438,17 @@ export function FilteredActionList({ [onSelectAllChange], ) - const showBodyLoader = loading && readScrollContainerRef.current !== null && loadingType.appearsInBody + // `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 (showBodyLoader) { - return ( - - ) + return } if (message) { return message