From b4c23edebd51fac8861d1beb3d147e90f50fc09d Mon Sep 17 00:00:00 2001 From: Ricky Zhang Date: Wed, 22 Jul 2026 09:56:32 -0400 Subject: [PATCH 1/2] Migrate UnderlinePanels to the Tabs component Rebuild the experimental UnderlinePanels on the Tabs primitive (Tabs + useTab/useTabList/useTabPanel) instead of @github/tab-container-element, preserving its public API and behavior. Add an optional id prop to Tabs and remove the @github/tab-container-element dependency. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5b92d807-358b-48fe-b099-5aed24842200 --- .changeset/underlinepanels-tabs-migration.md | 5 + package-lock.json | 5 - packages/react/package.json | 1 - packages/react/src/experimental/Tabs/Tabs.tsx | 3 +- packages/react/src/experimental/Tabs/types.ts | 10 +- .../UnderlinePanels.dev.stories.tsx | 27 ++++ .../UnderlinePanels/UnderlinePanels.test.tsx | 55 ++++++- .../UnderlinePanels/UnderlinePanels.tsx | 146 ++++++++++-------- 8 files changed, 176 insertions(+), 76 deletions(-) create mode 100644 .changeset/underlinepanels-tabs-migration.md diff --git a/.changeset/underlinepanels-tabs-migration.md b/.changeset/underlinepanels-tabs-migration.md new file mode 100644 index 00000000000..d81d0df45e4 --- /dev/null +++ b/.changeset/underlinepanels-tabs-migration.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +UnderlinePanels: The experimental `UnderlinePanels` component is now built on the experimental `Tabs` component instead of `@github/tab-container-element`. Its public API and behavior are unchanged. diff --git a/package-lock.json b/package-lock.json index 14e51e632b1..fbc05ce7a59 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3913,10 +3913,6 @@ "integrity": "sha512-L/2r0DNR/rMbmHWcsdmhtOiy2gESoGOhItNFD4zJ3nZfHl79Dx3N18Vfx/pYr2lruMOdk1cJZb4wEumm+Dxm1w==", "license": "MIT" }, - "node_modules/@github/tab-container-element": { - "version": "4.8.2", - "license": "MIT" - }, "node_modules/@hono/node-server": { "version": "1.19.14", "resolved": "https://registry.npmjs.org/@hono/node-server/-/node-server-1.19.14.tgz", @@ -28477,7 +28473,6 @@ "dependencies": { "@github/mini-throttle": "^2.1.1", "@github/relative-time-element": "^5.0.0", - "@github/tab-container-element": "^4.8.2", "@lit-labs/react": "1.2.1", "@oddbird/popover-polyfill": "^0.5.2", "@primer/behaviors": "^1.10.3", diff --git a/packages/react/package.json b/packages/react/package.json index 33843331411..883d05918d0 100644 --- a/packages/react/package.json +++ b/packages/react/package.json @@ -77,7 +77,6 @@ "dependencies": { "@github/mini-throttle": "^2.1.1", "@github/relative-time-element": "^5.0.0", - "@github/tab-container-element": "^4.8.2", "@lit-labs/react": "1.2.1", "@oddbird/popover-polyfill": "^0.5.2", "@primer/behaviors": "^1.10.3", diff --git a/packages/react/src/experimental/Tabs/Tabs.tsx b/packages/react/src/experimental/Tabs/Tabs.tsx index b8c267cd3d0..08514bb2eee 100644 --- a/packages/react/src/experimental/Tabs/Tabs.tsx +++ b/packages/react/src/experimental/Tabs/Tabs.tsx @@ -16,7 +16,8 @@ import {useTabPanel} from './useTabPanel' */ function Tabs(props: TabsProps) { const {children, onValueChange} = props - const groupId = useId() + const generatedId = useId() + const groupId = props.id ?? generatedId const [selectedValue, setSelectedValue] = useControllableState({ name: 'tab-selection', diff --git a/packages/react/src/experimental/Tabs/types.ts b/packages/react/src/experimental/Tabs/types.ts index 45337b1b9e8..898be8a2872 100644 --- a/packages/react/src/experimental/Tabs/types.ts +++ b/packages/react/src/experimental/Tabs/types.ts @@ -43,7 +43,15 @@ type UncontrolledTabsProps = { onValueChange?: ({value}: {value: string}) => void } -export type TabsProps = PropsWithChildren +type CommonTabsProps = { + /** + * Optional id used as the base for generated tab and panel ids. If omitted, a + * unique id is generated automatically. + */ + id?: string +} + +export type TabsProps = PropsWithChildren<(ControlledTabsProps | UncontrolledTabsProps) & CommonTabsProps> type Label = { 'aria-label': string diff --git a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.dev.stories.tsx b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.dev.stories.tsx index 4a5bfe7f980..f0d3c89ee77 100644 --- a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.dev.stories.tsx +++ b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.dev.stories.tsx @@ -1,6 +1,9 @@ +import {useState} from 'react' import type {ComponentProps} from '../../utils/types' import type {Meta, StoryFn} from '@storybook/react-vite' import UnderlinePanels from './UnderlinePanels' +import {AnchoredOverlay} from '../../AnchoredOverlay' +import {Button} from '../../Button' export default { title: 'Experimental/Components/UnderlinePanels/Dev', @@ -45,3 +48,27 @@ SingleTabPlayground.argTypes = { }, }, } + +export const InOverlay = () => { + const [open, setOpen] = useState(false) + + return ( + setOpen(true)} + onClose={() => setOpen(false)} + renderAnchor={props => } + overlayProps={{role: 'dialog', 'aria-modal': true, 'aria-label': 'Select a tab', style: {width: '320px'}}} + focusZoneSettings={{disabled: true}} + > + + Tab 1 + Tab 2 + Tab 3 + Panel 1 + Panel 2 + Panel 3 + + + ) +} diff --git a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.test.tsx b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.test.tsx index e14683ed9c9..3e4a9f97d9c 100644 --- a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.test.tsx +++ b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.test.tsx @@ -1,17 +1,17 @@ -// Most of the functionality is already tested in [@github/tab-container-element](https://github.com/github/tab-container-element) +// Most of the underlying tab behavior is provided by the experimental `Tabs` +// component and its hooks (see ../Tabs). These tests cover the UnderlinePanels +// public API and its integration with Tabs. import type React from 'react' import {act} 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 TabContainerElement from '@github/tab-container-element' import {implementsClassName, withExpectedConsoleError} from '../../utils/testing' import classes from './UnderlinePanels.module.css' -TabContainerElement.prototype.selectTab = vi.fn() - const UnderlinePanelsMockComponent = (props: {'aria-label'?: string; 'aria-labelledby'?: string; id?: string}) => ( Tab 1 @@ -25,12 +25,34 @@ const UnderlinePanelsMockComponent = (props: {'aria-label'?: string; 'aria-label describe('UnderlinePanels', () => { implementsClassName(UnderlinePanels, classes.StyledUnderlineWrapper) - implementsClassName(UnderlinePanels.Tab) - implementsClassName(UnderlinePanels.Panel) afterEach(() => { vi.restoreAllMocks() }) + // Tab/Panel require the Tabs context, so they're rendered inside + // UnderlinePanels rather than via `implementsClassName` (which renders alone). + it('UnderlinePanels.Tab renders with a custom className', () => { + const Tab = UnderlinePanels.Tab as React.ElementType + render( + + Tab 1 + Panel 1 + , + ) + + expect(screen.getByRole('tab', {name: 'Tab 1'})).toHaveClass('test-class') + }) + it('UnderlinePanels.Panel renders with a custom className', () => { + render( + + Tab 1 + Panel 1 + , + ) + + expect(screen.getByText('Panel 1')).toHaveClass('test-class') + }) + it('renders with a custom ID', () => { render() @@ -106,6 +128,27 @@ describe('UnderlinePanels', () => { expect(onSelect).toHaveBeenCalled() }) + it('selects the first tab by default and hides the other panels', () => { + render() + + expect(screen.getByRole('tab', {name: 'Tab 1'})).toHaveAttribute('aria-selected', 'true') + expect(screen.getByText('Panel 1')).toBeVisible() + expect(screen.getByText('Panel 2')).not.toBeVisible() + expect(screen.getByText('Panel 3')).not.toBeVisible() + }) + + it('switches the visible panel when a tab is selected (uncontrolled)', async () => { + const user = userEvent.setup() + render() + + await user.click(screen.getByRole('tab', {name: 'Tab 2'})) + + expect(screen.getByRole('tab', {name: 'Tab 2'})).toHaveAttribute('aria-selected', 'true') + expect(screen.getByRole('tab', {name: 'Tab 1'})).toHaveAttribute('aria-selected', 'false') + expect(screen.getByText('Panel 2')).toBeVisible() + expect(screen.getByText('Panel 1')).not.toBeVisible() + }) + it('throws an error when the number of tabs does not match the number of panels', () => { withExpectedConsoleError(() => { expect(() => { diff --git a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx index 07adf61eb9c..1688de3aefe 100644 --- a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx +++ b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx @@ -9,17 +9,9 @@ import React, { type FC, type PropsWithChildren, useMemo, - type ElementType, } from 'react' -import {TabContainerElement} from '@github/tab-container-element' import type {IconProps} from '@primer/octicons-react' -import {createComponent} from '../../utils/create-component' -import { - UnderlineItemList, - UnderlineWrapper, - UnderlineItem, - type UnderlineItemProps, -} from '../../internal/components/UnderlineTabbedInterface' +import {UnderlineItemList, UnderlineWrapper, UnderlineItem} from '../../internal/components/UnderlineTabbedInterface' import {useId} from '../../hooks' import {invariant} from '../../utils/invariant' import {useResizeObserver, type ResizeObserverEntry} from '../../hooks/useResizeObserver' @@ -28,6 +20,8 @@ import classes from './UnderlinePanels.module.css' import {clsx} from 'clsx' import {isSlot} from '../../utils/is-slot' import type {FCWithSlotMarker} from '../../utils/types' +import {Tabs, useTab, useTabList, useTabPanel} from '../Tabs' +import type {TabListHookProps} from '../Tabs/types' export type UnderlinePanelsProps = { /** @@ -81,7 +75,9 @@ export type TabProps = PropsWithChildren<{ export type PanelProps = React.HTMLAttributes -const TabContainerComponent = createComponent(TabContainerElement, 'tab-container') +// Internal-only positional value injected via cloneElement to pair the Nth tab +// with the Nth panel. Not part of the public Tab/Panel API. +type WithValue = {value?: string} // Carries flags that affect every Tab's rendering but that don't belong on the // consumer-facing Tab API. Passing them via context (instead of cloneElement) @@ -112,43 +108,64 @@ const UnderlinePanels: FCWithSlotMarker = ({ // called in the exact same order in every component render const parentId = useId(props.id) - const [tabs, tabPanels, tabsHaveIcons] = useMemo(() => { - // Walk children, clone each Tab with a generated id, and each Panel with a - // matching aria-labelledby. Derive in render so we never ship a - // "before-the-effect-ran" empty-tablist frame and so that re-renders of - // UnderlinePanels don't churn through an extra commit cycle. - // - // iconsVisible / loadingCounters are NOT baked into the cloned Tab - // elements — they flow through UnderlinePanelsContext, so this memo's deps - // can stay tight ([children, parentId]) and Tab elements stay - // referentially stable across resize-driven iconsVisible toggles. + 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]. let tabIndex = 0 let panelIndex = 0 const childrenWithProps = Children.map(children, child => { - if (isValidElement>(child) && (child.type === Tab || isSlot(child, Tab))) { - return cloneElement(child, {id: `${parentId}-tab-${tabIndex++}`}) + if (isValidElement(child) && (child.type === Tab || isSlot(child, Tab))) { + return cloneElement(child, {value: `${tabIndex++}`}) } - if (isValidElement(child) && (child.type === Panel || isSlot(child, Panel))) { - const childPanel = child as React.ReactElement - return cloneElement(childPanel, {'aria-labelledby': `${parentId}-tab-${panelIndex++}`}) + if (isValidElement(child) && (child.type === Panel || isSlot(child, Panel))) { + return cloneElement(child, {value: `${panelIndex++}`}) } return child }) const tabs: React.ReactNode[] = [] const tabPanels: React.ReactNode[] = [] + let selectedFromProps: string | undefined for (const child of Children.toArray(childrenWithProps)) { if (!isValidElement(child)) continue - if (child.type === Tab || isSlot(child, Tab)) tabs.push(child) - else if (child.type === Panel || isSlot(child, Panel)) tabPanels.push(child) + if (child.type === Tab || isSlot(child, Tab)) { + const ariaSelected = (child.props as {'aria-selected'?: boolean | string})['aria-selected'] + if (ariaSelected === true || ariaSelected === 'true') { + selectedFromProps = `${tabs.length}` + } + tabs.push(child) + } else if (child.type === Panel || isSlot(child, Panel)) { + tabPanels.push(child) + } } const tabsHaveIcons = tabs.some(tab => React.isValidElement(tab) && tab.props.icon) - return [tabs, tabPanels, tabsHaveIcons] as const - }, [children, parentId]) + return [tabs, tabPanels, tabsHaveIcons, selectedFromProps] as const + }, [children]) + + // 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') + const [prevSelectedFromProps, setPrevSelectedFromProps] = useState(selectedFromProps) + if (selectedFromProps !== prevSelectedFromProps) { + setPrevSelectedFromProps(selectedFromProps) + if (selectedFromProps !== undefined && selectedFromProps !== selectedValue) { + setSelectedValue(selectedFromProps) + } + } + + const {tabListProps} = useTabList({ + 'aria-label': ariaLabel, + 'aria-labelledby': ariaLabelledBy, + ref: listRef, + } as TabListHookProps) const contextValue = useMemo( () => ({iconsVisible, loadingCounters}), @@ -213,75 +230,80 @@ const UnderlinePanels: FCWithSlotMarker = ({ return ( - + setSelectedValue(value)}> - + {tabs} {tabPanels} - + ) } -const TabImpl: FCWithSlotMarker = ({'aria-selected': ariaSelected, onSelect, ...props}) => { +const TabImpl: FC = ({onSelect, value, ...itemProps}) => { const {loadingCounters} = useContext(UnderlinePanelsContext) - const clickHandler = React.useCallback( - (event: React.MouseEvent) => { - if (!event.defaultPrevented && typeof onSelect === 'function') { - onSelect(event) - } - }, - [onSelect], - ) - const keyDownHandler = React.useCallback( - (event: React.KeyboardEvent) => { - if ((event.key === ' ' || event.key === 'Enter') && !event.defaultPrevented && typeof onSelect === 'function') { - onSelect(event) - } - }, - [onSelect], - ) + const {tabProps} = useTab({value: value ?? ''}) + const {onKeyDown: tabOnKeyDown, onMouseDown: tabOnMouseDown, onFocus: tabOnFocus, ...restTabProps} = tabProps return ( ) => { + if (!event.defaultPrevented && typeof onSelect === 'function') { + onSelect(event) + } + }} + onKeyDown={(event: React.KeyboardEvent) => { + tabOnKeyDown?.(event) + if ((event.key === ' ' || event.key === 'Enter') && !event.defaultPrevented && typeof onSelect === 'function') { + onSelect(event) + } + }} + onMouseDown={tabOnMouseDown} + onFocus={tabOnFocus} loadingCounters={loadingCounters} - {...props} /> ) } -// Memoized so that UnderlinePanels re-rendering (e.g. when iconsVisible flips) -// only re-renders Tabs whose own props actually changed. iconsVisible and -// loadingCounters reach Tab via UnderlinePanelsContext, so Tabs still react -// to those changes through context propagation. +// Memoized so an UnderlinePanels re-render (e.g. iconsVisible flipping) only +// re-renders Tabs whose own props changed; iconsVisible/loadingCounters reach +// Tab via context. TabImpl.displayName = 'UnderlinePanels.Tab' const Tab = React.memo(TabImpl) as unknown as FCWithSlotMarker Tab.displayName = 'UnderlinePanels.Tab' -const Panel: FCWithSlotMarker = ({children, ...rest}) => { +const PanelImpl: FC = ({children, value, ...panelRest}) => { + const {tabPanelProps} = useTabPanel({value: value ?? ''}) + return ( -
+
{children}
) } +PanelImpl.displayName = 'UnderlinePanels.Panel' +const Panel = PanelImpl as unknown as FCWithSlotMarker + Panel.displayName = 'UnderlinePanels.Panel' export default Object.assign(UnderlinePanels, {Panel, Tab}) From 4506f8b56733e631f2623455502d3522e1c921e1 Mon Sep 17 00:00:00 2001 From: Ricky Zhang Date: Wed, 22 Jul 2026 10:41:06 -0400 Subject: [PATCH 2/2] Ensure UnderlinePanels.Tab type=button cannot be overridden Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 5b92d807-358b-48fe-b099-5aed24842200 --- .../react/src/experimental/UnderlinePanels/UnderlinePanels.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx index 1688de3aefe..1d0d3022a89 100644 --- a/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx +++ b/packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx @@ -262,9 +262,9 @@ const TabImpl: FC = ({onSelect, value, ...itemProps}) => { return ( ) => { if (!event.defaultPrevented && typeof onSelect === 'function') { onSelect(event)