From d455dc9b32c6b530cdb87d6e6b4a72c7ccfe5378 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Thu, 21 May 2026 12:26:00 +0200 Subject: [PATCH 1/9] chore: add first-open sizing investigation artifacts --- .../SelectPanel.examples.stories.tsx | 99 +++++++++++ .../src/SelectPanel/SelectPanel.test.tsx | 156 ++++++++++++++++++ 2 files changed, 255 insertions(+) diff --git a/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx b/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx index 5d9058bcefd..44e1ab93917 100644 --- a/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx @@ -843,3 +843,102 @@ export const VirtualizedBuiltIn = () => { ) } + +/** + * Demonstrates the first-open sizing regression where the overlay + * appears too small and shows a scrollbar unnecessarily on first open. + * After closing and reopening, it usually renders correctly. + * + * Issue: https://github.com/primer/react/issues/1831 + */ +export const FirstOpenSizingDebug = () => { + const [selected, setSelected] = useState([]) + const [open, setOpen] = useState(false) + const [loading, setLoading] = useState(true) + const [filteredItems, setFilteredItems] = useState([]) + const [filter, setFilter] = useState('') + const overlayRef = useRef(null) + const [measurements, setMeasurements] = useState<{ + clientHeight?: number + scrollHeight?: number + scrollWidth?: number + hasScrollbar?: boolean + timestamp?: string + }>({}) + + // Simulate loading delay similar to the real issue + React.useEffect(() => { + if (!open) { + setLoading(true) + } + const timer = window.setTimeout(() => { + if (open) { + setFilteredItems(items.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))) + setLoading(false) + } + }, 1500) + return () => window.clearTimeout(timer) + }, [open, filter]) + + // Measure overlay dimensions every 100ms to catch sizing changes + React.useEffect(() => { + if (!open) return + + const interval = window.setInterval(() => { + const overlay = document.querySelector('[data-testid="overlay"]') as HTMLElement | null + if (overlay) { + const hasScrollbar = overlay.scrollHeight > overlay.clientHeight + setMeasurements({ + clientHeight: overlay.clientHeight, + scrollHeight: overlay.scrollHeight, + scrollWidth: overlay.scrollWidth, + hasScrollbar, + timestamp: new Date().toLocaleTimeString(), + }) + + console.log( + `[SelectPanel] Overlay measurements: clientHeight=${overlay.clientHeight}, scrollHeight=${overlay.scrollHeight}, hasScrollbar=${hasScrollbar}`, + ) + } + }, 100) + + return () => window.clearInterval(interval) + }, [open]) + + return ( + + First-Open Sizing Debug +
+

+ Open={open} | Loading={loading} | Last measurement: {measurements.timestamp} +

+

+ Overlay size: {measurements.clientHeight}px (client) × {measurements.scrollHeight}px (scroll) +

+

+ Scrollbar: {measurements.hasScrollbar ? '✓ PRESENT (BUG!)' : '✗ Not present'} +

+
+ + + ) +} diff --git a/packages/react/src/SelectPanel/SelectPanel.test.tsx b/packages/react/src/SelectPanel/SelectPanel.test.tsx index c1d71b154c7..d43b751faff 100644 --- a/packages/react/src/SelectPanel/SelectPanel.test.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.test.tsx @@ -2291,3 +2291,159 @@ describe('SelectPanel displayInViewport prop', () => { expect(lastCall[2]?.displayInViewport).not.toBe(true) }) }) + +/** + * Test suite for SelectPanel first-open sizing regression + * Issue: https://github.com/primer/react/issues/1831 + * + * On first open with loading state, SelectPanel may render shorter than its content + * and show an unnecessary scrollbar. This occurs because position is calculated before + * items have loaded and rendered, using the spinner/loading state height rather than + * the final content height. + */ +describe('SelectPanel - First-Open Sizing with Loading State', () => { + const items: ItemInput[] = [ + {id: '1', text: 'Item 1'}, + {id: '2', text: 'Item 2'}, + {id: '3', text: 'Item 3'}, + {id: '4', text: 'Item 4'}, + {id: '5', text: 'Item 5'}, + {id: '6', text: 'Item 6'}, + {id: '7', text: 'Item 7'}, + {id: '8', text: 'Item 8'}, + ] + + const TestComponentWithLoadingDelay = () => { + const [selected, setSelected] = React.useState(items[0]) + const [open, setOpen] = React.useState(false) + const [loading, setLoading] = React.useState(true) + const [visibleItems, setVisibleItems] = React.useState([]) + + // Simulate items loading after a delay (typical async data fetch) + React.useEffect(() => { + if (!open) { + setLoading(true) + setVisibleItems([]) + } + + const timer = setTimeout(() => { + if (open) { + setVisibleItems(items) + setLoading(false) + } + }, 500) // 500ms delay simulates network/async operation + + return () => clearTimeout(timer) + }, [open]) + + return ( + <> + +
+ open:{open} loading:{loading} itemsCount:{visibleItems.length} +
+ + ) + } + + it('should measure overlay height correctly while loading', async () => { + const user = userEvent.setup() + render() + + const button = screen.getByRole('button', {name: /Select item/i}) + + // Open SelectPanel for the first time + await user.click(button) + + // Wait for overlay to appear (should show loading spinner) + await waitFor( + () => { + expect(screen.getByText(/^Item 1$/)).toBeInTheDocument() + }, + {timeout: 1000}, + ) + + // Get overlay measurements while still loading + const overlay = document.querySelector('[data-testid="overlay"]') as HTMLElement | null + const clientHeightWhileLoading = overlay?.clientHeight + const scrollHeightWhileLoading = overlay?.scrollHeight + + // Wait for items to load + await waitFor( + () => { + const state = screen.getByTestId('state') + expect(state).toHaveTextContent('loading:false') + }, + {timeout: 1500}, + ) + + // Get overlay measurements after loading + const clientHeightAfterLoading = overlay?.clientHeight + const scrollHeightAfterLoading = overlay?.scrollHeight + + // Log measurements for debugging + console.log('First-open sizing measurements:', { + whileLoading: {clientHeight: clientHeightWhileLoading, scrollHeight: scrollHeightWhileLoading}, + afterLoading: {clientHeight: clientHeightAfterLoading, scrollHeight: scrollHeightAfterLoading}, + }) + + // The overlay should have enough height to fit all content without scrollbar + if (clientHeightAfterLoading && scrollHeightAfterLoading) { + const hasScrollbar = scrollHeightAfterLoading > clientHeightAfterLoading + expect(hasScrollbar).toBe(false) + } + }) + + it('should have consistent height between first and second open', async () => { + const user = userEvent.setup() + const {unmount, rerender} = render() + + const button = screen.getByRole('button', {name: /Select item/i}) + + // First open + await user.click(button) + await waitFor(() => { + const state = screen.getByTestId('state') + expect(state).toHaveTextContent('loading:false') + }) + + const overlay1 = document.querySelector('[data-testid="overlay"]') + const height1 = overlay1?.getBoundingClientRect().height + + // Close + await user.click(button) + await waitFor(() => { + const state = screen.getByTestId('state') + expect(state).toHaveTextContent('open:false') + }) + + // Second open (loading state happens again) + await user.click(button) + await waitFor( + () => { + const state = screen.getByTestId('state') + expect(state).toHaveTextContent('loading:false') + }, + {timeout: 1500}, + ) + + const overlay2 = document.querySelector('[data-testid="overlay"]') + const height2 = overlay2?.getBoundingClientRect().height + + // Heights should be very similar (allowing for minor variations) + if (height1 && height2) { + const difference = Math.abs(height1 - height2) + console.log(`Height consistency check: first=${height1}px, second=${height2}px, diff=${difference}px`) + expect(difference).toBeLessThan(50) // Allow max 50px variance + } + }) +}) From 452f4c10874f0f578ebcf940bc98d5273dda9e34 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Thu, 21 May 2026 13:34:58 +0200 Subject: [PATCH 2/9] Add SelectPanel outside-top regression fixture --- .../SelectPanel.examples.stories.tsx | 154 ++++++------------ 1 file changed, 49 insertions(+), 105 deletions(-) diff --git a/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx b/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx index 44e1ab93917..b821a9facc9 100644 --- a/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx @@ -356,7 +356,6 @@ export const RepositionAfterLoading = () => { const [loading, setLoading] = useState(true) React.useEffect(() => { - // eslint-disable-next-line react-hooks/set-state-in-effect if (!open) setLoading(true) window.setTimeout(() => { if (open) { @@ -369,7 +368,6 @@ export const RepositionAfterLoading = () => { React.useEffect(() => { if (!loading) { - // eslint-disable-next-line react-hooks/set-state-in-effect setFilteredItems(items.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))) } // eslint-disable-next-line react-hooks/exhaustive-deps @@ -405,7 +403,6 @@ export const SelectPanelRepositionInsideDialog = () => { const [loading, setLoading] = useState(true) React.useEffect(() => { - // eslint-disable-next-line react-hooks/set-state-in-effect if (!open) setLoading(true) window.setTimeout(() => { if (open) { @@ -418,7 +415,6 @@ export const SelectPanelRepositionInsideDialog = () => { React.useEffect(() => { if (!loading) { - // eslint-disable-next-line react-hooks/set-state-in-effect setFilteredItems(items.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))) } // eslint-disable-next-line react-hooks/exhaustive-deps @@ -438,7 +434,6 @@ export const SelectPanelRepositionInsideDialog = () => { selected={selected} onSelectedChange={setSelected} onFilterChange={setFilter} - overlayProps={{anchorSide: 'outside-top'}} message={filteredItems.length === 0 ? NoResultsMessage(filter) : undefined} /> @@ -446,6 +441,55 @@ export const SelectPanelRepositionInsideDialog = () => { ) } +export const AutogrowAfterLoadingWithOutsideTopAnchor = () => { + const autogrowItems = [...items] + + const [selected, setSelected] = React.useState([autogrowItems[0], autogrowItems[1]]) + const [open, setOpen] = useState(false) + const [filter, setFilter] = React.useState('') + const [filteredItems, setFilteredItems] = React.useState([]) + const [loading, setLoading] = useState(true) + + React.useEffect(() => { + if (!open) setLoading(true) + const timer = window.setTimeout(() => { + if (open) { + setFilteredItems(autogrowItems.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))) + setLoading(false) + } + }, 2000) + + return () => window.clearTimeout(timer) + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [open]) + + React.useEffect(() => { + if (!loading) { + setFilteredItems(autogrowItems.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))) + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [filter]) + + return ( + +

Autogrow panel after loading with outside-top anchor

+ + + ) +} + export const WithDefaultMessage = () => { const [selected, setSelected] = useState(items.slice(1, 3)) const [filter, setFilter] = useState('') @@ -624,7 +668,6 @@ export const VirtualizedConsumerSide = () => { [open], ) - // eslint-disable-next-line react-hooks/incompatible-library const virtualizer = useVirtualizer({ count: filteredItems.length, getScrollElement: () => scrollContainer ?? null, @@ -843,102 +886,3 @@ export const VirtualizedBuiltIn = () => { ) } - -/** - * Demonstrates the first-open sizing regression where the overlay - * appears too small and shows a scrollbar unnecessarily on first open. - * After closing and reopening, it usually renders correctly. - * - * Issue: https://github.com/primer/react/issues/1831 - */ -export const FirstOpenSizingDebug = () => { - const [selected, setSelected] = useState([]) - const [open, setOpen] = useState(false) - const [loading, setLoading] = useState(true) - const [filteredItems, setFilteredItems] = useState([]) - const [filter, setFilter] = useState('') - const overlayRef = useRef(null) - const [measurements, setMeasurements] = useState<{ - clientHeight?: number - scrollHeight?: number - scrollWidth?: number - hasScrollbar?: boolean - timestamp?: string - }>({}) - - // Simulate loading delay similar to the real issue - React.useEffect(() => { - if (!open) { - setLoading(true) - } - const timer = window.setTimeout(() => { - if (open) { - setFilteredItems(items.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))) - setLoading(false) - } - }, 1500) - return () => window.clearTimeout(timer) - }, [open, filter]) - - // Measure overlay dimensions every 100ms to catch sizing changes - React.useEffect(() => { - if (!open) return - - const interval = window.setInterval(() => { - const overlay = document.querySelector('[data-testid="overlay"]') as HTMLElement | null - if (overlay) { - const hasScrollbar = overlay.scrollHeight > overlay.clientHeight - setMeasurements({ - clientHeight: overlay.clientHeight, - scrollHeight: overlay.scrollHeight, - scrollWidth: overlay.scrollWidth, - hasScrollbar, - timestamp: new Date().toLocaleTimeString(), - }) - - console.log( - `[SelectPanel] Overlay measurements: clientHeight=${overlay.clientHeight}, scrollHeight=${overlay.scrollHeight}, hasScrollbar=${hasScrollbar}`, - ) - } - }, 100) - - return () => window.clearInterval(interval) - }, [open]) - - return ( - - First-Open Sizing Debug -
-

- Open={open} | Loading={loading} | Last measurement: {measurements.timestamp} -

-

- Overlay size: {measurements.clientHeight}px (client) × {measurements.scrollHeight}px (scroll) -

-

- Scrollbar: {measurements.hasScrollbar ? '✓ PRESENT (BUG!)' : '✗ Not present'} -

-
- - - ) -} From 34e02f013a8f3647574823de5db1b0aef495c25b Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Thu, 21 May 2026 14:13:02 +0200 Subject: [PATCH 3/9] Fix SelectPanel outside-top anchor sizing regression --- .../src/SelectPanel/SelectPanel.test.tsx | 2 +- .../__tests__/useAnchoredPosition.test.tsx | 184 +++++++++++++++++- .../react/src/hooks/useAnchoredPosition.ts | 33 +++- 3 files changed, 214 insertions(+), 5 deletions(-) diff --git a/packages/react/src/SelectPanel/SelectPanel.test.tsx b/packages/react/src/SelectPanel/SelectPanel.test.tsx index d43b751faff..2958ebf6304 100644 --- a/packages/react/src/SelectPanel/SelectPanel.test.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.test.tsx @@ -2294,7 +2294,7 @@ describe('SelectPanel displayInViewport prop', () => { /** * Test suite for SelectPanel first-open sizing regression - * Issue: https://github.com/primer/react/issues/1831 + * Issue: https://github.com/github/issues/issues/18331 * * On first open with loading state, SelectPanel may render shorter than its content * and show an unnecessary scrollbar. This occurs because position is calculated before diff --git a/packages/react/src/hooks/__tests__/useAnchoredPosition.test.tsx b/packages/react/src/hooks/__tests__/useAnchoredPosition.test.tsx index 98a4d58fc2e..cd36856bfe3 100644 --- a/packages/react/src/hooks/__tests__/useAnchoredPosition.test.tsx +++ b/packages/react/src/hooks/__tests__/useAnchoredPosition.test.tsx @@ -1,8 +1,40 @@ import {render, waitFor, act} from '@testing-library/react' -import {it, expect, vi, describe} from 'vitest' +import {it, expect, vi, describe, beforeEach, afterEach} from 'vitest' import React from 'react' import {useAnchoredPosition} from '../../hooks/useAnchoredPosition' +type MockedAnchorPosition = { + anchorAlign: string + anchorSide: string + left: number + top: number +} + +type BehaviorsModule = Record & { + getAnchoredPosition: (...args: unknown[]) => MockedAnchorPosition +} + +const {mockedGetAnchoredPosition} = vi.hoisted(() => ({ + mockedGetAnchoredPosition: vi.fn(), +})) + +vi.mock('@primer/behaviors', async importOriginal => { + const actual = (await importOriginal()) as BehaviorsModule + + return { + ...actual, + getAnchoredPosition: (...args: unknown[]) => { + const implementation = mockedGetAnchoredPosition.getMockImplementation() + + if (implementation) { + return mockedGetAnchoredPosition(...args) + } + + return actual.getAnchoredPosition(...args) + }, + } +}) + const Component = ({callback}: {callback: (hookReturnValue: ReturnType) => void}) => { const floatingElementRef = React.useRef(null) const anchorElementRef = React.useRef(null) @@ -18,6 +50,33 @@ const Component = ({callback}: {callback: (hookReturnValue: ReturnType) => void + step: number +}) => { + const floatingElementRef = React.useRef(null) + const anchorElementRef = React.useRef(null) + callback(useAnchoredPosition({floatingElementRef, anchorElementRef, pinPosition: true}, [step])) + + return ( +
+
+
+
+ ) +} + +beforeEach(() => { + mockedGetAnchoredPosition.mockReset() +}) + +afterEach(() => { + vi.restoreAllMocks() +}) + it('should should return a position', async () => { const cb = vi.fn() render() @@ -142,3 +201,126 @@ describe('scroll recalculation', () => { }) }) }) + +describe('pinPosition', () => { + it('allows outside-top overlays to grow when content needs more height', async () => { + const callback = vi.fn() + let currentTop = 200 + let floatingHeight = 147 + let floatingScrollHeight = 147 + let overflowingDescendantHeight = 147 + let overflowingDescendantScrollHeight = 147 + + mockedGetAnchoredPosition.mockImplementation(() => ({ + anchorAlign: 'start', + anchorSide: 'outside-top', + left: 0, + top: currentTop, + })) + + const requestAnimationFrameSpy = vi.spyOn(window, 'requestAnimationFrame').mockImplementation(callback => { + callback(0) + return 0 + }) + + const {container, rerender} = render() + const [floatingElement, anchorElement] = Array.from(container.firstElementChild!.children) as [ + HTMLDivElement, + HTMLDivElement, + ] + const overflowingDescendant = document.createElement('div') + floatingElement.append(overflowingDescendant) + + Object.defineProperty(floatingElement, 'clientHeight', {configurable: true, get: () => floatingHeight}) + Object.defineProperty(floatingElement, 'scrollHeight', {configurable: true, get: () => floatingScrollHeight}) + Object.defineProperty(overflowingDescendant, 'clientHeight', { + configurable: true, + get: () => overflowingDescendantHeight, + }) + Object.defineProperty(overflowingDescendant, 'scrollHeight', { + configurable: true, + get: () => overflowingDescendantScrollHeight, + }) + Object.defineProperty(anchorElement, 'getBoundingClientRect', { + configurable: true, + value: () => ({top: 500}), + }) + + await waitFor(() => { + expect(callback.mock.lastCall?.[0].position.top).toBe(200) + }) + + rerender() + + await waitFor(() => { + expect(callback.mock.lastCall?.[0].position.top).toBe(200) + }) + + currentTop = 210 + floatingHeight = 91 + floatingScrollHeight = 91 + overflowingDescendantHeight = 91 + overflowingDescendantScrollHeight = 317 + rerender() + + await waitFor(() => { + expect(callback.mock.lastCall?.[0].position.top).toBe(210) + }) + + expect(floatingElement.style.height).toBe('') + expect(requestAnimationFrameSpy).toHaveBeenCalled() + }) + + it('keeps the previous height when outside-top content is genuinely shrinking', async () => { + const callback = vi.fn() + let currentTop = 200 + let floatingHeight = 147 + let floatingScrollHeight = 147 + + mockedGetAnchoredPosition.mockImplementation(() => ({ + anchorAlign: 'start', + anchorSide: 'outside-top', + left: 0, + top: currentTop, + })) + + vi.spyOn(window, 'requestAnimationFrame').mockImplementation(callback => { + callback(0) + return 0 + }) + + const {container, rerender} = render() + const [floatingElement, anchorElement] = Array.from(container.firstElementChild!.children) as [ + HTMLDivElement, + HTMLDivElement, + ] + + Object.defineProperty(floatingElement, 'clientHeight', {configurable: true, get: () => floatingHeight}) + Object.defineProperty(floatingElement, 'scrollHeight', {configurable: true, get: () => floatingScrollHeight}) + Object.defineProperty(anchorElement, 'getBoundingClientRect', { + configurable: true, + value: () => ({top: 500}), + }) + + await waitFor(() => { + expect(callback.mock.lastCall?.[0].position.top).toBe(200) + }) + + rerender() + + await waitFor(() => { + expect(callback.mock.lastCall?.[0].position.top).toBe(200) + }) + + currentTop = 210 + floatingHeight = 91 + floatingScrollHeight = 91 + rerender() + + await waitFor(() => { + expect(callback.mock.lastCall?.[0].position.top).toBe(200) + }) + + expect(floatingElement.style.height).toBe('147px') + }) +}) diff --git a/packages/react/src/hooks/useAnchoredPosition.ts b/packages/react/src/hooks/useAnchoredPosition.ts index 611d1b1e920..15309ba6440 100644 --- a/packages/react/src/hooks/useAnchoredPosition.ts +++ b/packages/react/src/hooks/useAnchoredPosition.ts @@ -34,6 +34,18 @@ export interface AnchoredPositionHookSettings extends Partial enabled?: boolean } +function hasOverflowingDescendant(element: HTMLElement | null) { + if (!element) return false + + for (const descendant of element.querySelectorAll('*')) { + if (descendant.scrollHeight > descendant.clientHeight || descendant.scrollWidth > descendant.clientWidth) { + return true + } + } + + return false +} + /** * Calculates the top and left values for an absolutely-positioned floating element * to be anchored to some anchor element. Returns refs for the floating element @@ -71,8 +83,23 @@ export function useAnchoredPosition( const updateElementHeight = () => { let heightUpdated = false setPrevHeight(prevHeight => { - // if the element is trying to shrink in height, restore to old height to prevent it from jumping - if (prevHeight && prevHeight > (floatingElementRef.current?.clientHeight ?? 0)) { + const floatingElement = floatingElementRef.current as HTMLElement | null + const currentHeight = floatingElement?.clientHeight ?? 0 + const desiredHeight = Math.max(floatingElement?.scrollHeight ?? 0, currentHeight) + const contentNeedsMoreHeight = desiredHeight > prevHeight || hasOverflowingDescendant(floatingElement) + + // if the element is trying to shrink in height, restore to old height to prevent it from jumping. + // When the content now needs to grow past the previous height, clear any stale inline height instead. + if (prevHeight && prevHeight > currentHeight) { + if (contentNeedsMoreHeight) { + requestAnimationFrame(() => { + if (floatingElementRef.current instanceof HTMLElement) { + floatingElementRef.current.style.height = '' + } + }) + return prevHeight + } + requestAnimationFrame(() => { ;(floatingElementRef.current as HTMLElement).style.height = `${prevHeight}px` }) @@ -111,7 +138,7 @@ export function useAnchoredPosition( } setPrevHeight(floatingElementRef.current?.clientHeight) }, - // eslint-disable-next-line react-hooks/exhaustive-deps, react-hooks/use-memo + // eslint-disable-next-line react-hooks/exhaustive-deps [floatingElementRef, anchorElementRef, enabled, ...dependencies], ) From 9f91660a8c40b4b94fa3164847faf0739f97d973 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Thu, 21 May 2026 14:24:14 +0200 Subject: [PATCH 4/9] fix: remove console.log and unused vars in tests --- packages/react/src/SelectPanel/SelectPanel.test.tsx | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/packages/react/src/SelectPanel/SelectPanel.test.tsx b/packages/react/src/SelectPanel/SelectPanel.test.tsx index 2958ebf6304..d426a990f08 100644 --- a/packages/react/src/SelectPanel/SelectPanel.test.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.test.tsx @@ -2372,10 +2372,8 @@ describe('SelectPanel - First-Open Sizing with Loading State', () => { {timeout: 1000}, ) - // Get overlay measurements while still loading + // Get overlay element const overlay = document.querySelector('[data-testid="overlay"]') as HTMLElement | null - const clientHeightWhileLoading = overlay?.clientHeight - const scrollHeightWhileLoading = overlay?.scrollHeight // Wait for items to load await waitFor( @@ -2390,12 +2388,6 @@ describe('SelectPanel - First-Open Sizing with Loading State', () => { const clientHeightAfterLoading = overlay?.clientHeight const scrollHeightAfterLoading = overlay?.scrollHeight - // Log measurements for debugging - console.log('First-open sizing measurements:', { - whileLoading: {clientHeight: clientHeightWhileLoading, scrollHeight: scrollHeightWhileLoading}, - afterLoading: {clientHeight: clientHeightAfterLoading, scrollHeight: scrollHeightAfterLoading}, - }) - // The overlay should have enough height to fit all content without scrollbar if (clientHeightAfterLoading && scrollHeightAfterLoading) { const hasScrollbar = scrollHeightAfterLoading > clientHeightAfterLoading @@ -2405,7 +2397,7 @@ describe('SelectPanel - First-Open Sizing with Loading State', () => { it('should have consistent height between first and second open', async () => { const user = userEvent.setup() - const {unmount, rerender} = render() + render() const button = screen.getByRole('button', {name: /Select item/i}) @@ -2442,7 +2434,6 @@ describe('SelectPanel - First-Open Sizing with Loading State', () => { // Heights should be very similar (allowing for minor variations) if (height1 && height2) { const difference = Math.abs(height1 - height2) - console.log(`Height consistency check: first=${height1}px, second=${height2}px, diff=${difference}px`) expect(difference).toBeLessThan(50) // Allow max 50px variance } }) From 248f32aec025afa1ed0abd6cc8a33631ea0dd3bd Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Thu, 21 May 2026 15:41:31 +0200 Subject: [PATCH 5/9] fix: address type errors in useAnchoredPosition and test --- packages/react/src/SelectPanel/SelectPanel.test.tsx | 1 + packages/react/src/hooks/useAnchoredPosition.ts | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/react/src/SelectPanel/SelectPanel.test.tsx b/packages/react/src/SelectPanel/SelectPanel.test.tsx index d426a990f08..c7339413327 100644 --- a/packages/react/src/SelectPanel/SelectPanel.test.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.test.tsx @@ -2342,6 +2342,7 @@ describe('SelectPanel - First-Open Sizing with Loading State', () => { title="Select item" open={open} onOpenChange={setOpen} + onFilterChange={() => {}} loading={loading} items={visibleItems} selected={selected} diff --git a/packages/react/src/hooks/useAnchoredPosition.ts b/packages/react/src/hooks/useAnchoredPosition.ts index 15309ba6440..e2b5b1bb84a 100644 --- a/packages/react/src/hooks/useAnchoredPosition.ts +++ b/packages/react/src/hooks/useAnchoredPosition.ts @@ -86,7 +86,8 @@ export function useAnchoredPosition( const floatingElement = floatingElementRef.current as HTMLElement | null const currentHeight = floatingElement?.clientHeight ?? 0 const desiredHeight = Math.max(floatingElement?.scrollHeight ?? 0, currentHeight) - const contentNeedsMoreHeight = desiredHeight > prevHeight || hasOverflowingDescendant(floatingElement) + const contentNeedsMoreHeight = + (prevHeight !== undefined && desiredHeight > prevHeight) || hasOverflowingDescendant(floatingElement) // if the element is trying to shrink in height, restore to old height to prevent it from jumping. // When the content now needs to grow past the previous height, clear any stale inline height instead. From 6c6c6f094971a9a45b8dd4832029501d099d0f17 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Thu, 21 May 2026 16:42:29 +0200 Subject: [PATCH 6/9] fix: add eslint-disable for set-state-in-effect in stories --- .../react/src/SelectPanel/SelectPanel.examples.stories.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx b/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx index b821a9facc9..bcb6999da3c 100644 --- a/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx @@ -356,6 +356,7 @@ export const RepositionAfterLoading = () => { const [loading, setLoading] = useState(true) React.useEffect(() => { + // eslint-disable-next-line react-hooks/set-state-in-effect -- Reset loading state when panel closes if (!open) setLoading(true) window.setTimeout(() => { if (open) { @@ -403,6 +404,7 @@ export const SelectPanelRepositionInsideDialog = () => { const [loading, setLoading] = useState(true) React.useEffect(() => { + // eslint-disable-next-line react-hooks/set-state-in-effect -- Reset loading state when panel closes if (!open) setLoading(true) window.setTimeout(() => { if (open) { @@ -451,6 +453,7 @@ export const AutogrowAfterLoadingWithOutsideTopAnchor = () => { const [loading, setLoading] = useState(true) React.useEffect(() => { + // eslint-disable-next-line react-hooks/set-state-in-effect -- Reset loading state when panel closes if (!open) setLoading(true) const timer = window.setTimeout(() => { if (open) { From f5b0fb4c2ab75d5f62b5cd0689322209903f76b9 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Mon, 1 Jun 2026 10:34:02 +0200 Subject: [PATCH 7/9] refactor: move setLoading from effect to onOpenChange handler --- .../SelectPanel.examples.stories.tsx | 33 ++++++++++++------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx b/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx index bcb6999da3c..0fcb943041f 100644 --- a/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx @@ -355,15 +355,19 @@ export const RepositionAfterLoading = () => { const [loading, setLoading] = useState(true) + const handleOpenChange = (isOpen: boolean) => { + if (!isOpen) setLoading(true) + setOpen(isOpen) + } + React.useEffect(() => { - // eslint-disable-next-line react-hooks/set-state-in-effect -- Reset loading state when panel closes - if (!open) setLoading(true) - window.setTimeout(() => { + const timer = window.setTimeout(() => { if (open) { setFilteredItems(items.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))) setLoading(false) } }, 2000) + return () => window.clearTimeout(timer) // eslint-disable-next-line react-hooks/exhaustive-deps }, [open]) @@ -383,7 +387,7 @@ export const RepositionAfterLoading = () => { title="Select labels" placeholderText="Filter Labels" open={open} - onOpenChange={setOpen} + onOpenChange={handleOpenChange} items={filteredItems} selected={selected} onSelectedChange={setSelected} @@ -403,15 +407,19 @@ export const SelectPanelRepositionInsideDialog = () => { const [loading, setLoading] = useState(true) + const handleOpenChange = (isOpen: boolean) => { + if (!isOpen) setLoading(true) + setOpen(isOpen) + } + React.useEffect(() => { - // eslint-disable-next-line react-hooks/set-state-in-effect -- Reset loading state when panel closes - if (!open) setLoading(true) - window.setTimeout(() => { + const timer = window.setTimeout(() => { if (open) { setFilteredItems(items.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))) setLoading(false) } }, 2000) + return () => window.clearTimeout(timer) // eslint-disable-next-line react-hooks/exhaustive-deps }, [open]) @@ -431,7 +439,7 @@ export const SelectPanelRepositionInsideDialog = () => { title="Select labels" placeholderText="Filter Labels" open={open} - onOpenChange={setOpen} + onOpenChange={handleOpenChange} items={filteredItems} selected={selected} onSelectedChange={setSelected} @@ -452,9 +460,12 @@ export const AutogrowAfterLoadingWithOutsideTopAnchor = () => { const [filteredItems, setFilteredItems] = React.useState([]) const [loading, setLoading] = useState(true) + const handleOpenChange = (isOpen: boolean) => { + if (!isOpen) setLoading(true) + setOpen(isOpen) + } + React.useEffect(() => { - // eslint-disable-next-line react-hooks/set-state-in-effect -- Reset loading state when panel closes - if (!open) setLoading(true) const timer = window.setTimeout(() => { if (open) { setFilteredItems(autogrowItems.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))) @@ -481,7 +492,7 @@ export const AutogrowAfterLoadingWithOutsideTopAnchor = () => { title="Select labels" placeholderText="Filter Labels" open={open} - onOpenChange={setOpen} + onOpenChange={handleOpenChange} items={filteredItems} selected={selected} onSelectedChange={setSelected} From c85b1d229c4ba149640fb1787c877835c305a89c Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Mon, 1 Jun 2026 11:09:12 +0200 Subject: [PATCH 8/9] fix: resolve lint errors for CI --- package-lock.json | 18 +++++++++--------- .../SelectPanel.examples.stories.tsx | 4 ++++ .../react/src/SelectPanel/SelectPanel.test.tsx | 1 + .../react/src/hooks/useAnchoredPosition.ts | 2 +- 4 files changed, 15 insertions(+), 10 deletions(-) diff --git a/package-lock.json b/package-lock.json index e079fe95480..f1389c85685 100644 --- a/package-lock.json +++ b/package-lock.json @@ -82,8 +82,8 @@ "react-dom": "^18.3.1" }, "devDependencies": { - "@primer/react": "38.25.0", - "@primer/styled-react": "1.0.9", + "@primer/react": "38.26.0", + "@primer/styled-react": "1.1.0", "@types/react": "^18.3.11", "@types/react-dom": "^18.3.0", "@vitejs/plugin-react": "^4.3.3", @@ -96,8 +96,8 @@ "name": "example-nextjs", "version": "0.0.0", "dependencies": { - "@primer/react": "38.25.0", - "@primer/styled-react": "1.0.9", + "@primer/react": "38.26.0", + "@primer/styled-react": "1.1.0", "next": "^16.1.7", "react": "^19.2.0", "react-dom": "^19.2.0", @@ -139,8 +139,8 @@ "version": "0.0.0", "dependencies": { "@primer/octicons-react": "^19.21.0", - "@primer/react": "38.25.0", - "@primer/styled-react": "1.0.9", + "@primer/react": "38.26.0", + "@primer/styled-react": "1.1.0", "clsx": "^2.1.1", "next": "^16.1.7", "react": "^19.2.0", @@ -27700,7 +27700,7 @@ }, "packages/react": { "name": "@primer/react", - "version": "38.25.0", + "version": "38.26.0", "license": "MIT", "dependencies": { "@github/mini-throttle": "^2.1.1", @@ -28088,7 +28088,7 @@ }, "packages/styled-react": { "name": "@primer/styled-react", - "version": "1.0.9", + "version": "1.1.0", "dependencies": { "@styled-system/css": "^5.1.5", "@styled-system/props": "^5.1.5", @@ -28105,7 +28105,7 @@ "@babel/preset-react": "^7.28.5", "@babel/preset-typescript": "^7.28.5", "@primer/primitives": "10.x || 11.x", - "@primer/react": "^38.24.0", + "@primer/react": "^38.26.0", "@rollup/plugin-babel": "^6.1.0", "@storybook/react-vite": "^10.3.3", "@types/react": "18.3.11", diff --git a/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx b/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx index 0fcb943041f..71d6406cf59 100644 --- a/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx @@ -373,6 +373,7 @@ export const RepositionAfterLoading = () => { React.useEffect(() => { if (!loading) { + // eslint-disable-next-line react-hooks/set-state-in-effect -- Updating filtered items based on filter change setFilteredItems(items.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))) } // eslint-disable-next-line react-hooks/exhaustive-deps @@ -425,6 +426,7 @@ export const SelectPanelRepositionInsideDialog = () => { React.useEffect(() => { if (!loading) { + // eslint-disable-next-line react-hooks/set-state-in-effect -- Updating filtered items based on filter change setFilteredItems(items.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))) } // eslint-disable-next-line react-hooks/exhaustive-deps @@ -479,6 +481,7 @@ export const AutogrowAfterLoadingWithOutsideTopAnchor = () => { React.useEffect(() => { if (!loading) { + // eslint-disable-next-line react-hooks/set-state-in-effect -- Updating filtered items based on filter change setFilteredItems(autogrowItems.filter(item => item.text.toLowerCase().startsWith(filter.toLowerCase()))) } // eslint-disable-next-line react-hooks/exhaustive-deps @@ -682,6 +685,7 @@ export const VirtualizedConsumerSide = () => { [open], ) + // eslint-disable-next-line react-hooks/incompatible-library const virtualizer = useVirtualizer({ count: filteredItems.length, getScrollElement: () => scrollContainer ?? null, diff --git a/packages/react/src/SelectPanel/SelectPanel.test.tsx b/packages/react/src/SelectPanel/SelectPanel.test.tsx index c7339413327..b08eddba4de 100644 --- a/packages/react/src/SelectPanel/SelectPanel.test.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.test.tsx @@ -2322,6 +2322,7 @@ describe('SelectPanel - First-Open Sizing with Loading State', () => { // Simulate items loading after a delay (typical async data fetch) React.useEffect(() => { if (!open) { + // eslint-disable-next-line react-hooks/set-state-in-effect -- Reset loading state for test fixture setLoading(true) setVisibleItems([]) } diff --git a/packages/react/src/hooks/useAnchoredPosition.ts b/packages/react/src/hooks/useAnchoredPosition.ts index e2b5b1bb84a..df66cf6c89c 100644 --- a/packages/react/src/hooks/useAnchoredPosition.ts +++ b/packages/react/src/hooks/useAnchoredPosition.ts @@ -139,7 +139,7 @@ export function useAnchoredPosition( } setPrevHeight(floatingElementRef.current?.clientHeight) }, - // eslint-disable-next-line react-hooks/exhaustive-deps + // eslint-disable-next-line react-hooks/exhaustive-deps, react-hooks/use-memo [floatingElementRef, anchorElementRef, enabled, ...dependencies], ) From 91eaa0a728c80d8eea52efd2e05ba222e2df1ba6 Mon Sep 17 00:00:00 2001 From: Siddharth Kshetrapal Date: Mon, 1 Jun 2026 11:20:49 +0200 Subject: [PATCH 9/9] fix: correct test button selector and state rendering --- .../react/src/SelectPanel/SelectPanel.test.tsx | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/packages/react/src/SelectPanel/SelectPanel.test.tsx b/packages/react/src/SelectPanel/SelectPanel.test.tsx index b08eddba4de..1ede525fd17 100644 --- a/packages/react/src/SelectPanel/SelectPanel.test.tsx +++ b/packages/react/src/SelectPanel/SelectPanel.test.tsx @@ -2351,7 +2351,7 @@ describe('SelectPanel - First-Open Sizing with Loading State', () => { height="large" />
- open:{open} loading:{loading} itemsCount:{visibleItems.length} + open:{String(open)} loading:{String(loading)} itemsCount:{visibleItems.length}
) @@ -2361,7 +2361,7 @@ describe('SelectPanel - First-Open Sizing with Loading State', () => { const user = userEvent.setup() render() - const button = screen.getByRole('button', {name: /Select item/i}) + const button = screen.getByRole('button', {name: /Item 1/i}) // Open SelectPanel for the first time await user.click(button) @@ -2401,14 +2401,17 @@ describe('SelectPanel - First-Open Sizing with Loading State', () => { const user = userEvent.setup() render() - const button = screen.getByRole('button', {name: /Select item/i}) + const button = screen.getByRole('button', {name: /Item 1/i}) // First open await user.click(button) - await waitFor(() => { - const state = screen.getByTestId('state') - expect(state).toHaveTextContent('loading:false') - }) + await waitFor( + () => { + const state = screen.getByTestId('state') + expect(state).toHaveTextContent('loading:false') + }, + {timeout: 1500}, + ) const overlay1 = document.querySelector('[data-testid="overlay"]') const height1 = overlay1?.getBoundingClientRect().height