From ff7e3be4ff5fe0439931bc87445f91c23b7a7e26 Mon Sep 17 00:00:00 2001 From: atomiks Date: Mon, 20 Jul 2026 09:20:20 +1000 Subject: [PATCH 1/2] [select][tabs] Simplify composite item registries --- packages/react/src/select/item/SelectItem.tsx | 16 +-- .../select/positioner/SelectPositioner.tsx | 114 +++++++++--------- packages/react/src/tabs/panel/TabsPanel.tsx | 17 +-- packages/react/src/tabs/root/TabsRoot.tsx | 46 ++----- .../react/src/tabs/root/TabsRootContext.ts | 1 - 5 files changed, 77 insertions(+), 117 deletions(-) diff --git a/packages/react/src/select/item/SelectItem.tsx b/packages/react/src/select/item/SelectItem.tsx index ff69a678ef0..5f3a2a6eb77 100644 --- a/packages/react/src/select/item/SelectItem.tsx +++ b/packages/react/src/select/item/SelectItem.tsx @@ -42,9 +42,11 @@ export const SelectItem = React.memo( } = componentProps; const textRef = React.useRef(null); + const metadata = React.useMemo(() => ({ value: itemValue }), [itemValue]); const listItem = useCompositeListItem({ guess: true, label, + metadata, textRef, }); @@ -55,7 +57,6 @@ export const SelectItem = React.memo( setValue, selectionRef, typingRef, - valuesRef, multiple, selectedItemTextRef, disabled: selectDisabled, @@ -73,19 +74,6 @@ export const SelectItem = React.memo( const itemRef = React.useRef(null); - useIsoLayoutEffect(() => { - if (!hasRegistered) { - return undefined; - } - - const values = valuesRef.current; - values[index] = itemValue; - - return () => { - delete values[index]; - }; - }, [hasRegistered, index, itemValue, valuesRef]); - useIsoLayoutEffect(() => { if (!hasRegistered) { return; diff --git a/packages/react/src/select/positioner/SelectPositioner.tsx b/packages/react/src/select/positioner/SelectPositioner.tsx index e845cec1409..d2b041b661a 100644 --- a/packages/react/src/select/positioner/SelectPositioner.tsx +++ b/packages/react/src/select/positioner/SelectPositioner.tsx @@ -5,7 +5,10 @@ import { useIsoLayoutEffect } from '@base-ui/utils/useIsoLayoutEffect'; import { useStableCallback } from '@base-ui/utils/useStableCallback'; import { useStore } from '@base-ui/utils/store'; import { useSelectRootContext, useSelectFloatingContext } from '../root/SelectRootContext'; -import { CompositeList } from '../../internals/composite/list/CompositeList'; +import { + CompositeList, + type CompositeMetadata, +} from '../../internals/composite/list/CompositeList'; import type { BaseUIComponentProps } from '../../internals/types'; import { useAnchorPositioning, @@ -151,67 +154,66 @@ export const SelectPositioner = React.forwardRef(function SelectPositioner( inert: !open, }); - const prevMapSizeRef = React.useRef(0); - - const onMapChange = useStableCallback( - (map: Map) => { - if (valuesRef.current.length === 0) { - return; - } + const onMapChange = useStableCallback((map: Map>) => { + const previousValues = valuesRef.current; + const nextValues = Array.from(map.values(), (item) => item.value); + valuesRef.current = nextValues; - const prevSize = prevMapSizeRef.current; - prevMapSizeRef.current = map.size; - - if (map.size === prevSize) { - return; - } - - const eventDetails = createChangeEventDetails(REASONS.none); - - if (prevSize !== 0 && !store.state.multiple && value !== null) { - const selectedValueIndex = findItemIndex(valuesRef.current, value, isItemEqualToValue); - if (selectedValueIndex === -1) { - const initialSelectedValue = initialValueRef.current; - const hasInitial = - initialSelectedValue != null && - findItemIndex(valuesRef.current, initialSelectedValue, isItemEqualToValue) !== -1; - const nextValue = hasInitial ? initialSelectedValue : null; - setValue(nextValue, eventDetails); + if (nextValues.length === previousValues.length) { + return; + } - if (nextValue === null) { - store.set('selectedIndex', null); - selectedItemTextRef.current = null; - } + const eventDetails = createChangeEventDetails(REASONS.none); + + if (previousValues.length !== 0 && !store.state.multiple && value !== null) { + const selectedValueIndex = findItemIndex(nextValues, value, isItemEqualToValue); + if (selectedValueIndex === -1) { + const initialSelectedValue = initialValueRef.current; + const hasInitial = + initialSelectedValue != null && + findItemIndex(nextValues, initialSelectedValue, isItemEqualToValue) !== -1; + const nextValue = hasInitial ? initialSelectedValue : null; + setValue(nextValue, eventDetails); + + if (nextValue === null) { + store.set('selectedIndex', null); + selectedItemTextRef.current = null; } } + } - if (prevSize !== 0 && store.state.multiple && Array.isArray(value)) { - const nextValue = value.filter( - (selectedItemValue) => - findItemIndex(valuesRef.current, selectedItemValue, isItemEqualToValue) !== -1, - ); - if (nextValue.length !== value.length) { - setValue(nextValue, eventDetails); - - if (nextValue.length === 0) { - store.set('selectedIndex', null); - selectedItemTextRef.current = null; - } + if (previousValues.length !== 0 && store.state.multiple && Array.isArray(value)) { + const nextValue = value.filter( + (selectedItemValue) => + findItemIndex(nextValues, selectedItemValue, isItemEqualToValue) !== -1, + ); + if (nextValue.length !== value.length) { + setValue(nextValue, eventDetails); + + if (nextValue.length === 0) { + store.set('selectedIndex', null); + selectedItemTextRef.current = null; } } + } - if (open && alignItemWithTriggerActive) { - store.update({ - scrollUpArrowVisible: false, - scrollDownArrowVisible: false, - }); + if (open && alignItemWithTriggerActive) { + store.update({ + scrollUpArrowVisible: false, + scrollDownArrowVisible: false, + }); - const stylesToClear: React.CSSProperties = { height: '' }; - clearStyles(positionerElement, stylesToClear); - clearStyles(popupRef.current, stylesToClear); - } - }, - ); + const stylesToClear: React.CSSProperties = { height: '' }; + clearStyles(positionerElement, stylesToClear); + clearStyles(popupRef.current, stylesToClear); + } + }); + + useIsoLayoutEffect(() => { + return () => { + valuesRef.current = []; + }; + }, [valuesRef]); const contextValue: SelectPositionerContext = React.useMemo( () => ({ @@ -226,7 +228,11 @@ export const SelectPositioner = React.forwardRef(function SelectPositioner( ); return ( - + + elementsRef={listRef} + labelsRef={labelsRef} + onMapChange={onMapChange} + > {mounted && modal && } {element} diff --git a/packages/react/src/tabs/panel/TabsPanel.tsx b/packages/react/src/tabs/panel/TabsPanel.tsx index f0a1959d16e..53412774595 100644 --- a/packages/react/src/tabs/panel/TabsPanel.tsx +++ b/packages/react/src/tabs/panel/TabsPanel.tsx @@ -1,7 +1,6 @@ 'use client'; import * as React from 'react'; import { inertValue } from '@base-ui/utils/inertValue'; -import { useIsoLayoutEffect } from '@base-ui/utils/useIsoLayoutEffect'; import { useBaseUiId } from '../../internals/useBaseUiId'; import type { StateAttributesMapping } from '../../internals/getStateAttributesProps'; import { transitionStatusMapping } from '../../internals/stateAttributesMapping'; @@ -37,12 +36,12 @@ export const TabsPanel = React.forwardRef(function TabsPanel( getTabIdByPanelValue, orientation, tabActivationDirection, - registerMountedTabPanel, } = useTabsRootContext(); const id = useBaseUiId(); - const { ref: listItemRef, index } = useCompositeListItem(); + const metadata = React.useMemo(() => ({ id, value }), [id, value]); + const { ref: listItemRef, index } = useCompositeListItem({ metadata }); const open = value === selectedValue; const { mounted, transitionStatus, setMounted } = useTransitionStatus(open); @@ -88,18 +87,6 @@ export const TabsPanel = React.forwardRef(function TabsPanel( }, }); - useIsoLayoutEffect(() => { - if (hidden && !keepMounted) { - return undefined; - } - - if (id == null) { - return undefined; - } - - return registerMountedTabPanel(value, id); - }, [hidden, keepMounted, value, id, registerMountedTabPanel]); - const shouldRender = keepMounted || mounted; if (!shouldRender) { return null; diff --git a/packages/react/src/tabs/root/TabsRoot.tsx b/packages/react/src/tabs/root/TabsRoot.tsx index 153181ee0ca..89be1b9e2ac 100644 --- a/packages/react/src/tabs/root/TabsRoot.tsx +++ b/packages/react/src/tabs/root/TabsRoot.tsx @@ -43,8 +43,8 @@ export const TabsRoot = React.forwardRef(function TabsRoot( const hasExplicitDefaultValueProp = componentProps.defaultValue !== undefined; const tabPanelRefs = React.useRef<(HTMLElement | null)[]>([]); - const [mountedTabPanels, setMountedTabPanels] = React.useState( - () => new Map(), + const [tabPanelMap, setTabPanelMap] = React.useState( + () => new Map>(), ); const [value, setValue] = useControlled({ @@ -136,38 +136,18 @@ export const TabsRoot = React.forwardRef(function TabsRoot( }, ); - const registerMountedTabPanel = useStableCallback( - (panelValue: TabsTab.Value | number, panelId: string) => { - setMountedTabPanels((prev) => { - if (prev.get(panelValue) === panelId) { - return prev; - } - - const next = new Map(prev); - next.set(panelValue, panelId); - return next; - }); - - return () => { - setMountedTabPanels((prev) => { - if (prev.get(panelValue) !== panelId) { - return prev; - } - - const next = new Map(prev); - next.delete(panelValue); - return next; - }); - }; - }, - ); - // get the `id` attribute of to set as the value of `aria-controls` on const getTabPanelIdByValue = React.useCallback( (tabValue: TabsTab.Value) => { - return mountedTabPanels.get(tabValue); + let matchingId: string | undefined; + for (const panelMetadata of tabPanelMap.values()) { + if (panelMetadata.value === tabValue && panelMetadata.id != null) { + matchingId = panelMetadata.id; + } + } + return matchingId; }, - [mountedTabPanels], + [tabPanelMap], ); // get the `id` attribute of to set as the value of `aria-labelledby` on @@ -190,7 +170,6 @@ export const TabsRoot = React.forwardRef(function TabsRoot( getTabPanelIdByValue, onValueChange, orientation, - registerMountedTabPanel, setTabMap, tabActivationDirection, value, @@ -201,7 +180,6 @@ export const TabsRoot = React.forwardRef(function TabsRoot( getTabPanelIdByValue, onValueChange, orientation, - registerMountedTabPanel, setTabMap, tabActivationDirection, value, @@ -353,7 +331,9 @@ export const TabsRoot = React.forwardRef(function TabsRoot( return ( - elementsRef={tabPanelRefs}>{element} + elementsRef={tabPanelRefs} onMapChange={setTabPanelMap}> + {element} + ); }); diff --git a/packages/react/src/tabs/root/TabsRootContext.ts b/packages/react/src/tabs/root/TabsRootContext.ts index a156c479699..774cb02efdc 100644 --- a/packages/react/src/tabs/root/TabsRootContext.ts +++ b/packages/react/src/tabs/root/TabsRootContext.ts @@ -29,7 +29,6 @@ export interface TabsRootContext { * Gets the `id` attribute of the TabPanel that corresponds to the given Tab value. */ getTabPanelIdByValue: (tabValue: TabsTab.Value) => string | undefined; - registerMountedTabPanel: (panelValue: TabsTab.Value | number, panelId: string) => () => void; setTabMap: (map: Map>) => void; /** * The position of the active tab relative to the previously active tab. From ee9ef91405723c17bb118246cf6ffc992297ba8f Mon Sep 17 00:00:00 2001 From: atomiks Date: Mon, 20 Jul 2026 23:51:13 +1000 Subject: [PATCH 2/2] [tabs] Preserve latest panel registration --- .../react/src/tabs/panel/TabsPanel.test.tsx | 30 +++++++++++++ packages/react/src/tabs/panel/TabsPanel.tsx | 16 ++++++- packages/react/src/tabs/root/TabsRoot.tsx | 44 +++++++++++++------ .../react/src/tabs/root/TabsRootContext.ts | 1 + 4 files changed, 76 insertions(+), 15 deletions(-) diff --git a/packages/react/src/tabs/panel/TabsPanel.test.tsx b/packages/react/src/tabs/panel/TabsPanel.test.tsx index 87213e7aa50..3f5671c3623 100644 --- a/packages/react/src/tabs/panel/TabsPanel.test.tsx +++ b/packages/react/src/tabs/panel/TabsPanel.test.tsx @@ -25,6 +25,36 @@ describe('', () => { }); describe('panels sharing a value', () => { + it('gives ownership to a panel that starts sharing the value later', async () => { + function App() { + const [firstValue, setFirstValue] = React.useState('a'); + + return ( + + + + + B + + + + + + ); + } + + const { user } = await render(); + const tab = screen.getByRole('tab'); + + expect(tab).toHaveAttribute('aria-controls', screen.getByTestId('second').id); + + await user.click(screen.getByRole('button', { name: 'share value' })); + + expect(tab).toHaveAttribute('aria-controls', screen.getByTestId('first').id); + }); + it('keeps the surviving registration when a shadowed panel unmounts', async () => { function App() { const [shadowedMounted, setShadowedMounted] = React.useState(true); diff --git a/packages/react/src/tabs/panel/TabsPanel.tsx b/packages/react/src/tabs/panel/TabsPanel.tsx index 53412774595..e3ccc1c7b81 100644 --- a/packages/react/src/tabs/panel/TabsPanel.tsx +++ b/packages/react/src/tabs/panel/TabsPanel.tsx @@ -1,6 +1,7 @@ 'use client'; import * as React from 'react'; import { inertValue } from '@base-ui/utils/inertValue'; +import { useIsoLayoutEffect } from '@base-ui/utils/useIsoLayoutEffect'; import { useBaseUiId } from '../../internals/useBaseUiId'; import type { StateAttributesMapping } from '../../internals/getStateAttributesProps'; import { transitionStatusMapping } from '../../internals/stateAttributesMapping'; @@ -36,12 +37,12 @@ export const TabsPanel = React.forwardRef(function TabsPanel( getTabIdByPanelValue, orientation, tabActivationDirection, + registerMountedTabPanel, } = useTabsRootContext(); const id = useBaseUiId(); - const metadata = React.useMemo(() => ({ id, value }), [id, value]); - const { ref: listItemRef, index } = useCompositeListItem({ metadata }); + const { ref: listItemRef, index } = useCompositeListItem(); const open = value === selectedValue; const { mounted, transitionStatus, setMounted } = useTransitionStatus(open); @@ -87,6 +88,17 @@ export const TabsPanel = React.forwardRef(function TabsPanel( }, }); + useIsoLayoutEffect(() => { + // On React 17 `useId` resolves in a passive effect, so `id` is still + // undefined during this layout effect on the first commit. Skip the + // registration until the effect re-runs with the resolved id. + if (id == null || (hidden && !keepMounted)) { + return undefined; + } + + return registerMountedTabPanel(value, id); + }, [hidden, keepMounted, value, id, registerMountedTabPanel]); + const shouldRender = keepMounted || mounted; if (!shouldRender) { return null; diff --git a/packages/react/src/tabs/root/TabsRoot.tsx b/packages/react/src/tabs/root/TabsRoot.tsx index c55bbf0e9be..bde4c6bfe01 100644 --- a/packages/react/src/tabs/root/TabsRoot.tsx +++ b/packages/react/src/tabs/root/TabsRoot.tsx @@ -43,8 +43,8 @@ export const TabsRoot = React.forwardRef(function TabsRoot( const hasExplicitDefaultValueProp = componentProps.defaultValue !== undefined; const tabPanelRefs = React.useRef<(HTMLElement | null)[]>([]); - const [tabPanelMap, setTabPanelMap] = React.useState( - () => new Map>(), + const [mountedTabPanels, setMountedTabPanels] = React.useState( + () => new Map(), ); const [value, setValue] = useControlled({ @@ -135,18 +135,36 @@ export const TabsRoot = React.forwardRef(function TabsRoot( }, ); + const registerMountedTabPanel = useStableCallback( + (panelValue: TabsTab.Value, panelId: string) => { + setMountedTabPanels((prev) => { + const next = new Map(prev); + next.set(panelValue, panelId); + return next; + }); + + return () => { + setMountedTabPanels((prev) => { + // Another panel with the same value took ownership in the meantime; + // leave its registration in place. + if (prev.get(panelValue) !== panelId) { + return prev; + } + + const next = new Map(prev); + next.delete(panelValue); + return next; + }); + }; + }, + ); + // get the `id` attribute of to set as the value of `aria-controls` on const getTabPanelIdByValue = React.useCallback( (tabValue: TabsTab.Value) => { - let matchingId: string | undefined; - for (const panelMetadata of tabPanelMap.values()) { - if (panelMetadata.value === tabValue && panelMetadata.id != null) { - matchingId = panelMetadata.id; - } - } - return matchingId; + return mountedTabPanels.get(tabValue); }, - [tabPanelMap], + [mountedTabPanels], ); // get the `id` attribute of to set as the value of `aria-labelledby` on @@ -169,6 +187,7 @@ export const TabsRoot = React.forwardRef(function TabsRoot( getTabPanelIdByValue, onValueChange, orientation, + registerMountedTabPanel, setTabMap, tabActivationDirection, value, @@ -179,6 +198,7 @@ export const TabsRoot = React.forwardRef(function TabsRoot( getTabPanelIdByValue, onValueChange, orientation, + registerMountedTabPanel, setTabMap, tabActivationDirection, value, @@ -324,9 +344,7 @@ export const TabsRoot = React.forwardRef(function TabsRoot( return ( - elementsRef={tabPanelRefs} onMapChange={setTabPanelMap}> - {element} - + elementsRef={tabPanelRefs}>{element} ); }); diff --git a/packages/react/src/tabs/root/TabsRootContext.ts b/packages/react/src/tabs/root/TabsRootContext.ts index 214cdd00028..0073816e4e6 100644 --- a/packages/react/src/tabs/root/TabsRootContext.ts +++ b/packages/react/src/tabs/root/TabsRootContext.ts @@ -29,6 +29,7 @@ export interface TabsRootContext { * Gets the `id` attribute of the TabPanel that corresponds to the given Tab value. */ getTabPanelIdByValue: (tabValue: TabsTab.Value) => string | undefined; + registerMountedTabPanel: (panelValue: TabsTab.Value, panelId: string) => () => void; setTabMap: (map: Map>) => void; /** * The position of the active tab relative to the previously active tab.