diff --git a/.changeset/merged-forwarded-refs.md b/.changeset/merged-forwarded-refs.md new file mode 100644 index 00000000000..d5917de3135 --- /dev/null +++ b/.changeset/merged-forwarded-refs.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +Ref handling: Preserve forwarded refs while maintaining internal component refs. diff --git a/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-dark-colorblind-linux.png b/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-dark-colorblind-linux.png index f8e1ab05535..87c73fb218a 100644 Binary files a/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-dark-colorblind-linux.png and b/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-dark-colorblind-linux.png differ diff --git a/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-dark-dimmed-linux.png b/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-dark-dimmed-linux.png index cfb6cd79bd2..b1b779bee07 100644 Binary files a/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-dark-dimmed-linux.png and b/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-dark-dimmed-linux.png differ diff --git a/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-dark-high-contrast-linux.png b/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-dark-high-contrast-linux.png index ce73d055d7c..af7c469d762 100644 Binary files a/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-dark-high-contrast-linux.png and b/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-dark-high-contrast-linux.png differ diff --git a/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-dark-linux.png b/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-dark-linux.png index f8e1ab05535..87c73fb218a 100644 Binary files a/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-dark-linux.png and b/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-dark-linux.png differ diff --git a/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-dark-tritanopia-linux.png b/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-dark-tritanopia-linux.png index f8e1ab05535..87c73fb218a 100644 Binary files a/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-dark-tritanopia-linux.png and b/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-dark-tritanopia-linux.png differ diff --git a/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-light-colorblind-linux.png b/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-light-colorblind-linux.png index 9fc682e174c..664f9696e21 100644 Binary files a/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-light-colorblind-linux.png and b/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-light-colorblind-linux.png differ diff --git a/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-light-high-contrast-linux.png b/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-light-high-contrast-linux.png index 39f2562bc5d..42f5c15d17c 100644 Binary files a/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-light-high-contrast-linux.png and b/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-light-high-contrast-linux.png differ diff --git a/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-light-linux.png b/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-light-linux.png index 9fc682e174c..664f9696e21 100644 Binary files a/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-light-linux.png and b/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-light-linux.png differ diff --git a/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-light-tritanopia-linux.png b/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-light-tritanopia-linux.png index 9fc682e174c..664f9696e21 100644 Binary files a/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-light-tritanopia-linux.png and b/.playwright/snapshots/components/FilteredActionList.test.ts-snapshots/FilteredActionList-With-Groups-light-tritanopia-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-dark-colorblind-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-dark-colorblind-linux.png index be025ee96fb..c870ea3fe83 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-dark-colorblind-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-dark-colorblind-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-dark-dimmed-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-dark-dimmed-linux.png index 195d312bde8..85c3dee13ca 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-dark-dimmed-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-dark-dimmed-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-dark-high-contrast-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-dark-high-contrast-linux.png index a56e53a3b0a..e1be5c29043 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-dark-high-contrast-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-dark-high-contrast-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-dark-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-dark-linux.png index a24d2e156b4..c870ea3fe83 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-dark-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-dark-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-dark-tritanopia-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-dark-tritanopia-linux.png index be025ee96fb..c870ea3fe83 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-dark-tritanopia-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-dark-tritanopia-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-light-colorblind-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-light-colorblind-linux.png index a9015d4d37b..99379a6989e 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-light-colorblind-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-light-colorblind-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-light-high-contrast-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-light-high-contrast-linux.png index c55bf9bcb7e..c8bd1f00321 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-light-high-contrast-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-light-high-contrast-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-light-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-light-linux.png index a9015d4d37b..99379a6989e 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-light-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-light-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-light-tritanopia-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-light-tritanopia-linux.png index a9015d4d37b..99379a6989e 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-light-tritanopia-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-light-tritanopia-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-responsive-width-light-full-screen-on-narrow--true-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-responsive-width-light-full-screen-on-narrow--true-linux.png index c22b4644884..c1c44c4bfb1 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-responsive-width-light-full-screen-on-narrow--true-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-Default-responsive-width-light-full-screen-on-narrow--true-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-dark-colorblind-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-dark-colorblind-linux.png index 408a2bad5b9..bce50439f88 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-dark-colorblind-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-dark-colorblind-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-dark-dimmed-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-dark-dimmed-linux.png index bb9f7df2f7b..8e8219e8a46 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-dark-dimmed-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-dark-dimmed-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-dark-high-contrast-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-dark-high-contrast-linux.png index 533328d276f..5a10d6e1fa9 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-dark-high-contrast-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-dark-high-contrast-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-dark-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-dark-linux.png index 408a2bad5b9..bce50439f88 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-dark-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-dark-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-dark-tritanopia-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-dark-tritanopia-linux.png index 408a2bad5b9..bce50439f88 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-dark-tritanopia-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-dark-tritanopia-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-light-colorblind-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-light-colorblind-linux.png index 42686117360..b260465cea3 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-light-colorblind-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-light-colorblind-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-light-high-contrast-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-light-high-contrast-linux.png index ac0378e51d0..a40e96ffee8 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-light-high-contrast-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-light-high-contrast-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-light-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-light-linux.png index 42686117360..b260465cea3 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-light-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-light-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-light-tritanopia-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-light-tritanopia-linux.png index 42686117360..b260465cea3 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-light-tritanopia-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Css-light-tritanopia-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-dark-colorblind-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-dark-colorblind-linux.png index fe075f7c5bf..d2ceb63254d 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-dark-colorblind-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-dark-colorblind-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-dark-dimmed-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-dark-dimmed-linux.png index e84e2ed92b5..9ec9289de43 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-dark-dimmed-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-dark-dimmed-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-dark-high-contrast-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-dark-high-contrast-linux.png index 59db6eb7fb4..2b0b7170f97 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-dark-high-contrast-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-dark-high-contrast-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-dark-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-dark-linux.png index fe075f7c5bf..d2ceb63254d 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-dark-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-dark-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-dark-tritanopia-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-dark-tritanopia-linux.png index fe075f7c5bf..d2ceb63254d 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-dark-tritanopia-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-dark-tritanopia-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-light-colorblind-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-light-colorblind-linux.png index f48a70315c0..938da7ceb1c 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-light-colorblind-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-light-colorblind-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-light-high-contrast-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-light-high-contrast-linux.png index ee3a3cf3edc..e8b2b99713f 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-light-high-contrast-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-light-high-contrast-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-light-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-light-linux.png index f48a70315c0..938da7ceb1c 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-light-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-light-linux.png differ diff --git a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-light-tritanopia-linux.png b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-light-tritanopia-linux.png index f48a70315c0..938da7ceb1c 100644 Binary files a/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-light-tritanopia-linux.png and b/.playwright/snapshots/components/SelectPanel.test.ts-snapshots/SelectPanel-With-Groups-light-tritanopia-linux.png differ diff --git a/packages/react/src/ActionList/ActionList.test.tsx b/packages/react/src/ActionList/ActionList.test.tsx index e12b21c5bb6..4740ae58a7f 100644 --- a/packages/react/src/ActionList/ActionList.test.tsx +++ b/packages/react/src/ActionList/ActionList.test.tsx @@ -615,32 +615,24 @@ describe('ActionList with role="tree"', () => { }) }) -describe('ActionList forwarded ref (primer_react_merged_forwarded_refs)', () => { - for (const enabled of [true, false]) { - describe(`with the flag ${enabled ? 'enabled' : 'disabled'}`, () => { - it('forwards a ref object to the element', () => { - const ref = createRef() - HTMLRender( - - - Item - - , - ) - expect(ref.current).toBeInstanceOf(HTMLElement) - }) - - it('calls a callback ref with the element', () => { - const refCallback = vi.fn() - HTMLRender( - - - Item - - , - ) - expect(refCallback.mock.calls.some(([el]) => el instanceof HTMLElement)).toBe(true) - }) - }) - } +describe('ActionList forwarded ref', () => { + it('forwards a ref object to the element', () => { + const ref = createRef() + HTMLRender( + + Item + , + ) + expect(ref.current).toBeInstanceOf(HTMLElement) + }) + + it('calls a callback ref with the element', () => { + const refCallback = vi.fn() + HTMLRender( + + Item + , + ) + expect(refCallback.mock.calls.some(([el]) => el instanceof HTMLElement)).toBe(true) + }) }) diff --git a/packages/react/src/ActionList/List.tsx b/packages/react/src/ActionList/List.tsx index 0abeb7e4722..a548c095d3c 100644 --- a/packages/react/src/ActionList/List.tsx +++ b/packages/react/src/ActionList/List.tsx @@ -5,7 +5,7 @@ import {useSlots} from '../hooks/useSlots' import {Heading} from './Heading' import {useId} from '../hooks/useId' import {ListContext, type ActionListProps} from './shared' -import {useMergedRefs, useProvidedRefOrCreate} from '../hooks' +import {useMergedRefs} from '../hooks' import {FocusKeys, useFocusZone} from '../hooks/useFocusZone' import {clsx} from 'clsx' import classes from './ActionList.module.css' @@ -44,14 +44,8 @@ const UnwrappedList = ( const ariaLabelledBy = slots.heading ? (slots.heading.props.id ?? headingId) : listLabelledBy const listRole = role || listRoleFromContainer - const mergedRefEnabled = useFeatureFlag('primer_react_merged_forwarded_refs') const listRef = useRef(null) const mergedRef = useMergedRefs(listRef, forwardedRef) - // Feature-flag scaffolding for `primer_react_merged_forwarded_refs`. - // At graduation: remove the three declarations below, and replace all instances of `readRef` with `listRef` and `appliedRef` with `mergedRef`. - const providedOrCreatedRef = useProvidedRefOrCreate(forwardedRef as React.RefObject) - const readRef = mergedRefEnabled ? listRef : providedOrCreatedRef - const appliedRef = mergedRefEnabled ? mergedRef : providedOrCreatedRef const itemGapFlag = useFeatureFlag('primer_react_action_list_item_gap') const itemGapEnabled = itemGapFlag && !disableItemGap && container === 'NavList' @@ -61,7 +55,7 @@ const UnwrappedList = ( useFocusZone({ disabled: !enableFocusZone, - containerRef: readRef, + containerRef: listRef, bindKeys: FocusKeys.ArrowVertical | FocusKeys.HomeAndEnd | FocusKeys.PageUpDown, focusOutBehavior: listRole === 'menu' || container === 'SelectPanel' || container === 'FilteredActionList' ? 'wrap' : undefined, @@ -93,7 +87,7 @@ const UnwrappedList = ( // Two querySelector calls after render is trivially cheap compared to what the browser // was doing on every DOM mutation with `:has()`. useIsomorphicLayoutEffect(() => { - const list = readRef.current + const list = listRef.current if (!list) return const hasMixed = list.querySelector('[data-has-description="true"]') !== null && @@ -109,12 +103,11 @@ const UnwrappedList = ( return ( {slots.heading} - {/* @ts-expect-error ref needs a non nullable ref */} { }) describe('AnchoredOverlay anchor element replacement', () => { + it('passes the provided ref object to the anchor renderer', () => { + const anchorRef = createRef() + let renderedRef: React.Ref | undefined + + render( + {}} + onClose={() => {}} + anchorRef={anchorRef} + renderAnchor={props => { + renderedRef = (props as typeof props & {ref?: React.Ref}).ref + return ( + + ) + }} + > + Content + , + ) + + expect(renderedRef).toBe(anchorRef) + expect(anchorRef.current).toBeInstanceOf(HTMLButtonElement) + }) + it('should re-apply anchor-name to a new anchor DOM element when the overlay reopens', () => { function TestComponent() { const anchorRef = useRef(null) @@ -766,48 +793,40 @@ describe('AnchoredOverlay anchor element replacement', () => { }) }) -describe('AnchoredOverlay overlay forwarded ref (primer_react_merged_forwarded_refs)', () => { - for (const enabled of [true, false]) { - describe(`with the flag ${enabled ? 'enabled' : 'disabled'}`, () => { - it('forwards a ref object to the overlay', () => { - const ref = createRef() - render( - - - {}} - onOpen={() => {}} - renderAnchor={props => } - overlayProps={{ref}} - > - - - - , - ) - expect(ref.current).toBeInstanceOf(HTMLDivElement) - }) +describe('AnchoredOverlay overlay forwarded ref', () => { + it('forwards a ref object to the overlay', () => { + const ref = createRef() + render( + + {}} + onOpen={() => {}} + renderAnchor={props => } + overlayProps={{ref}} + > + + + , + ) + expect(ref.current).toBeInstanceOf(HTMLDivElement) + }) - it('calls a callback ref with the overlay', () => { - const refCallback = vi.fn() - render( - - - {}} - onOpen={() => {}} - renderAnchor={props => } - overlayProps={{ref: refCallback}} - > - - - - , - ) - expect(refCallback.mock.calls.some(([el]) => el instanceof HTMLDivElement)).toBe(true) - }) - }) - } + it('calls a callback ref with the overlay', () => { + const refCallback = vi.fn() + render( + + {}} + onOpen={() => {}} + renderAnchor={props => } + overlayProps={{ref: refCallback}} + > + + + , + ) + expect(refCallback.mock.calls.some(([el]) => el instanceof HTMLDivElement)).toBe(true) + }) }) diff --git a/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx b/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx index bc373025e56..b9997bee379 100644 --- a/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx +++ b/packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx @@ -202,12 +202,12 @@ export const AnchoredOverlay: React.FC(null) - // eslint-disable-next-line react-hooks/refs - if (anchorRef.current !== anchorElement) { + // The object ref can point to a replacement element without changing identity. + // eslint-disable-next-line react-hooks/exhaustive-deps + useEffect(() => { setAnchorElement(anchorRef.current) - } + }) const [overlayRef, updateOverlayRef] = useRenderForcingRef() - const mergedRefEnabled = useFeatureFlag('primer_react_merged_forwarded_refs') const [overlayElement, setOverlayElement] = useState(null) const mergedOverlayRef = useMergedRefs(updateOverlayRef, useMergedRefs(overlayProps?.ref, setOverlayElement)) const anchorId = useId(externalAnchorId) @@ -438,17 +438,7 @@ export const AnchoredOverlay: React.FC { - setOverlayElement(node) - if (overlayProps?.ref) { - assignRef(overlayProps.ref, node) - } - updateOverlayRef(node) - } - } + ref={mergedOverlayRef} data-anchor-position={cssAnchorPositioning} data-side={cssAnchorPositioning ? side : position?.anchorSide} > @@ -556,15 +546,4 @@ function getCSSAnchorPositionTryFallbacks( return undefined } -function assignRef( - ref: React.MutableRefObject | ((instance: T | null) => void) | null | undefined, - value: T | null, -) { - if (typeof ref === 'function') { - ref(value) - } else if (ref) { - ref.current = value - } -} - AnchoredOverlay.displayName = 'AnchoredOverlay' diff --git a/packages/react/src/ButtonGroup/ButtonGroup.test.tsx b/packages/react/src/ButtonGroup/ButtonGroup.test.tsx index 64d935ed1bd..088864a43cd 100644 --- a/packages/react/src/ButtonGroup/ButtonGroup.test.tsx +++ b/packages/react/src/ButtonGroup/ButtonGroup.test.tsx @@ -1,7 +1,6 @@ import {render, screen} from '@testing-library/react' import ButtonGroup from './ButtonGroup' import {createRef} from 'react' -import {FeatureFlags} from '../FeatureFlags' import {describe, expect, it, vi} from 'vitest' import {implementsClassName} from '../utils/testing' import classes from './ButtonGroup.module.css' @@ -25,32 +24,24 @@ describe('ButtonGroup', () => { }) }) -describe('ButtonGroup forwarded ref (primer_react_merged_forwarded_refs)', () => { - for (const enabled of [true, false]) { - describe(`with the flag ${enabled ? 'enabled' : 'disabled'}`, () => { - it('forwards a ref object to the element', () => { - const ref = createRef() - render( - - - - - , - ) - expect(ref.current).toBeInstanceOf(HTMLDivElement) - }) +describe('ButtonGroup forwarded ref', () => { + it('forwards a ref object to the element', () => { + const ref = createRef() + render( + + + , + ) + expect(ref.current).toBeInstanceOf(HTMLDivElement) + }) - it('calls a callback ref with the element', () => { - const refCallback = vi.fn() - render( - - - - - , - ) - expect(refCallback.mock.calls.some(([el]) => el instanceof HTMLDivElement)).toBe(true) - }) - }) - } + it('calls a callback ref with the element', () => { + const refCallback = vi.fn() + render( + + + , + ) + expect(refCallback.mock.calls.some(([el]) => el instanceof HTMLDivElement)).toBe(true) + }) }) diff --git a/packages/react/src/ButtonGroup/ButtonGroup.tsx b/packages/react/src/ButtonGroup/ButtonGroup.tsx index ecba2d33604..c2277f159b7 100644 --- a/packages/react/src/ButtonGroup/ButtonGroup.tsx +++ b/packages/react/src/ButtonGroup/ButtonGroup.tsx @@ -2,8 +2,7 @@ import React, {useRef, type PropsWithChildren} from 'react' import classes from './ButtonGroup.module.css' import {clsx} from 'clsx' import {FocusKeys, useFocusZone} from '../hooks/useFocusZone' -import {useMergedRefs, useProvidedRefOrCreate} from '../hooks' -import {useFeatureFlag} from '../FeatureFlags' +import {useMergedRefs} from '../hooks' import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' export type ButtonGroupProps = PropsWithChildren<{ @@ -17,14 +16,8 @@ const ButtonGroup = React.forwardRef(function ButtonGroup( {as: BaseComponent = 'div', children, className, role, ...rest}, forwardRef, ) { - const mergedRefEnabled = useFeatureFlag('primer_react_merged_forwarded_refs') const buttonRef = useRef(null) const mergedRef = useMergedRefs(buttonRef, forwardRef) - // Feature-flag scaffolding for `primer_react_merged_forwarded_refs`. - // At graduation: remove the three declarations below, and replace all instances of `readRef` with `buttonRef` and `appliedRef` with `mergedRef`. - const providedOrCreatedRef = useProvidedRefOrCreate(forwardRef as React.RefObject) - const readRef = mergedRefEnabled ? buttonRef : providedOrCreatedRef - const appliedRef = mergedRefEnabled ? mergedRef : providedOrCreatedRef const buttons = React.Children.map(children, (child, index) => (
{child} @@ -32,7 +25,7 @@ const ButtonGroup = React.forwardRef(function ButtonGroup( )) useFocusZone({ - containerRef: readRef, + containerRef: buttonRef, disabled: role !== 'toolbar', bindKeys: FocusKeys.ArrowHorizontal, focusOutBehavior: 'wrap', @@ -40,8 +33,7 @@ const ButtonGroup = React.forwardRef(function ButtonGroup( return ( { }) }) -describe('Checkbox forwarded ref (primer_react_merged_forwarded_refs)', () => { - for (const enabled of [true, false]) { - describe(`with the flag ${enabled ? 'enabled' : 'disabled'}`, () => { - it('forwards a ref object to the element', () => { - const ref = createRef() - render( - - - , - ) - expect(ref.current).toBeInstanceOf(HTMLInputElement) - }) - - it('calls a callback ref with the element', () => { - const refCallback = vi.fn() - render( - - - , - ) - expect(refCallback.mock.calls.some(([el]) => el instanceof HTMLInputElement)).toBe(true) - }) - }) - } +describe('Checkbox forwarded ref', () => { + it('forwards a ref object to the element', () => { + const ref = createRef() + render() + expect(ref.current).toBeInstanceOf(HTMLInputElement) + }) + + it('calls a callback ref with the element', () => { + const refCallback = vi.fn() + render() + expect(refCallback.mock.calls.some(([el]) => el instanceof HTMLInputElement)).toBe(true) + }) }) diff --git a/packages/react/src/Checkbox/Checkbox.tsx b/packages/react/src/Checkbox/Checkbox.tsx index 047a0660c02..cc4edf9e3d2 100644 --- a/packages/react/src/Checkbox/Checkbox.tsx +++ b/packages/react/src/Checkbox/Checkbox.tsx @@ -1,6 +1,5 @@ import {clsx} from 'clsx' -import {useMergedRefs, useProvidedRefOrCreate} from '../hooks' -import {useFeatureFlag} from '../FeatureFlags' +import {useMergedRefs} from '../hooks' import React, { useContext, useEffect, @@ -66,28 +65,22 @@ const Checkbox = React.forwardRef( ref, // eslint-disable-next-line @typescript-eslint/no-explicit-any ): ReactElement => { - const mergedRefEnabled = useFeatureFlag('primer_react_merged_forwarded_refs') const checkboxRef = useRef(null) const mergedRef = useMergedRefs(checkboxRef, ref) - // Feature-flag scaffolding for `primer_react_merged_forwarded_refs`. - // At graduation: remove the three declarations below, and replace all instances of `readRef` with `checkboxRef` and `appliedRef` with `mergedRef`. - const providedOrCreatedRef = useProvidedRefOrCreate(ref as React.RefObject) - const readRef = mergedRefEnabled ? checkboxRef : providedOrCreatedRef - const appliedRef = mergedRefEnabled ? mergedRef : providedOrCreatedRef const checkboxGroupContext = useContext(CheckboxGroupContext) const handleOnChange: ChangeEventHandler = e => { checkboxGroupContext.onChange && checkboxGroupContext.onChange(e) onChange && onChange(e) - if (indeterminate && readRef.current) { - readRef.current.indeterminate = true - readRef.current.setAttribute('aria-checked', 'mixed') + if (indeterminate && checkboxRef.current) { + checkboxRef.current.indeterminate = true + checkboxRef.current.setAttribute('aria-checked', 'mixed') } } const inputProps = { type: 'checkbox', disabled, - ref: appliedRef, + ref: mergedRef, checked: indeterminate ? false : checked, defaultChecked, required, @@ -100,13 +93,13 @@ const Checkbox = React.forwardRef( } useLayoutEffect(() => { - if (readRef.current) { - readRef.current.indeterminate = indeterminate || false + if (checkboxRef.current) { + checkboxRef.current.indeterminate = indeterminate || false } - }, [indeterminate, checked, readRef]) + }, [indeterminate, checked]) useEffect(() => { - const {current: checkbox} = readRef + const {current: checkbox} = checkboxRef if (!checkbox) { return } @@ -118,7 +111,6 @@ const Checkbox = React.forwardRef( } }) return ( - // @ts-expect-error inputProp needs a non nullable ref { }) }) -describe('Dialog auto-focus button forwarded ref (primer_react_merged_forwarded_refs)', () => { - for (const enabled of [true, false]) { - it(`focuses the auto-focus footer button via the gated ref with the flag ${enabled ? 'enabled' : 'disabled'}`, async () => { - const {getByRole} = render( - - {}} - footerButtons={[{buttonType: 'primary', content: 'Footer button', autoFocus: true}]} - > - Body - - , - ) - await waitFor(() => expect(getByRole('button', {name: 'Footer button'})).toHaveFocus()) - }) - } +it('focuses the auto-focus footer button', async () => { + const {getByRole} = render( + {}} footerButtons={[{buttonType: 'primary', content: 'Footer button', autoFocus: true}]}> + Body + , + ) + await waitFor(() => expect(getByRole('button', {name: 'Footer button'})).toHaveFocus()) }) diff --git a/packages/react/src/Dialog/Dialog.tsx b/packages/react/src/Dialog/Dialog.tsx index ef7b8d888d4..908092137ea 100644 --- a/packages/react/src/Dialog/Dialog.tsx +++ b/packages/react/src/Dialog/Dialog.tsx @@ -1,8 +1,7 @@ import React, {useCallback, useEffect, useRef, useState, type CSSProperties, type SyntheticEvent} from 'react' import type {ButtonProps} from '../Button' import {Button, IconButton} from '../Button' -import {useMergedRefs, useOnEscapePress, useProvidedRefOrCreate} from '../hooks' -import {useFeatureFlag} from '../FeatureFlags' +import {useMergedRefs, useOnEscapePress} from '../hooks' import {useFocusTrap} from '../hooks/useFocusTrap' import {XIcon} from '@primer/octicons-react' import {useFocusZone} from '../hooks/useFocusZone' @@ -488,26 +487,20 @@ const Footer = React.forwardRef(function Foot Footer.displayName = 'Dialog.Footer' const Buttons: React.FC> = ({buttons}) => { - const mergedRefEnabled = useFeatureFlag('primer_react_merged_forwarded_refs') const providedButtonRef = buttons.find(button => button.autoFocus)?.ref const autoFocusRef = useRef(null) const mergedRef = useMergedRefs(autoFocusRef, providedButtonRef) - // Feature-flag scaffolding for `primer_react_merged_forwarded_refs`. - // At graduation: remove the three declarations below, and replace all instances of `readRef` with `autoFocusRef` and `appliedRef` with `mergedRef`. - const providedOrCreatedRef = useProvidedRefOrCreate(providedButtonRef) - const readRef = mergedRefEnabled ? autoFocusRef : providedOrCreatedRef - const appliedRef = mergedRefEnabled ? mergedRef : providedOrCreatedRef let autoFocusCount = 0 const [hasRendered, setHasRendered] = useState(0) useEffect(() => { // hack to work around dialogs originating from other focus traps. if (hasRendered === 1) { - readRef.current?.focus() + autoFocusRef.current?.focus() } else { // eslint-disable-next-line react-hooks/set-state-in-effect, react-you-might-not-need-an-effect/no-derived-state setHasRendered(hasRendered + 1) } - }, [readRef, hasRendered]) + }, [hasRendered]) return ( <> @@ -520,8 +513,7 @@ const Buttons: React.FC> {...buttonProps} // 'normal' value is equivalent to 'default', this is used for backwards compatibility variant={buttonType === 'normal' ? 'default' : buttonType} - // @ts-expect-error it needs a non nullable ref - ref={autoFocus && autoFocusCount === 0 ? (autoFocusCount++, appliedRef) : null} + ref={autoFocus && autoFocusCount === 0 ? (autoFocusCount++, mergedRef) : null} > {content} diff --git a/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts b/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts index 2663906ca1a..0596463923b 100644 --- a/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts +++ b/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts @@ -8,6 +8,5 @@ export const DefaultFeatureFlags = FeatureFlagScope.create({ primer_react_action_list_group_heading_trailing_action: false, primer_react_action_list_item_gap: false, primer_react_timeline_list_semantics: false, - primer_react_merged_forwarded_refs: false, primer_react_button_loading_disabled_styles: false, }) diff --git a/packages/react/src/FilteredActionList/FilteredActionList.test.tsx b/packages/react/src/FilteredActionList/FilteredActionList.test.tsx index 7d46fc52cb6..affa2f01999 100644 --- a/packages/react/src/FilteredActionList/FilteredActionList.test.tsx +++ b/packages/react/src/FilteredActionList/FilteredActionList.test.tsx @@ -2,7 +2,6 @@ import {fireEvent, render} from '@testing-library/react' import {describe, expect, it, vi} from 'vitest' import React from 'react' import {FilteredActionList} from '../FilteredActionList' -import {FeatureFlags} from '../FeatureFlags' import {FilteredActionListBodyLoader, FilteredActionListLoadingTypes} from './FilteredActionListLoaders' import {implementsClassName} from '../utils/testing' import classes from './FilteredActionList.module.css' @@ -131,38 +130,22 @@ describe('FilteredActionListBodyLoader', () => { }) }) -describe('FilteredActionList forwarded refs (primer_react_merged_forwarded_refs)', () => { - for (const enabled of [true, false]) { - describe(`with the flag ${enabled ? 'enabled' : 'disabled'}`, () => { - it('forwards the input ref object', () => { - const ref = React.createRef() - render( - - - , - ) - expect(ref.current).toBeInstanceOf(HTMLInputElement) - }) - - it('forwards the scroll container ref object', () => { - const ref = React.createRef() - render( - - - , - ) - expect(ref.current).toBeInstanceOf(HTMLDivElement) - }) - - it('calls a scroll container callback ref', () => { - const refCallback = vi.fn() - render( - - - , - ) - expect(refCallback.mock.calls.some(([el]) => el instanceof HTMLDivElement)).toBe(true) - }) - }) - } +describe('FilteredActionList forwarded refs', () => { + it('forwards the input ref object', () => { + const ref = React.createRef() + render() + expect(ref.current).toBeInstanceOf(HTMLInputElement) + }) + + it('forwards the scroll container ref object', () => { + const ref = React.createRef() + render() + expect(ref.current).toBeInstanceOf(HTMLDivElement) + }) + + it('calls a scroll container callback ref', () => { + const refCallback = vi.fn() + render() + expect(refCallback.mock.calls.some(([el]) => el instanceof HTMLDivElement)).toBe(true) + }) }) diff --git a/packages/react/src/FilteredActionList/FilteredActionList.tsx b/packages/react/src/FilteredActionList/FilteredActionList.tsx index f946fe8200a..402cab53b85 100644 --- a/packages/react/src/FilteredActionList/FilteredActionList.tsx +++ b/packages/react/src/FilteredActionList/FilteredActionList.tsx @@ -20,8 +20,7 @@ import {isValidElementType} from 'react-is' import {useAnnouncements} from './useAnnouncements' import {clsx} from 'clsx' import {useVirtualizer} from '@tanstack/react-virtual' -import {useMergedRefs, useProvidedRefOrCreate} from '../hooks' -import {useFeatureFlag} from '../FeatureFlags' +import {useMergedRefs} from '../hooks' import {FilteredActionListInput} from './FilteredActionListInput' const menuScrollMargins: ScrollIntoViewOptions = {startMargin: 0, endMargin: 8} @@ -190,25 +189,11 @@ export function FilteredActionList({ const inputAndListContainerRef = useRef(null) const listRef = useRef(null) - const mergedRefEnabled = useFeatureFlag('primer_react_merged_forwarded_refs') - const scrollContainerRef = useRef(null) const mergedScrollContainerRef = useMergedRefs(scrollContainerRef, providedScrollContainerRef) - // Feature-flag scaffolding for `primer_react_merged_forwarded_refs`. - // At graduation: remove the three declarations below, and replace all instances of `readScrollContainerRef` with `scrollContainerRef` and `appliedScrollContainerRef` with `mergedScrollContainerRef`. - const providedOrCreatedScrollContainerRef = useProvidedRefOrCreate( - providedScrollContainerRef as React.RefObject, - ) - const readScrollContainerRef = mergedRefEnabled ? scrollContainerRef : providedOrCreatedScrollContainerRef - const appliedScrollContainerRef = mergedRefEnabled ? mergedScrollContainerRef : providedOrCreatedScrollContainerRef const inputRef = useRef(null) const mergedInputRef = useMergedRefs(inputRef, providedInputRef) - // Feature-flag scaffolding for `primer_react_merged_forwarded_refs`. - // At graduation: remove the three declarations below, and replace all instances of `readInputRef` with `inputRef` and `appliedInputRef` with `mergedInputRef`. - const providedOrCreatedInputRef = useProvidedRefOrCreate(providedInputRef) - const readInputRef = mergedRefEnabled ? inputRef : providedOrCreatedInputRef - const appliedInputRef = mergedRefEnabled ? mergedInputRef : providedOrCreatedInputRef const usingRovingTabindex = _PrivateFocusManagement === 'roving-tabindex' const [listContainerElement, setListContainerElement] = useState(null) @@ -299,8 +284,8 @@ export function FilteredActionList({ ) useEffect(() => { // eslint-disable-next-line react-you-might-not-need-an-effect/no-pass-data-to-parent - onInputRefChanged?.(readInputRef) - }, [readInputRef, onInputRefChanged]) + onInputRefChanged?.(inputRef) + }, [onInputRefChanged]) // Matches the most common ActionList.Item height (single-line text + description). // Items are measured dynamically via `measureElement`, so this only affects the @@ -310,7 +295,7 @@ export function FilteredActionList({ // eslint-disable-next-line react-hooks/incompatible-library const virtualizer = useVirtualizer({ count: items.length, - getScrollElement: () => readScrollContainerRef.current, + getScrollElement: () => scrollContainerRef.current, estimateSize: () => DEFAULT_VIRTUAL_ITEM_HEIGHT, overscan: 10, enabled: isVirtualized, @@ -348,7 +333,7 @@ export function FilteredActionList({ focusableElementFilter: element => { return !(element instanceof HTMLInputElement) }, - activeDescendantFocus: readInputRef, + activeDescendantFocus: inputRef, onActiveDescendantChanged: (current, previous, directlyActivated) => { activeDescendantRef.current = current @@ -360,8 +345,8 @@ export function FilteredActionList({ } } - if (current && readScrollContainerRef.current && (directlyActivated || focusPrependedElements)) { - scrollIntoView(current, readScrollContainerRef.current, { + if (current && scrollContainerRef.current && (directlyActivated || focusPrependedElements)) { + scrollIntoView(current, scrollContainerRef.current, { ...menuScrollMargins, behavior: scrollBehavior, }) @@ -378,13 +363,13 @@ export function FilteredActionList({ ) useEffect(() => { - if (activeDescendantRef.current && readScrollContainerRef.current) { - scrollIntoView(activeDescendantRef.current, readScrollContainerRef.current, { + if (activeDescendantRef.current && scrollContainerRef.current) { + scrollIntoView(activeDescendantRef.current, scrollContainerRef.current, { ...menuScrollMargins, behavior: scrollBehavior, }) } - }, [items, readInputRef, readScrollContainerRef, scrollBehavior]) + }, [items, scrollBehavior]) useEffect(() => { // eslint-disable-next-line react-you-might-not-need-an-effect/no-event-handler @@ -396,8 +381,8 @@ export function FilteredActionList({ // Listen for focus changes within the container const handleFocusIn = (event: FocusEvent) => { - if (event.target === readInputRef.current || list.contains(event.target as Node)) { - setIsInputFocused(readInputRef.current && readInputRef.current === document.activeElement ? true : false) + if (event.target === inputRef.current || list.contains(event.target as Node)) { + setIsInputFocused(inputRef.current && inputRef.current === document.activeElement ? true : false) } } @@ -407,27 +392,27 @@ export function FilteredActionList({ inputAndListContainerElement.removeEventListener('focusin', handleFocusIn) } } - }, [items, readInputRef, listContainerElement, usingRovingTabindex]) // Re-run when items change to update active indicators + }, [items, listContainerElement, usingRovingTabindex]) // Re-run when items change to update active indicators useEffect(() => { // eslint-disable-next-line react-you-might-not-need-an-effect/no-event-handler if (usingRovingTabindex && !loading) { // eslint-disable-next-line react-you-might-not-need-an-effect/no-adjust-state-on-prop-change - setIsInputFocused(readInputRef.current && readInputRef.current === document.activeElement ? true : false) + setIsInputFocused(inputRef.current && inputRef.current === document.activeElement ? true : false) } - }, [loading, readInputRef, usingRovingTabindex]) + }, [loading, usingRovingTabindex]) const onInputFocus = useAnnouncements( items, listRef, - readInputRef, + inputRef, announcementsEnabled, loading, messageText, _PrivateFocusManagement, filterValue, ) - useScrollFlash(readScrollContainerRef) + useScrollFlash(scrollContainerRef) const handleSelectAllChange = useCallback( (e: React.ChangeEvent) => { @@ -439,10 +424,8 @@ export function FilteredActionList({ ) function getBodyContent() { - if (loading && readScrollContainerRef.current && loadingType.appearsInBody) { - return ( - - ) + if (loading && scrollContainerRef.current && loadingType.appearsInBody) { + return } if (message) { return message @@ -579,7 +562,7 @@ export function FilteredActionList({ data-component="FilteredActionList" >
)} - {/* @ts-expect-error div needs a non nullable ref */} -
+
{getBodyContent()}
diff --git a/packages/react/src/PageHeader/PageHeader.test.tsx b/packages/react/src/PageHeader/PageHeader.test.tsx index 63621ae4208..ed36c943711 100644 --- a/packages/react/src/PageHeader/PageHeader.test.tsx +++ b/packages/react/src/PageHeader/PageHeader.test.tsx @@ -2,7 +2,6 @@ import {describe, expect, it, vi} from 'vitest' import React from 'react' import {render} from '@testing-library/react' import {PageHeader} from '.' -import {FeatureFlags} from '../FeatureFlags' import {implementsClassName} from '../utils/testing' import classes from './PageHeader.module.css' @@ -308,44 +307,34 @@ describe('PageHeader', () => { }) }) -describe('PageHeader forwarded ref (primer_react_merged_forwarded_refs)', () => { - for (const enabled of [true, false]) { - describe(`with the flag ${enabled ? 'enabled' : 'disabled'}`, () => { - it('forwards the Root ref object', () => { - const ref = React.createRef() - render( - - - Content - - , - ) - expect(ref.current).toBeInstanceOf(HTMLDivElement) - }) +describe('PageHeader forwarded ref', () => { + it('forwards the Root ref object', () => { + const ref = React.createRef() + render( + + Content + , + ) + expect(ref.current).toBeInstanceOf(HTMLDivElement) + }) - it('calls a Root callback ref', () => { - const refCallback = vi.fn() - render( - - - Content - - , - ) - expect(refCallback.mock.calls.some(([el]) => el instanceof HTMLDivElement)).toBe(true) - }) + it('calls a Root callback ref', () => { + const refCallback = vi.fn() + render( + + Content + , + ) + expect(refCallback.mock.calls.some(([el]) => el instanceof HTMLDivElement)).toBe(true) + }) - it('forwards the TitleArea ref object', () => { - const ref = React.createRef() - render( - - - Heading - - , - ) - expect(ref.current).toBeInstanceOf(HTMLDivElement) - }) - }) - } + it('forwards the TitleArea ref object', () => { + const ref = React.createRef() + render( + + Heading + , + ) + expect(ref.current).toBeInstanceOf(HTMLDivElement) + }) }) diff --git a/packages/react/src/PageHeader/PageHeader.tsx b/packages/react/src/PageHeader/PageHeader.tsx index c334ec38440..680e089170c 100644 --- a/packages/react/src/PageHeader/PageHeader.tsx +++ b/packages/react/src/PageHeader/PageHeader.tsx @@ -10,8 +10,7 @@ import {getResponsiveAttributes} from '../internal/utils/getResponsiveAttributes import type {ForwardRefComponent as PolymorphicForwardRefComponent} from '../utils/polymorphic' import {areAllValuesTheSame, haveRegularAndWideSameValue} from '../utils/getBreakpointDeclarations' import {warning} from '../utils/warning' -import {useMergedRefs, useProvidedRefOrCreate} from '../hooks' -import {useFeatureFlag} from '../FeatureFlags' +import {useMergedRefs} from '../hooks' import type {AriaRole, FCWithSlotMarker} from '../utils/types' import {clsx} from 'clsx' @@ -50,14 +49,8 @@ export type PageHeaderProps = { const Root = React.forwardRef>( ({children, className, as: BaseComponent = 'div', 'aria-label': ariaLabel, role, hasBorder}, forwardedRef) => { - const mergedRefEnabled = useFeatureFlag('primer_react_merged_forwarded_refs') const rootRef = useRef(null) const mergedRef = useMergedRefs(rootRef, forwardedRef) - // Feature-flag scaffolding for `primer_react_merged_forwarded_refs`. - // At graduation: remove the three declarations below, and replace all instances of `readRef` with `rootRef` and `appliedRef` with `mergedRef`. - const providedOrCreatedRef = useProvidedRefOrCreate(forwardedRef as React.RefObject) - const readRef = mergedRefEnabled ? rootRef : providedOrCreatedRef - const appliedRef = mergedRefEnabled ? mergedRef : providedOrCreatedRef // Hoist title size + navigation visibility off children onto the root so // styling can use plain attribute selectors instead of `:has()`. We descend @@ -125,8 +118,8 @@ const Root = React.forwardRef { + if (!rootRef.current || rootRef.current.children.length <= 0) return + const titleArea = Array.from(rootRef.current.children as HTMLCollection).find(child => { return child instanceof HTMLElement && child.getAttribute('data-component') === 'TitleArea' }) @@ -158,12 +151,12 @@ const Root = React.forwardRef>( ({children, className, hidden = false, variant = 'medium'}, forwardedRef) => { - const mergedRefEnabled = useFeatureFlag('primer_react_merged_forwarded_refs') - // Feature-flag scaffolding for `primer_react_merged_forwarded_refs`. - // At graduation: remove the two declarations below, and replace all instances of `appliedRef` with `forwardedRef`. - const providedOrCreatedRef = useProvidedRefOrCreate(forwardedRef as React.RefObject) - const appliedRef = mergedRefEnabled ? forwardedRef : providedOrCreatedRef return (
{ }) }) -describe('TextInput forwarded ref (primer_react_merged_forwarded_refs)', () => { - for (const enabled of [true, false]) { - describe(`with the flag ${enabled ? 'enabled' : 'disabled'}`, () => { - it('forwards a ref object to the element', () => { - const ref = createRef() - render( - - - , - ) - expect(ref.current).toBeInstanceOf(HTMLInputElement) - }) - - it('calls a callback ref with the element', () => { - const refCallback = vi.fn() - render( - - - , - ) - expect(refCallback.mock.calls.some(([el]) => el instanceof HTMLInputElement)).toBe(true) - }) - }) - } +describe('TextInput forwarded ref', () => { + it('forwards a ref object to the element', () => { + const ref = createRef() + render() + expect(ref.current).toBeInstanceOf(HTMLInputElement) + }) + + it('calls a callback ref with the element', () => { + const refCallback = vi.fn() + render() + expect(refCallback.mock.calls.some(([el]) => el instanceof HTMLInputElement)).toBe(true) + }) }) diff --git a/packages/react/src/TextInput/TextInput.tsx b/packages/react/src/TextInput/TextInput.tsx index de2c0e12af9..bd961de1f5e 100644 --- a/packages/react/src/TextInput/TextInput.tsx +++ b/packages/react/src/TextInput/TextInput.tsx @@ -17,8 +17,7 @@ import visuallyHiddenClasses from '../_VisuallyHidden.module.css' import {getCharacterCountState, SCREEN_READER_DELAY} from '../utils/character-counter' import {AriaStatus} from '../live-region' import Text from '../Text' -import {useMergedRefs, useProvidedRefOrCreate} from '../hooks' -import {useFeatureFlag} from '../FeatureFlags' +import {useMergedRefs} from '../hooks' export type TextInputNonPassthroughProps = { /** @deprecated Use `leadingVisual` or `trailingVisual` prop instead */ @@ -113,14 +112,8 @@ const TextInput = React.forwardRef( ref, ) => { const [isInputFocused, setIsInputFocused] = useState(false) - const mergedRefEnabled = useFeatureFlag('primer_react_merged_forwarded_refs') const inputRef = useRef(null) const mergedRef = useMergedRefs(inputRef, ref) - // Feature-flag scaffolding for `primer_react_merged_forwarded_refs`. - // At graduation: remove the three declarations below, and replace all instances of `readRef` with `inputRef` and `appliedRef` with `mergedRef`. - const providedOrCreatedRef = useProvidedRefOrCreate(ref as React.RefObject) - const readRef = mergedRefEnabled ? inputRef : providedOrCreatedRef - const appliedRef = mergedRefEnabled ? mergedRef : providedOrCreatedRef // For uncontrolled usage we track the length of the input's content so the // character counter can be derived during render rather than synced from an @@ -149,8 +142,8 @@ const TextInput = React.forwardRef( const focusInput: MouseEventHandler = e => { // Don't call focus() if the input itself was clicked on date/time inputs. - if (e.target !== readRef.current || !isSegmentedInputType) { - readRef.current?.focus() + if (e.target !== inputRef.current || !isSegmentedInputType) { + inputRef.current?.focus() } } const leadingVisualId = useId() @@ -227,8 +220,7 @@ const TextInput = React.forwardRef( {typeof LeadingVisual !== 'string' && isValidElementType(LeadingVisual) ? : LeadingVisual} 18 ? elementChild?.props.ref : (elementChild as {ref?: React.Ref} | undefined)?.ref - const mergedRefEnabled = useFeatureFlag('primer_react_merged_forwarded_refs') const triggerRef = useRef(null) const mergedTriggerRef = useMergedRefs(triggerRef, useMergedRefs(forwardedRef, childRef)) - // Feature-flag scaffolding for `primer_react_merged_forwarded_refs`. - // At graduation: remove the three declarations below, and replace all instances of `readTriggerRef` with `triggerRef` and `appliedTriggerRef` with `mergedTriggerRef`. - const providedOrCreatedRef = useProvidedRefOrCreate((forwardedRef ?? childRef) as React.RefObject) - const readTriggerRef = mergedRefEnabled ? triggerRef : providedOrCreatedRef - const appliedTriggerRef = mergedRefEnabled ? mergedTriggerRef : providedOrCreatedRef const tooltipElRef = useRef(null) const [calculatedDirection, setCalculatedDirection] = useState(direction) @@ -158,13 +151,13 @@ export const Tooltip: ForwardRefExoticComponent< try { if ( tooltipElRef.current && - readTriggerRef.current instanceof HTMLElement && + triggerRef.current instanceof HTMLElement && tooltipElRef.current.hasAttribute('popover') && !tooltipElRef.current.matches(':popover-open') && !_privateDisableTooltip ) { const tooltip = tooltipElRef.current - const trigger = readTriggerRef.current + const trigger = triggerRef.current tooltip.showPopover() setIsPopoverOpen(true) /* @@ -205,7 +198,7 @@ export const Tooltip: ForwardRefExoticComponent< try { if ( tooltipElRef.current && - readTriggerRef.current && + triggerRef.current && tooltipElRef.current.hasAttribute('popover') && tooltipElRef.current.matches(':popover-open') ) { @@ -235,8 +228,8 @@ export const Tooltip: ForwardRefExoticComponent< const value = useMemo(() => ({tooltipId}), [tooltipId]) useEffect(() => { - if (!tooltipElRef.current || !readTriggerRef.current) return - const trigger = readTriggerRef.current + if (!tooltipElRef.current || !triggerRef.current) return + const trigger = triggerRef.current const isInvalidTrigger = !(trigger instanceof HTMLElement) warning( isInvalidTrigger, @@ -284,7 +277,7 @@ export const Tooltip: ForwardRefExoticComponent< const tooltip = tooltipElRef.current tooltip.setAttribute('popover', 'auto') - }, [tooltipElRef, readTriggerRef, direction, type]) + }, [tooltipElRef, direction, type]) useOnEscapePress( (event: KeyboardEvent) => { @@ -310,8 +303,7 @@ export const Tooltip: ForwardRefExoticComponent< <> {React.isValidElement(child) && React.cloneElement(child as React.ReactElement, { - // @ts-expect-error the provided-or-created ref path needs a non nullable ref - ref: appliedTriggerRef, + ref: mergedTriggerRef, // If it is a type description, we use tooltip to describe the trigger 'aria-describedby': (() => { // If tooltip is not a description type, keep the original aria-describedby diff --git a/packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx b/packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx index 46ca86f8d88..e7c41ef6555 100644 --- a/packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx +++ b/packages/react/src/TooltipV2/__tests__/Tooltip.test.tsx @@ -13,7 +13,6 @@ import classes from '../Tooltip.module.css' import type {JSX} from 'react' import {createRef, forwardRef, useImperativeHandle} from 'react' -import {FeatureFlags} from '../../FeatureFlags' import {implementsClassName, withExpectedConsoleError, withExpectedConsoleWarning} from '../../utils/testing' const TooltipComponent = (props: Omit & {text?: string}) => ( @@ -253,80 +252,66 @@ describe('Tooltip data-component attributes', () => { }) }) -describe('Tooltip forwarded ref (primer_react_merged_forwarded_refs)', () => { - for (const enabled of [true, false]) { - describe(`with the flag ${enabled ? 'enabled' : 'disabled'}`, () => { - it('forwards a ref object to the trigger element', () => { - const ref = createRef() - HTMLRender( - - - - - , - ) - expect(ref.current).toBeInstanceOf(HTMLButtonElement) - expect(ref.current).toHaveTextContent('Button Text') - }) +describe('Tooltip forwarded ref', () => { + it('forwards a ref object to the trigger element', () => { + const ref = createRef() + HTMLRender( + + + , + ) + expect(ref.current).toBeInstanceOf(HTMLButtonElement) + expect(ref.current).toHaveTextContent('Button Text') + }) - it('calls a callback ref with the trigger element', () => { - const refCallback = vi.fn() - const {getByRole} = HTMLRender( - - - - - , - ) - const button = getByRole('button') - expect(refCallback).toHaveBeenCalledWith(button) - }) + it('calls a callback ref with the trigger element', () => { + const refCallback = vi.fn() + const {getByRole} = HTMLRender( + + + , + ) + const button = getByRole('button') + expect(refCallback).toHaveBeenCalledWith(button) + }) - it("preserves trigger element's own ref", () => { - const buttonRefCallback = vi.fn() + it("preserves trigger element's own ref", () => { + const buttonRefCallback = vi.fn() - const {getByRole} = HTMLRender( - - - - - , - ) + const {getByRole} = HTMLRender( + + + , + ) - const button = getByRole('button') - expect(buttonRefCallback).toHaveBeenCalledWith(button) - }) + const button = getByRole('button') + expect(buttonRefCallback).toHaveBeenCalledWith(button) + }) - it.skipIf(!enabled)("merges trigger's own ref with tooltip ref", () => { - const buttonRefCallback = vi.fn() - const tooltipRefCallback = vi.fn() + it("merges trigger's own ref with tooltip ref", () => { + const buttonRefCallback = vi.fn() + const tooltipRefCallback = vi.fn() - const {getByRole} = HTMLRender( - - - - - , - ) + const {getByRole} = HTMLRender( + + + , + ) - const button = getByRole('button') - expect(buttonRefCallback).toHaveBeenCalledWith(button) - expect(tooltipRefCallback).toHaveBeenCalledWith(button) - }) + const button = getByRole('button') + expect(buttonRefCallback).toHaveBeenCalledWith(button) + expect(tooltipRefCallback).toHaveBeenCalledWith(button) + }) - it('warns without crashing when a custom trigger ref does not resolve to an HTML element', () => { - withExpectedConsoleWarning(() => { - const {getByRole} = HTMLRender( - - - Button Text - - , - ) + it('warns without crashing when a custom trigger ref does not resolve to an HTML element', () => { + withExpectedConsoleWarning(() => { + const {getByRole} = HTMLRender( + + Button Text + , + ) - expect(() => fireEvent.focus(getByRole('button', {name: 'Button Text'}))).not.toThrow() - }) - }) + expect(() => fireEvent.focus(getByRole('button', {name: 'Button Text'}))).not.toThrow() }) - } + }) }) diff --git a/packages/react/src/experimental/Tabs/Tabs.test.tsx b/packages/react/src/experimental/Tabs/Tabs.test.tsx index ca39bad2b4c..7b30d867409 100644 --- a/packages/react/src/experimental/Tabs/Tabs.test.tsx +++ b/packages/react/src/experimental/Tabs/Tabs.test.tsx @@ -4,7 +4,6 @@ import React from 'react' import {describe, test, expect, vi} from 'vitest' import {Tabs, TabList, Tab, TabPanel} from './Tabs' import {useTabList} from './useTabList' -import {FeatureFlags} from '../../FeatureFlags' import {implementsClassName} from '../../utils/testing' describe('Tabs', () => { @@ -720,28 +719,16 @@ function TabListRefHarness({tabRef}: {tabRef: React.Ref}) ) } -describe('useTabList forwarded ref (primer_react_merged_forwarded_refs)', () => { - for (const enabled of [true, false]) { - describe(`with the flag ${enabled ? 'enabled' : 'disabled'}`, () => { - test('forwards a ref object to the tablist', () => { - const ref = React.createRef() - render( - - - , - ) - expect(ref.current).toBeInstanceOf(HTMLDivElement) - }) +describe('useTabList forwarded ref', () => { + test('forwards a ref object to the tablist', () => { + const ref = React.createRef() + render() + expect(ref.current).toBeInstanceOf(HTMLDivElement) + }) - test('calls a callback ref with the tablist', () => { - const refCallback = vi.fn() - render( - - - , - ) - expect(refCallback.mock.calls.some(([el]) => el instanceof HTMLDivElement)).toBe(true) - }) - }) - } + test('calls a callback ref with the tablist', () => { + const refCallback = vi.fn() + render() + expect(refCallback.mock.calls.some(([el]) => el instanceof HTMLDivElement)).toBe(true) + }) }) diff --git a/packages/react/src/experimental/Tabs/useTabList.ts b/packages/react/src/experimental/Tabs/useTabList.ts index ebc36385d8e..1ee7fe1785b 100644 --- a/packages/react/src/experimental/Tabs/useTabList.ts +++ b/packages/react/src/experimental/Tabs/useTabList.ts @@ -1,23 +1,16 @@ import type React from 'react' import {useRef} from 'react' -import {useMergedRefs, useProvidedRefOrCreate} from '../../hooks' -import {useFeatureFlag} from '../../FeatureFlags' +import {useMergedRefs} from '../../hooks' import type {TabListHookProps, TabListHookResult} from './types' export function useTabList(props: TabListHookProps): TabListHookResult { const {'aria-label': ariaLabel, 'aria-labelledby': ariaLabelledby, 'aria-orientation': ariaOrientation} = props - const mergedRefEnabled = useFeatureFlag('primer_react_merged_forwarded_refs') const tabListRef = useRef(null) const mergedRef = useMergedRefs(tabListRef, props.ref) - // Feature-flag scaffolding for `primer_react_merged_forwarded_refs`. - // At graduation: remove the three declarations below, and replace all instances of `readRef` with `tabListRef` and `appliedRef` with `mergedRef`. - const providedOrCreatedRef = useProvidedRefOrCreate(props.ref as React.RefObject) - const readRef = mergedRefEnabled ? tabListRef : providedOrCreatedRef - const appliedRef = mergedRefEnabled ? mergedRef : providedOrCreatedRef const onKeyDown = (event: React.KeyboardEvent) => { - const {current: tablist} = readRef + const {current: tablist} = tabListRef if (tablist === null) { return } @@ -73,7 +66,7 @@ export function useTabList(props: TabListHookProps): T return { tabListProps: { - ref: appliedRef, + ref: mergedRef, 'aria-label': ariaLabel, 'aria-labelledby': ariaLabelledby, 'aria-orientation': ariaOrientation ?? 'horizontal', diff --git a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx index 605229ea25a..71d496e07c4 100644 --- a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx +++ b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx @@ -334,16 +334,8 @@ const UnderlinePanels: FCWithSlotMarker = ({ className={clsx(className, classes.StyledUnderlineWrapper)} {...props} > - - {tabs} - + {/* @ts-expect-error ref needs a non nullable ref */} + {tabs} {tabPanels}