diff --git a/.changeset/graduate-underline-panels-controlled-api.md b/.changeset/graduate-underline-panels-controlled-api.md new file mode 100644 index 00000000000..46559b4179e --- /dev/null +++ b/.changeset/graduate-underline-panels-controlled-api.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +Tabs and UnderlinePanels: Make the controlled selection and manual activation APIs available without a feature flag. Re-selecting the active tab no longer fires the value change callback. diff --git a/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts b/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts index dcf90e27d15..fcf8dcca3ab 100644 --- a/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts +++ b/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts @@ -9,5 +9,4 @@ export const DefaultFeatureFlags = FeatureFlagScope.create({ primer_react_action_list_item_gap: false, primer_react_timeline_list_semantics: false, primer_react_merged_forwarded_refs: false, - primer_react_underline_panels_controlled: false, }) diff --git a/packages/react/src/experimental/Tabs/README.mdx b/packages/react/src/experimental/Tabs/README.mdx index 568eb51217b..2519b401676 100644 --- a/packages/react/src/experimental/Tabs/README.mdx +++ b/packages/react/src/experimental/Tabs/README.mdx @@ -70,8 +70,6 @@ By default, selection follows focus: navigating with the Arrow, Home, and End ke ``` -Manual activation is behind the `primer_react_underline_panels_controlled` feature flag; with the flag off, `activationMode` is always `'automatic'`. - ### Example: `ActionList` diff --git a/packages/react/src/experimental/Tabs/Tabs.test.tsx b/packages/react/src/experimental/Tabs/Tabs.test.tsx index b7292fe54d7..ca39bad2b4c 100644 --- a/packages/react/src/experimental/Tabs/Tabs.test.tsx +++ b/packages/react/src/experimental/Tabs/Tabs.test.tsx @@ -94,16 +94,14 @@ describe('Tabs', () => { const onValueChange = vi.fn() render( - - - - Tab A - Tab B - - Panel A - Panel B - - , + + + Tab A + Tab B + + Panel A + Panel B + , ) await user.click(screen.getByRole('tab', {name: 'Tab A'})) @@ -609,18 +607,16 @@ describe('Tabs', () => { describe('manual activation', () => { const renderManualTabs = (onValueChange?: (args: {value: string}) => void) => render( - - - - Tab A - Tab B - Tab C - - Panel A - Panel B - Panel C - - , + + + Tab A + Tab B + Tab C + + Panel A + Panel B + Panel C + , ) test('arrow keys move focus without changing selection', async () => { diff --git a/packages/react/src/experimental/Tabs/Tabs.tsx b/packages/react/src/experimental/Tabs/Tabs.tsx index 27ec7f6b089..3d440b9a974 100644 --- a/packages/react/src/experimental/Tabs/Tabs.tsx +++ b/packages/react/src/experimental/Tabs/Tabs.tsx @@ -2,7 +2,6 @@ import React, {useId, useMemo, type ElementRef} from 'react' import useIsomorphicLayoutEffect from '../../utils/useIsomorphicLayoutEffect' import {useControllableState} from '../../hooks/useControllableState' import {TabsContext} from './TabsContext' -import {useFeatureFlag} from '../../FeatureFlags' import type {TabListProps, TabPanelProps, TabProps, TabsContextValue, TabsProps} from './types' import {useTab} from './useTab' import {useTabList} from './useTabList' @@ -19,9 +18,7 @@ function Tabs(props: TabsProps) { const {children, onValueChange} = props const generatedId = useId() const groupId = props.id ?? generatedId - // Feature-flag scaffolding: at graduation, drop this and the guards that read it. - const controlledApiEnabled = useFeatureFlag('primer_react_underline_panels_controlled') - const activationMode = controlledApiEnabled ? (props.activationMode ?? 'automatic') : 'automatic' + const activationMode = props.activationMode ?? 'automatic' const [selectedValue, setSelectedValue] = useControllableState({ name: 'tab-selection', @@ -41,7 +38,7 @@ function Tabs(props: TabsProps) { focusedValue, selectTab(value: string) { setFocusedValue(undefined) - if (controlledApiEnabled && value === selectedValue) { + if (value === selectedValue) { return } setSelectedValue(value) @@ -51,7 +48,7 @@ function Tabs(props: TabsProps) { setFocusedValue(value) }, } - }, [groupId, selectedValue, activationMode, focusedValue, controlledApiEnabled, setSelectedValue]) + }, [groupId, selectedValue, activationMode, focusedValue, setSelectedValue]) useIsomorphicLayoutEffect(() => { savedOnValueChange.current = onValueChange diff --git a/packages/react/src/experimental/Tabs/useTabList.ts b/packages/react/src/experimental/Tabs/useTabList.ts index e59ee8b0bf4..ebc36385d8e 100644 --- a/packages/react/src/experimental/Tabs/useTabList.ts +++ b/packages/react/src/experimental/Tabs/useTabList.ts @@ -8,7 +8,6 @@ export function useTabList(props: TabListHookProps): T const {'aria-label': ariaLabel, 'aria-labelledby': ariaLabelledby, 'aria-orientation': ariaOrientation} = props const mergedRefEnabled = useFeatureFlag('primer_react_merged_forwarded_refs') - const controlledApiEnabled = useFeatureFlag('primer_react_underline_panels_controlled') const tabListRef = useRef(null) const mergedRef = useMergedRefs(tabListRef, props.ref) // Feature-flag scaffolding for `primer_react_merged_forwarded_refs`. @@ -26,12 +25,10 @@ export function useTabList(props: TabListHookProps): T const tabs = getFocusableTabs(tablist) const getCurrentIndex = () => { - if (controlledApiEnabled) { - const activeElement = tablist.ownerDocument.activeElement - const focusedIndex = activeElement instanceof HTMLElement ? tabs.indexOf(activeElement) : -1 - if (focusedIndex !== -1) { - return focusedIndex - } + const activeElement = tablist.ownerDocument.activeElement + const focusedIndex = activeElement instanceof HTMLElement ? tabs.indexOf(activeElement) : -1 + if (focusedIndex !== -1) { + return focusedIndex } return tabs.findIndex(tab => { return tab.getAttribute('aria-selected') === 'true' diff --git a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.docs.json b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.docs.json index 631730a9242..26f6330a1e6 100644 --- a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.docs.json +++ b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.docs.json @@ -56,7 +56,7 @@ "name": "value", "type": "string", "defaultValue": "", - "description": "The value of the selected tab, keyed to each `UnderlinePanels.Tab`/`UnderlinePanels.Panel` `value`. Provide this (with `onChange`) for a controlled component where the selected tab is the single source of truth. Requires the `primer_react_underline_panels_controlled` feature flag." + "description": "The value of the selected tab, keyed to each `UnderlinePanels.Tab`/`UnderlinePanels.Panel` `value`. Provide this (with `onChange`) for a controlled component where the selected tab is the single source of truth." }, { "name": "defaultValue", @@ -74,7 +74,7 @@ "name": "activationMode", "type": "'automatic' | 'manual'", "defaultValue": "'automatic'", - "description": "Controls how tabs are activated with the keyboard. `'automatic'` selects on focus (Arrow/Home/End select immediately); `'manual'` moves focus only and commits selection on Enter, Space, or click. Prefer `'manual'` when displaying a panel is not instant (e.g. it triggers a network request). Requires the `primer_react_underline_panels_controlled` feature flag." + "description": "Controls how tabs are activated with the keyboard. `'automatic'` selects on focus (Arrow/Home/End select immediately); `'manual'` moves focus only and commits selection on Enter, Space, or click. Prefer `'manual'` when displaying a panel is not instant (e.g. it triggers a network request)." }, { "name": "children", diff --git a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.features.stories.tsx b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.features.stories.tsx index faf2481ebfc..2c017570d6d 100644 --- a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.features.stories.tsx +++ b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.features.stories.tsx @@ -3,7 +3,6 @@ import {action} from 'storybook/actions' import {useState} from 'react' import {INITIAL_VIEWPORTS} from 'storybook/viewport' import UnderlinePanels from './UnderlinePanels' -import {useFeatureFlag} from '../../FeatureFlags' import {AnchoredOverlay} from '../../AnchoredOverlay' import {Button} from '../../Button' import type {ComponentProps} from '../../utils/types' @@ -123,26 +122,11 @@ export const WithCountersInLoadingState = () => { ) } -// These stories exercise the controlled API, which is gated. Rather than force the flag on (which -// would override the toolbar), surface its state so the toolbar can be used to compare on vs off. -const FlagState = () => { - const enabled = useFeatureFlag('primer_react_underline_panels_controlled') - - return enabled ? null : ( -

- primer_react_underline_panels_controlled is off, so value,{' '} - defaultValue, onChange, and activationMode are ignored and tabs fall back - to positional selection. Toggle the flag in the Storybook toolbar to compare. -

- ) -} - export const Controlled = () => { const [refType, setRefType] = useState('branch') return ( <> - { export const Uncontrolled = () => ( <> - { return ( <> -

With activationMode="manual", arrow keys only move focus; press Enter or Space (or click) to commit selection. Prefer this when switching tabs triggers async work like a fetch. @@ -222,7 +204,6 @@ export const InOverlay = () => { return ( <> - setOpen(true)} diff --git a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.test.tsx b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.test.tsx index 6e4138b90f5..a080adf631d 100644 --- a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.test.tsx +++ b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.test.tsx @@ -11,7 +11,6 @@ import {CodeIcon, EyeIcon} from '@primer/octicons-react' import UnderlinePanels from './UnderlinePanels' import {AnchoredOverlay} from '../../AnchoredOverlay' import {implementsClassName, withExpectedConsoleError, withExpectedConsoleWarning} from '../../utils/testing' -import {FeatureFlags} from '../../FeatureFlags' import classes from './UnderlinePanels.module.css' const UnderlinePanelsMockComponent = (props: {'aria-label'?: string; 'aria-labelledby'?: string; id?: string}) => ( @@ -201,24 +200,18 @@ describe('UnderlinePanels', () => { }) describe('controlled value / onChange / activationMode', () => { - const Flagged = ({children}: {children: React.ReactNode}) => ( - {children} - ) - const RefTabs = (props: { value?: string defaultValue?: string activationMode?: 'automatic' | 'manual' onChange?: ({value}: {value: string}) => void }) => ( - - - Branches - Tags - Branch panel - Tag panel - - + + Branches + Tags + Branch panel + Tag panel + ) it('`value` selects the matching tab and shows its panel', () => { @@ -357,14 +350,12 @@ describe('UnderlinePanels', () => { const user = userEvent.setup() const onChange = vi.fn() render( - - - Tab 1 - Tab 2 - Panel 1 - Panel 2 - - , + + Tab 1 + Tab 2 + Panel 1 + Panel 2 + , ) await act(async () => { @@ -387,20 +378,18 @@ describe('UnderlinePanels', () => { {tab: 'tag', panel: 'tag'}, ] return ( - - - {pairs.map((p, i) => ( - - {p.tab} - - ))} - {pairs.map((p, i) => ( - - {p.panel} panel - - ))} - - + + {pairs.map((p, i) => ( + + {p.tab} + + ))} + {pairs.map((p, i) => ( + + {p.panel} panel + + ))} + ) } @@ -456,52 +445,6 @@ describe('UnderlinePanels', () => { }) }) }) - - describe('with the feature flag disabled', () => { - const UnflaggedRefTabs = (props: { - value?: string - activationMode?: 'automatic' | 'manual' - onChange?: ({value}: {value: string}) => void - }) => ( - - Branches - Tags - Branch panel - Tag panel - - ) - - it('ignores `value` and falls back to positional selection', () => { - render() - - expect(screen.getByRole('tab', {name: 'Branches'})).toHaveAttribute('aria-selected', 'true') - expect(screen.getByRole('tab', {name: 'Tags'})).toHaveAttribute('aria-selected', 'false') - expect(screen.getByText('Branch panel')).toBeVisible() - }) - - it('does not call onChange', async () => { - const user = userEvent.setup() - const onChange = vi.fn() - render() - - await user.click(screen.getByRole('tab', {name: 'Tags'})) - - expect(onChange).not.toHaveBeenCalled() - expect(screen.getByRole('tab', {name: 'Tags'})).toHaveAttribute('aria-selected', 'true') - }) - - it('ignores `activationMode="manual"` and still selects on arrow keys', async () => { - const user = userEvent.setup() - render() - - await act(async () => { - screen.getByRole('tab', {name: 'Branches'}).focus() - await user.keyboard('{ArrowRight}') - }) - - expect(screen.getByRole('tab', {name: 'Tags'})).toHaveAttribute('aria-selected', 'true') - }) - }) }) }) diff --git a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx index 8496ea3f869..605229ea25a 100644 --- a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx +++ b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx @@ -14,7 +14,6 @@ import type {IconProps} from '@primer/octicons-react' import {UnderlineItemList, UnderlineWrapper, UnderlineItem} from '../../internal/components/UnderlineTabbedInterface' import {useId} from '../../hooks' import {invariant} from '../../utils/invariant' -import {useFeatureFlag} from '../../FeatureFlags' import {warning} from '../../utils/warning' import {useResizeObserver, type ResizeObserverEntry} from '../../hooks/useResizeObserver' import useIsomorphicLayoutEffect from '../../utils/useIsomorphicLayoutEffect' @@ -133,18 +132,12 @@ const UnderlinePanels: FCWithSlotMarker = ({ children, loadingCounters, className, - value: valueProp, - defaultValue: defaultValueProp, - onChange: onChangeProp, + value: controlledValue, + defaultValue, + onChange, activationMode = 'automatic', ...props }) => { - // Feature-flag scaffolding: at graduation, drop these three and use the props directly. - const controlledApiEnabled = useFeatureFlag('primer_react_underline_panels_controlled') - const controlledValue = controlledApiEnabled ? valueProp : undefined - const defaultValue = controlledApiEnabled ? defaultValueProp : undefined - const onChange = controlledApiEnabled ? onChangeProp : undefined - const [iconsVisible, setIconsVisible] = useState(true) const wrapperRef = useRef(null) const listRef = useRef(null) @@ -159,13 +152,13 @@ const UnderlinePanels: FCWithSlotMarker = ({ const childrenWithProps = Children.map(children, child => { if (isValidElement(child) && (child.type === Tab || isSlot(child, Tab))) { - const value = (controlledApiEnabled ? child.props.value : undefined) ?? `${tabIndex}` + const value = child.props.value ?? `${tabIndex}` tabIndex++ return cloneElement(child, {value}) } if (isValidElement(child) && (child.type === Panel || isSlot(child, Panel))) { - const value = (controlledApiEnabled ? child.props.value : undefined) ?? `${panelIndex}` + const value = child.props.value ?? `${panelIndex}` panelIndex++ return cloneElement(child, {value}) } @@ -197,7 +190,7 @@ const UnderlinePanels: FCWithSlotMarker = ({ const tabsHaveIcons = tabs.some(tab => React.isValidElement(tab) && tab.props.icon) return [tabs, tabPanels, tabsHaveIcons, selectedFromProps, tabValues, panelValues] as const - }, [children, controlledApiEnabled]) + }, [children]) // Hand-rolled rather than `useControllableState` because of the third, back-compat // `aria-selected` seed, which is re-synced during render.