diff --git a/.changeset/underlinepanels-controlled-value.md b/.changeset/underlinepanels-controlled-value.md new file mode 100644 index 00000000000..812be69d2f6 --- /dev/null +++ b/.changeset/underlinepanels-controlled-value.md @@ -0,0 +1,5 @@ +--- +"@primer/react": minor +--- + +UnderlinePanels: add controlled `value`/`defaultValue`/`onChange` and `activationMode` props, keyed to a per-`Tab`/`Panel` `value`, for data-driven tabs. Behind the `primer_react_underline_panels_controlled` feature flag; the existing `aria-selected`/`onSelect` API is unchanged. diff --git a/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts b/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts index fcf8dcca3ab..dcf90e27d15 100644 --- a/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts +++ b/packages/react/src/FeatureFlags/DefaultFeatureFlags.ts @@ -9,4 +9,5 @@ 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 9bfdebc1314..568eb51217b 100644 --- a/packages/react/src/experimental/Tabs/README.mdx +++ b/packages/react/src/experimental/Tabs/README.mdx @@ -60,6 +60,18 @@ All styling and layout is left up to you! This approach provides maximum flexibility, allowing you to create tabbed interfaces that fit seamlessly into your application's design while leveraging the robust functionality provided by the `Tabs` component. +### Activation mode + +By default, selection follows focus: navigating with the Arrow, Home, and End keys immediately selects the newly focused tab (`activationMode="automatic"`). Set `activationMode="manual"` to move focus without changing selection — selection is then committed with Enter, Space, or a pointer click. Prefer manual activation when displaying a panel is not instant (for example, it triggers a network request), per the [WAI-ARIA Authoring Practices](https://www.w3.org/WAI/ARIA/apg/patterns/tabs/). + +```tsx + + {/* ... */} + +``` + +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.features.stories.tsx b/packages/react/src/experimental/Tabs/Tabs.features.stories.tsx index 3a196b7d3b7..a90a9fa1054 100644 --- a/packages/react/src/experimental/Tabs/Tabs.features.stories.tsx +++ b/packages/react/src/experimental/Tabs/Tabs.features.stories.tsx @@ -61,6 +61,31 @@ export const Controlled = () => { ) } +export const ManualActivation = () => ( + <> + + This example shows the `Tabs` component with `activationMode` set to `manual`. Arrow keys move focus between tabs + without selecting them; press Enter or Space (or click) to select the focused tab. + + { + action('onValueChange')({value}) + }} + > + + One + Two + Three + + Panel one + Panel two + Panel three + + +) + export const Vertical = () => ( <> diff --git a/packages/react/src/experimental/Tabs/Tabs.test.tsx b/packages/react/src/experimental/Tabs/Tabs.test.tsx index c0d1c1956ab..b7292fe54d7 100644 --- a/packages/react/src/experimental/Tabs/Tabs.test.tsx +++ b/packages/react/src/experimental/Tabs/Tabs.test.tsx @@ -89,6 +89,28 @@ describe('Tabs', () => { expect(onValueChange).toHaveBeenCalledTimes(1) }) + test('onValueChange is not called when the selected tab is re-selected', async () => { + const user = userEvent.setup() + const onValueChange = vi.fn() + + render( + + + + Tab A + Tab B + + Panel A + Panel B + + , + ) + + await user.click(screen.getByRole('tab', {name: 'Tab A'})) + + expect(onValueChange).not.toHaveBeenCalled() + }) + describe('TabList', () => { test('renders with role="tablist"', () => { render( @@ -583,6 +605,115 @@ describe('Tabs', () => { expect(tabB).toHaveAttribute('tabindex', '0') expect(tabC).toHaveAttribute('tabindex', '-1') }) + + describe('manual activation', () => { + const renderManualTabs = (onValueChange?: (args: {value: string}) => void) => + render( + + + + Tab A + Tab B + Tab C + + Panel A + Panel B + Panel C + + , + ) + + test('arrow keys move focus without changing selection', async () => { + const user = userEvent.setup() + const onValueChange = vi.fn() + renderManualTabs(onValueChange) + + const tabA = screen.getByRole('tab', {name: 'Tab A'}) + const tabB = screen.getByRole('tab', {name: 'Tab B'}) + + await act(async () => { + tabA.focus() + await user.keyboard('{ArrowRight}') + }) + + expect(tabB).toHaveFocus() + expect(tabA).toHaveAttribute('aria-selected', 'true') + expect(tabB).toHaveAttribute('aria-selected', 'false') + expect(onValueChange).not.toHaveBeenCalled() + expect(tabB).toHaveAttribute('tabindex', '0') + expect(tabA).toHaveAttribute('tabindex', '-1') + }) + + test('Enter commits the focused tab', async () => { + const user = userEvent.setup() + const onValueChange = vi.fn() + renderManualTabs(onValueChange) + + const tabA = screen.getByRole('tab', {name: 'Tab A'}) + const tabB = screen.getByRole('tab', {name: 'Tab B'}) + + await act(async () => { + tabA.focus() + await user.keyboard('{ArrowRight}') + await user.keyboard('{Enter}') + }) + + expect(tabB).toHaveAttribute('aria-selected', 'true') + expect(onValueChange).toHaveBeenCalledTimes(1) + expect(onValueChange).toHaveBeenCalledWith({value: 'b'}) + }) + + test('Space commits the focused tab', async () => { + const user = userEvent.setup() + const onValueChange = vi.fn() + renderManualTabs(onValueChange) + + const tabA = screen.getByRole('tab', {name: 'Tab A'}) + const tabC = screen.getByRole('tab', {name: 'Tab C'}) + + await act(async () => { + tabA.focus() + await user.keyboard('{ArrowLeft}') + await user.keyboard(' ') + }) + + expect(tabC).toHaveAttribute('aria-selected', 'true') + expect(onValueChange).toHaveBeenCalledWith({value: 'c'}) + }) + + test('clicking commits selection', async () => { + const onValueChange = vi.fn() + renderManualTabs(onValueChange) + + const tabB = screen.getByRole('tab', {name: 'Tab B'}) + + await act(() => { + fireEvent.mouseDown(tabB) + }) + + expect(tabB).toHaveAttribute('aria-selected', 'true') + expect(onValueChange).toHaveBeenCalledWith({value: 'b'}) + }) + + test('subsequent arrow keys move relative to the focused tab, not the selected tab', async () => { + const user = userEvent.setup() + renderManualTabs() + + const tabA = screen.getByRole('tab', {name: 'Tab A'}) + const tabB = screen.getByRole('tab', {name: 'Tab B'}) + const tabC = screen.getByRole('tab', {name: 'Tab C'}) + + await act(async () => { + tabA.focus() + await user.keyboard('{ArrowRight}') + await user.keyboard('{ArrowRight}') + }) + + expect(tabA).toHaveAttribute('aria-selected', 'true') + expect(tabC).toHaveFocus() + expect(tabB).not.toHaveFocus() + }) + }) }) function TabListRefHarness({tabRef}: {tabRef: React.Ref}) { diff --git a/packages/react/src/experimental/Tabs/Tabs.tsx b/packages/react/src/experimental/Tabs/Tabs.tsx index 08514bb2eee..27ec7f6b089 100644 --- a/packages/react/src/experimental/Tabs/Tabs.tsx +++ b/packages/react/src/experimental/Tabs/Tabs.tsx @@ -2,6 +2,7 @@ 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' @@ -18,6 +19,9 @@ 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 [selectedValue, setSelectedValue] = useControllableState({ name: 'tab-selection', @@ -25,17 +29,29 @@ function Tabs(props: TabsProps) { value: props.value, }) + // In manual activation the roving tab stop follows focus, not selection, so track it separately. + const [focusedValue, setFocusedValue] = React.useState() + const savedOnValueChange = React.useRef(onValueChange) const contextValue: TabsContextValue = useMemo(() => { return { groupId, selectedValue, + activationMode, + focusedValue, selectTab(value: string) { + setFocusedValue(undefined) + if (controlledApiEnabled && value === selectedValue) { + return + } setSelectedValue(value) savedOnValueChange.current?.({value}) }, + focusTab(value: string) { + setFocusedValue(value) + }, } - }, [groupId, selectedValue, setSelectedValue]) + }, [groupId, selectedValue, activationMode, focusedValue, controlledApiEnabled, setSelectedValue]) useIsomorphicLayoutEffect(() => { savedOnValueChange.current = onValueChange diff --git a/packages/react/src/experimental/Tabs/types.ts b/packages/react/src/experimental/Tabs/types.ts index 2c518f48447..046de6e1c52 100644 --- a/packages/react/src/experimental/Tabs/types.ts +++ b/packages/react/src/experimental/Tabs/types.ts @@ -49,6 +49,13 @@ type CommonTabsProps = { * unique id is generated automatically. */ id?: string + + /** + * `'automatic'` (default) selects on focus, so Arrow/Home/End keys select immediately. `'manual'` + * only moves focus; selection commits on Enter/Space/click. Prefer `'manual'` when displaying a + * panel is not instant (e.g. it triggers a network request). + */ + activationMode?: 'automatic' | 'manual' } export type TabsProps = PropsWithChildren<(ControlledTabsProps | UncontrolledTabsProps) & CommonTabsProps> @@ -89,7 +96,15 @@ export type TabPanelProps = { export type TabsContextValue = { groupId: string selectedValue: string + activationMode: 'automatic' | 'manual' + /** + * In `'manual'` activation, the most recently focused tab; the roving tab stop follows it. Set on + * any focus (initially the selected tab); `undefined` before the first focus and after a commit, + * when the tab stop falls back to the selected tab. + */ + focusedValue: string | undefined selectTab(value: string): void + focusTab(value: string): void } export type TabListHookProps = TabListProps & { diff --git a/packages/react/src/experimental/Tabs/useTab.ts b/packages/react/src/experimental/Tabs/useTab.ts index ab35a525ad5..cce08181d53 100644 --- a/packages/react/src/experimental/Tabs/useTab.ts +++ b/packages/react/src/experimental/Tabs/useTab.ts @@ -13,6 +13,9 @@ export function useTab(props: Pick) { if (event.key === ' ' || event.key === 'Enter') { tabs.selectTab(value) @@ -28,6 +31,12 @@ export function useTab(props: Pick(props: Pick(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`. @@ -24,6 +25,19 @@ 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 + } + } + return tabs.findIndex(tab => { + return tab.getAttribute('aria-selected') === 'true' + }) + } + const isVertical = ariaOrientation === 'vertical' const nextKey = isVertical ? 'ArrowDown' : 'ArrowRight' const prevKey = isVertical ? 'ArrowUp' : 'ArrowLeft' @@ -34,24 +48,20 @@ export function useTabList(props: TabListHookProps): T } if (event.key === nextKey) { - const selectedTabIndex = tabs.findIndex(tab => { - return tab.getAttribute('aria-selected') === 'true' - }) - if (selectedTabIndex === -1) { + const currentTabIndex = getCurrentIndex() + if (currentTabIndex === -1) { return } - const nextTabIndex = (selectedTabIndex + 1) % tabs.length + const nextTabIndex = (currentTabIndex + 1) % tabs.length tabs[nextTabIndex].focus() } else if (event.key === prevKey) { - const selectedTabIndex = tabs.findIndex(tab => { - return tab.getAttribute('aria-selected') === 'true' - }) - if (selectedTabIndex === -1) { + const currentTabIndex = getCurrentIndex() + if (currentTabIndex === -1) { return } - const nextTabIndex = (tabs.length + selectedTabIndex - 1) % tabs.length + const nextTabIndex = (tabs.length + currentTabIndex - 1) % tabs.length tabs[nextTabIndex].focus() } else if (event.key === 'Home') { if (tabs[0]) { diff --git a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.docs.json b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.docs.json index 332e803b990..631730a9242 100644 --- a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.docs.json +++ b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.docs.json @@ -24,6 +24,18 @@ }, { "id": "experimental-components-underlinepanels-features--with-icons-hidden-on-narrow-screen" + }, + { + "id": "experimental-components-underlinepanels-features--controlled" + }, + { + "id": "experimental-components-underlinepanels-features--uncontrolled" + }, + { + "id": "experimental-components-underlinepanels-features--manual-activation" + }, + { + "id": "experimental-components-underlinepanels-features--in-overlay" } ], "importPath": "@primer/react/experimental", @@ -40,6 +52,30 @@ "defaultValue": "", "description": "ID of the element containing the name for the tab list" }, + { + "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." + }, + { + "name": "defaultValue", + "type": "string", + "defaultValue": "", + "description": "The value of the tab selected by default, for an uncontrolled component. Cannot be combined with `value`." + }, + { + "name": "onChange", + "type": "({value}: {value: string}) => void", + "defaultValue": "", + "description": "Callback fired whenever the selected tab changes, for every selection method — pointer, Enter/Space, and Arrow/Home/End keys. Unlike `UnderlinePanels.Tab`'s `onSelect` (which only fires on click and Enter/Space), this makes the selected value usable as a single source of truth for data-driven tabs." + }, + { + "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." + }, { "name": "children", "type": "Array", @@ -70,6 +106,12 @@ { "name": "UnderlinePanels.Tab", "props": [ + { + "name": "value", + "type": "string", + "defaultValue": "", + "description": "A value that uniquely identifies this tab, paired with the `UnderlinePanels.Panel` of the same `value`. Provide this to use domain values (e.g. `'branch'`) with the container's controlled `value`/`onChange` API. When omitted, tabs and panels are paired by DOM order." + }, { "name": "aria-selected", "type": "| boolean | 'true' | 'false'", @@ -80,7 +122,7 @@ "name": "onSelect", "type": "(event) => void", "defaultValue": "", - "description": "The handler that gets called when the tab is selected" + "description": "The handler that gets called when the tab is selected. Note: this fires only on click and Enter/Space — not on arrow-key navigation. For a callback that fires for every selection method, use the container's `onChange` prop instead." }, { "name": "counter", @@ -98,7 +140,14 @@ }, { "name": "UnderlinePanels.Panel", - "props": [], + "props": [ + { + "name": "value", + "type": "string", + "defaultValue": "", + "description": "A value that uniquely identifies this panel, paired with the `UnderlinePanels.Tab` of the same `value`. When omitted, tabs and panels are paired by DOM order." + } + ], "passthrough": { "element": "div", "url": "https://developer.mozilla.org/en-US/docs/Web/HTML/Element/div#Attributes" diff --git a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.features.stories.tsx b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.features.stories.tsx index df32f401167..faf2481ebfc 100644 --- a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.features.stories.tsx +++ b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.features.stories.tsx @@ -1,17 +1,24 @@ import type {Meta} from '@storybook/react-vite' +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' import { CodeIcon, CommentDiscussionIcon, EyeIcon, GearIcon, + GitBranchIcon, GitPullRequestIcon, GraphIcon, PlayIcon, ProjectIcon, ShieldLockIcon, + TagIcon, } from '@primer/octicons-react' export default { @@ -115,3 +122,129 @@ 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 ( + <> + + { + action('onChange')({value}) + setRefType(value) + }} + > + + Branches + + + Tags + + Find or create a branch… + Search or create a new tag… + +

+ Selected ref type: {refType} +

+ + ) +} + +export const Uncontrolled = () => ( + <> + + { + action('onChange')({value}) + }} + > + Branches + Tags + Find or create a branch… + Search or create a new tag… + + +) + +export const ManualActivation = () => { + const [refType, setRefType] = useState('branch') + + 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. +

+ { + action('onChange')({value}) + setRefType(value) + }} + > + Branches + Tags + Find or create a branch… + Search or create a new tag… + + + ) +} + +// The tablist implements its own roving tabindex, so disable the overlay's focus zone to stop the +// two from both managing `tabindex` — a competing focus zone can leave every tab at `tabindex="-1"` +// and trap keyboard users. +export const InOverlay = () => { + const [open, setOpen] = useState(false) + const [refType, setRefType] = useState('branch') + + return ( + <> + + setOpen(true)} + onClose={() => setOpen(false)} + renderAnchor={props => } + overlayProps={{role: 'dialog', 'aria-modal': true, 'aria-label': 'Select a ref type', style: {width: '320px'}}} + focusZoneSettings={{disabled: true}} + > + { + action('onChange')({value}) + setRefType(value) + }} + > + Branches + Tags + Find or create a branch… + Search or create a new tag… + + + + ) +} diff --git a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.test.tsx b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.test.tsx index 3e4a9f97d9c..6e4138b90f5 100644 --- a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.test.tsx +++ b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.test.tsx @@ -3,13 +3,15 @@ // public API and its integration with Tabs. import type React from 'react' -import {act} from 'react' +import {act, useState} from 'react' import {render, screen} from '@testing-library/react' import userEvent from '@testing-library/user-event' import {describe, it, afterEach, beforeEach, expect, vi} from 'vitest' import {CodeIcon, EyeIcon} from '@primer/octicons-react' import UnderlinePanels from './UnderlinePanels' -import {implementsClassName, withExpectedConsoleError} from '../../utils/testing' +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}) => ( @@ -197,6 +199,310 @@ describe('UnderlinePanels', () => { }).toThrow('Only one tab can be selected at a time.') }) }) + + 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 + + + ) + + it('`value` selects the matching tab and shows its panel', () => { + render() + + expect(screen.getByRole('tab', {name: 'Tags'})).toHaveAttribute('aria-selected', 'true') + expect(screen.getByRole('tab', {name: 'Branches'})).toHaveAttribute('aria-selected', 'false') + expect(screen.getByText('Tag panel')).toBeVisible() + expect(screen.getByText('Branch panel')).not.toBeVisible() + }) + + it('calls onChange with the domain value when a tab is clicked', async () => { + const user = userEvent.setup() + const onChange = vi.fn() + render() + + await user.click(screen.getByRole('tab', {name: 'Tags'})) + + expect(onChange).toHaveBeenCalledWith({value: 'tag'}) + }) + + it('does not call onChange when the selected tab is re-clicked', async () => { + const user = userEvent.setup() + const onChange = vi.fn() + render() + + await user.click(screen.getByRole('tab', {name: 'Branches'})) + + expect(onChange).not.toHaveBeenCalled() + }) + + it('calls onChange on arrow-key navigation (automatic activation)', async () => { + const user = userEvent.setup() + const onChange = vi.fn() + render() + + await act(async () => { + screen.getByRole('tab', {name: 'Branches'}).focus() + await user.keyboard('{ArrowRight}') + }) + + expect(onChange).toHaveBeenCalledWith({value: 'tag'}) + }) + + it('does not update selection while controlled unless `value` changes', async () => { + const user = userEvent.setup() + const onChange = vi.fn() + const {rerender} = render() + + await user.click(screen.getByRole('tab', {name: 'Tags'})) + + expect(screen.getByRole('tab', {name: 'Branches'})).toHaveAttribute('aria-selected', 'true') + + rerender() + expect(screen.getByRole('tab', {name: 'Tags'})).toHaveAttribute('aria-selected', 'true') + }) + + it('`defaultValue` sets the initial selection and updates internally (uncontrolled)', async () => { + const user = userEvent.setup() + const onChange = vi.fn() + render() + + expect(screen.getByRole('tab', {name: 'Tags'})).toHaveAttribute('aria-selected', 'true') + + await user.click(screen.getByRole('tab', {name: 'Branches'})) + + expect(screen.getByRole('tab', {name: 'Branches'})).toHaveAttribute('aria-selected', 'true') + expect(onChange).toHaveBeenCalledWith({value: 'branch'}) + expect(screen.getByText('Branch panel')).toBeVisible() + }) + + describe('manual activation', () => { + it('arrow keys move focus without selecting or firing onChange', async () => { + const user = userEvent.setup() + const onChange = vi.fn() + render() + + const branch = screen.getByRole('tab', {name: 'Branches'}) + const tag = screen.getByRole('tab', {name: 'Tags'}) + + await act(async () => { + branch.focus() + await user.keyboard('{ArrowRight}') + }) + + expect(tag).toHaveFocus() + expect(branch).toHaveAttribute('aria-selected', 'true') + expect(onChange).not.toHaveBeenCalled() + expect(tag).toHaveAttribute('tabindex', '0') + expect(branch).toHaveAttribute('tabindex', '-1') + }) + + it('commits selection with Enter', async () => { + const user = userEvent.setup() + const onChange = vi.fn() + render() + + await act(async () => { + screen.getByRole('tab', {name: 'Branches'}).focus() + await user.keyboard('{ArrowRight}') + await user.keyboard('{Enter}') + }) + + expect(onChange).toHaveBeenCalledTimes(1) + expect(onChange).toHaveBeenCalledWith({value: 'tag'}) + }) + + it('commits selection with Enter when uncontrolled (defaultValue)', async () => { + const user = userEvent.setup() + const onChange = vi.fn() + render() + + const branch = screen.getByRole('tab', {name: 'Branches'}) + const tag = screen.getByRole('tab', {name: 'Tags'}) + + await act(async () => { + branch.focus() + await user.keyboard('{ArrowRight}') + }) + + expect(tag).toHaveFocus() + expect(branch).toHaveAttribute('aria-selected', 'true') + expect(onChange).not.toHaveBeenCalled() + + await act(async () => { + await user.keyboard('{Enter}') + }) + + expect(onChange).toHaveBeenCalledWith({value: 'tag'}) + expect(tag).toHaveAttribute('aria-selected', 'true') + expect(screen.getByText('Tag panel')).toBeVisible() + }) + }) + + it('fires onChange on arrow keys even without explicit values (positional back-compat)', async () => { + const user = userEvent.setup() + const onChange = vi.fn() + render( + + + Tab 1 + Tab 2 + Panel 1 + Panel 2 + + , + ) + + await act(async () => { + screen.getByRole('tab', {name: 'Tab 1'}).focus() + await user.keyboard('{ArrowRight}') + }) + + expect(onChange).toHaveBeenCalledWith({value: '1'}) + expect(screen.getByRole('tab', {name: 'Tab 2'})).toHaveAttribute('aria-selected', 'true') + }) + + describe('dev validation and fallback', () => { + const RefTabsRaw = (props: { + value?: string + defaultValue?: string + tabs?: Array<{tab: string; panel: string}> + }) => { + const pairs = props.tabs ?? [ + {tab: 'branch', panel: 'branch'}, + {tab: 'tag', panel: 'tag'}, + ] + return ( + + + {pairs.map((p, i) => ( + + {p.tab} + + ))} + {pairs.map((p, i) => ( + + {p.panel} panel + + ))} + + + ) + } + + it('clamps to the first tab (and warns) when the selected value matches no tab', () => { + withExpectedConsoleWarning(() => { + render() + }) + + const branch = screen.getByRole('tab', {name: 'branch'}) + const tag = screen.getByRole('tab', {name: 'tag'}) + + expect(branch).toHaveAttribute('aria-selected', 'true') + expect(branch).toHaveAttribute('tabindex', '0') + expect(tag).toHaveAttribute('tabindex', '-1') + expect(screen.getByText('branch panel')).toBeVisible() + }) + + it('warns when `value` and `defaultValue` are combined, and `value` wins', () => { + withExpectedConsoleWarning(() => { + render() + }) + + expect(screen.getByRole('tab', {name: 'branch'})).toHaveAttribute('aria-selected', 'true') + }) + + it('throws when two tabs share the same value', () => { + withExpectedConsoleError(() => { + expect(() => { + render( + , + ) + }).toThrow('Every tab must have a unique `value`. Found duplicate "branch".') + }) + }) + + it('throws when a tab has no matching panel', () => { + withExpectedConsoleError(() => { + expect(() => { + render( + , + ) + }).toThrow('Tab with `value` "tag" has no matching panel') + }) + }) + }) + + 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') + }) + }) + }) }) describe('UnderlinePanels — render architecture', () => { @@ -382,3 +688,40 @@ describe('UnderlinePanels — list resize observation', () => { expect(wrapper).toHaveAttribute('data-icons-visible', 'false') }) }) + +describe('UnderlinePanels — AnchoredOverlay composition', () => { + const OverlayHarness = ({onChange}: {onChange?: ({value}: {value: string}) => void}) => { + const [open, setOpen] = useState(true) + return ( + setOpen(true)} + onClose={() => setOpen(false)} + renderAnchor={props => ( + + )} + focusZoneSettings={{disabled: true}} + > + + Branches + Tags + Branch panel + Tag panel + + + ) + } + + it('keeps the tablist roving tabindex intact (exactly one tab is tabbable)', async () => { + render() + await act(async () => {}) + + const branch = screen.getByRole('tab', {name: 'Branches'}) + const tag = screen.getByRole('tab', {name: 'Tags'}) + + expect(branch).toHaveAttribute('tabindex', '0') + expect(tag).toHaveAttribute('tabindex', '-1') + }) +}) diff --git a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx index 1d0d3022a89..8496ea3f869 100644 --- a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx +++ b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx @@ -14,6 +14,8 @@ 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' import classes from './UnderlinePanels.module.css' @@ -36,6 +38,27 @@ export type UnderlinePanelsProps = { * ID of the element containing the name for the tab list */ 'aria-labelledby'?: React.AriaAttributes['aria-labelledby'] + /** + * 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. + */ + value?: string + /** + * The value of the tab that should be selected by default, keyed to each tab's `value`. Use this + * for an uncontrolled component. Cannot be combined with `value`. + */ + defaultValue?: string + /** + * Fires whenever the selected tab changes, for every selection method including arrow keys. + * Unlike `UnderlinePanels.Tab`'s `onSelect`, which only fires on click and Enter/Space. + */ + onChange?: ({value}: {value: string}) => void + /** + * `'automatic'` (default) selects on focus/arrow keys; `'manual'` only moves focus until + * Enter/Space/click. Prefer `'manual'` when displaying a panel is not instant. + */ + activationMode?: 'automatic' | 'manual' /** * Custom string to use when generating the IDs of tabs and `aria-labelledby` for the panels */ @@ -55,6 +78,12 @@ export type UnderlinePanelsProps = { } export type TabProps = PropsWithChildren<{ + /** + * A value that uniquely identifies this tab, paired with the `UnderlinePanels.Panel` of the same + * `value`. Provide this to use domain values (e.g. `'branch'`) with the container's controlled + * `value`/`onChange` API. When omitted, tabs and panels are paired by their DOM order. + */ + value?: string /** * Whether this is the selected tab */ @@ -73,10 +102,15 @@ export type TabProps = PropsWithChildren<{ icon?: FC }> -export type PanelProps = React.HTMLAttributes +export type PanelProps = React.HTMLAttributes & { + /** + * A value that uniquely identifies this panel, paired with the `UnderlinePanels.Tab` of the same + * `value`. When omitted, tabs and panels are paired by their DOM order. + */ + value?: string +} -// Internal-only positional value injected via cloneElement to pair the Nth tab -// with the Nth panel. Not part of the public Tab/Panel API. +// Explicit consumer `value`, or a positional fallback pairing the Nth tab with the Nth panel. type WithValue = {value?: string} // Carries flags that affect every Tab's rendering but that don't belong on the @@ -99,8 +133,18 @@ const UnderlinePanels: FCWithSlotMarker = ({ children, loadingCounters, className, + value: valueProp, + defaultValue: defaultValueProp, + onChange: onChangeProp, + 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) @@ -108,59 +152,78 @@ const UnderlinePanels: FCWithSlotMarker = ({ // called in the exact same order in every component render const parentId = useId(props.id) - const [tabs, tabPanels, tabsHaveIcons, selectedFromProps] = useMemo(() => { - // Clone each Tab/Panel with a positional `value` so the Tabs hooks can pair - // the Nth tab with the Nth panel. Derived in render (not an effect) to avoid - // an empty-tablist frame; iconsVisible/loadingCounters flow via context so - // this memo can stay keyed on [children]. + const [tabs, tabPanels, tabsHaveIcons, selectedFromProps, tabValues, panelValues] = useMemo(() => { + // Derived in render (not an effect) to avoid an empty-tablist frame. let tabIndex = 0 let panelIndex = 0 const childrenWithProps = Children.map(children, child => { if (isValidElement(child) && (child.type === Tab || isSlot(child, Tab))) { - return cloneElement(child, {value: `${tabIndex++}`}) + const value = (controlledApiEnabled ? child.props.value : undefined) ?? `${tabIndex}` + tabIndex++ + return cloneElement(child, {value}) } if (isValidElement(child) && (child.type === Panel || isSlot(child, Panel))) { - return cloneElement(child, {value: `${panelIndex++}`}) + const value = (controlledApiEnabled ? child.props.value : undefined) ?? `${panelIndex}` + panelIndex++ + return cloneElement(child, {value}) } return child }) const tabs: React.ReactNode[] = [] const tabPanels: React.ReactNode[] = [] + const tabValues: string[] = [] + const panelValues: string[] = [] let selectedFromProps: string | undefined for (const child of Children.toArray(childrenWithProps)) { if (!isValidElement(child)) continue if (child.type === Tab || isSlot(child, Tab)) { + const value = (child.props as WithValue).value + if (value !== undefined) tabValues.push(value) const ariaSelected = (child.props as {'aria-selected'?: boolean | string})['aria-selected'] if (ariaSelected === true || ariaSelected === 'true') { - selectedFromProps = `${tabs.length}` + selectedFromProps = value } tabs.push(child) } else if (child.type === Panel || isSlot(child, Panel)) { + const value = (child.props as WithValue).value + if (value !== undefined) panelValues.push(value) tabPanels.push(child) } } const tabsHaveIcons = tabs.some(tab => React.isValidElement(tab) && tab.props.icon) - return [tabs, tabPanels, tabsHaveIcons, selectedFromProps] as const - }, [children]) + return [tabs, tabPanels, tabsHaveIcons, selectedFromProps, tabValues, panelValues] as const + }, [children, controlledApiEnabled]) - // Hybrid selection: seed from the consumer's `aria-selected` prop, but let - // clicks/keyboard update selection internally (mirrors the previous - // tab-container-element behavior). Re-sync via React's "adjust state during - // render" pattern when the selected prop changes. - const [selectedValue, setSelectedValue] = useState(() => selectedFromProps ?? '0') + // Hand-rolled rather than `useControllableState` because of the third, back-compat + // `aria-selected` seed, which is re-synced during render. + const isControlled = controlledValue !== undefined + const [uncontrolledValue, setUncontrolledValue] = useState(() => defaultValue ?? selectedFromProps ?? '0') const [prevSelectedFromProps, setPrevSelectedFromProps] = useState(selectedFromProps) - if (selectedFromProps !== prevSelectedFromProps) { + if (!isControlled && selectedFromProps !== prevSelectedFromProps) { setPrevSelectedFromProps(selectedFromProps) - if (selectedFromProps !== undefined && selectedFromProps !== selectedValue) { - setSelectedValue(selectedFromProps) + if (selectedFromProps !== undefined && selectedFromProps !== uncontrolledValue) { + setUncontrolledValue(selectedFromProps) } } + const selectedValue = isControlled ? controlledValue : uncontrolledValue + + // Fall back to the first tab when the value matches none, so the tablist keeps a tabbable + // entry point. Does not fire `onChange`. + const effectiveValue = tabValues.includes(selectedValue) ? selectedValue : (tabValues[0] ?? selectedValue) + + const handleValueChange = (value: string) => { + if (!isControlled) { + setUncontrolledValue(value) + } + onChange?.({value}) + } + const {tabListProps} = useTabList({ 'aria-label': ariaLabel, 'aria-labelledby': ariaLabelledBy, @@ -220,17 +283,58 @@ const UnderlinePanels: FCWithSlotMarker = ({ return ariaSelected === true || ariaSelected === 'true' }) + // Structural problems throw; recoverable misconfiguration warns. invariant(selectedTabs.length <= 1, 'Only one tab can be selected at a time.') invariant( tabs.length === tabPanels.length, `The number of tabs and panels must be equal. Counted ${tabs.length} tabs and ${tabPanels.length} panels.`, ) + + // Tab and panel ids are derived from `value`, so duplicates would collide. + const duplicateTabValue = tabValues.find((value, index) => tabValues.indexOf(value) !== index) + invariant( + duplicateTabValue === undefined, + `Every tab must have a unique \`value\`. Found duplicate "${duplicateTabValue}".`, + ) + const duplicatePanelValue = panelValues.find((value, index) => panelValues.indexOf(value) !== index) + invariant( + duplicatePanelValue === undefined, + `Every panel must have a unique \`value\`. Found duplicate "${duplicatePanelValue}".`, + ) + + const unmatchedTabValue = tabValues.find(value => !panelValues.includes(value)) + invariant( + unmatchedTabValue === undefined, + `Tab with \`value\` "${unmatchedTabValue}" has no matching panel. Give a \`UnderlinePanels.Panel\` the same \`value\`.`, + ) + + warning( + isControlled && selectedTabs.length > 0, + 'UnderlinePanels: do not combine the controlled `value` prop with `aria-selected` on a tab. `value` takes precedence; use `value`/`onChange` (or `defaultValue`) to drive selection.', + ) + + warning( + controlledValue !== undefined && defaultValue !== undefined, + 'UnderlinePanels: do not combine the controlled `value` prop with `defaultValue`. `value` takes precedence; use one or the other.', + ) + + // Already clamped to the first tab above; warn since it's rarely intended. + const providedValue = controlledValue ?? defaultValue + warning( + providedValue !== undefined && tabValues.length > 0 && !tabValues.includes(providedValue), + `UnderlinePanels: the selected value "${providedValue}" does not match any tab, so the first tab is selected instead. Provide a \`value\`/\`defaultValue\` that matches a \`UnderlinePanels.Tab\` \`value\`.`, + ) } return ( - setSelectedValue(value)}> + handleValueChange(value)} + >