From 6211c7a61af40c6f0d24c8e140ea81c07eba96fb Mon Sep 17 00:00:00 2001 From: Chad Chadbourne <13856531+chad1008@users.noreply.github.com> Date: Fri, 8 Dec 2023 11:29:51 -0500 Subject: [PATCH] Tabs: replace `id` with new `tabId` prop (#56883) * replace `id` with `tabId` * update stories and tests * update `ColorGradientControl` implementation * changelog * use `tabId` in tests for consistency * update `ColorPanel` implementation --- .../components/colors-gradients/control.js | 8 +- .../components/global-styles/color-panel.js | 4 +- packages/components/CHANGELOG.md | 4 + packages/components/src/tabs/README.md | 8 +- .../src/tabs/stories/index.story.tsx | 96 +++++++++---------- packages/components/src/tabs/tab.tsx | 6 +- packages/components/src/tabs/tabpanel.tsx | 10 +- packages/components/src/tabs/test/index.tsx | 48 +++++----- packages/components/src/tabs/types.ts | 11 ++- 9 files changed, 104 insertions(+), 91 deletions(-) diff --git a/packages/block-editor/src/components/colors-gradients/control.js b/packages/block-editor/src/components/colors-gradients/control.js index 0cb2fcdda44875..cf82510f78c896 100644 --- a/packages/block-editor/src/components/colors-gradients/control.js +++ b/packages/block-editor/src/components/colors-gradients/control.js @@ -141,15 +141,15 @@ function ColorGradientControlInner( { } > - + { __( 'Solid' ) } - + { __( 'Gradient' ) } ( { tab.label } @@ -274,7 +274,7 @@ function ColorPanelDropdown( { return ( = ( props ) => { return ( - Tab 1 - Tab 2 - Tab 3 + Tab 1 + Tab 2 + Tab 3 - +

Selected tab: Tab 1

- +

Selected tab: Tab 2

- +

Selected tab: Tab 3

This tabpanel has its focusable prop set to @@ -71,19 +71,19 @@ const DisabledTabTemplate: StoryFn< typeof Tabs > = ( props ) => { return ( - + Tab 1 - Tab 2 - Tab 3 + Tab 2 + Tab 3 - +

Selected tab: Tab 1

- +

Selected tab: Tab 2

- +

Selected tab: Tab 3

@@ -96,31 +96,31 @@ const WithTabIconsAndTooltipsTemplate: StoryFn< typeof Tabs > = ( props ) => { } /> } /> } /> - +

Selected tab: Tab 1

- +

Selected tab: Tab 2

- +

Selected tab: Tab 3

@@ -140,18 +140,18 @@ const UsingSlotFillTemplate: StoryFn< typeof Tabs > = ( props ) => { - Tab 1 - Tab 2 - Tab 3 + Tab 1 + Tab 2 + Tab 3 - +

Selected tab: Tab 1

- +

Selected tab: Tab 2

- +

Selected tab: Tab 3

@@ -196,9 +196,9 @@ const CloseButtonTemplate: StoryFn< typeof Tabs > = ( props ) => { } } > - Tab 1 - Tab 2 - Tab 3 + Tab 1 + Tab 2 + Tab 3 - +

Selected tab: Tab 1

- +

Selected tab: Tab 2

- +

Selected tab: Tab 3

@@ -251,19 +251,19 @@ const ControlledModeTemplate: StoryFn< typeof Tabs > = ( props ) => { } } > - Tab 1 + Tab 1 - Tab 2 + Tab 2 - Tab 3 + Tab 3 - +

Selected tab: Tab 1

- +

Selected tab: Tab 2

- +

Selected tab: Tab 3

@@ -314,19 +314,19 @@ const TabBecomesDisabledTemplate: StoryFn< typeof Tabs > = ( props ) => { - Tab 1 - + Tab 1 + Tab 2 - Tab 3 + Tab 3 - +

Selected tab: Tab 1

- +

Selected tab: Tab 2

- +

Selected tab: Tab 3

@@ -348,17 +348,17 @@ const TabGetsRemovedTemplate: StoryFn< typeof Tabs > = ( props ) => { - { ! removeTab1 && Tab 1 } - Tab 2 - Tab 3 + { ! removeTab1 && Tab 1 } + Tab 2 + Tab 3 - +

Selected tab: Tab 1

- +

Selected tab: Tab 2

- +

Selected tab: Tab 3

diff --git a/packages/components/src/tabs/tab.tsx b/packages/components/src/tabs/tab.tsx index 4bfc99e8ef43b1..e1aa85c636cdd1 100644 --- a/packages/components/src/tabs/tab.tsx +++ b/packages/components/src/tabs/tab.tsx @@ -15,15 +15,15 @@ import type { WordPressComponentProps } from '../context'; export const Tab = forwardRef< HTMLButtonElement, - WordPressComponentProps< TabProps, 'button', false > ->( function Tab( { children, id, disabled, render, ...otherProps }, ref ) { + Omit< WordPressComponentProps< TabProps, 'button', false >, 'id' > +>( function Tab( { children, tabId, disabled, render, ...otherProps }, ref ) { const context = useTabsContext(); if ( ! context ) { warning( '`Tabs.Tab` must be wrapped in a `Tabs` component.' ); return null; } const { store, instanceId } = context; - const instancedTabId = `${ instanceId }-${ id }`; + const instancedTabId = `${ instanceId }-${ tabId }`; return ( ->( function TabPanel( { children, id, focusable = true, ...otherProps }, ref ) { + Omit< WordPressComponentProps< TabPanelProps, 'div', false >, 'id' > +>( function TabPanel( + { children, tabId, focusable = true, ...otherProps }, + ref +) { const context = useTabsContext(); if ( ! context ) { warning( '`Tabs.TabPanel` must be wrapped in a `Tabs` component.' ); return null; } const { store, instanceId } = context; + const instancedTabId = `${ instanceId }-${ tabId }`; return ( diff --git a/packages/components/src/tabs/test/index.tsx b/packages/components/src/tabs/test/index.tsx index f923dc455fd7bd..7e2d4671224858 100644 --- a/packages/components/src/tabs/test/index.tsx +++ b/packages/components/src/tabs/test/index.tsx @@ -16,7 +16,7 @@ import Tabs from '..'; import type { TabsProps } from '../types'; type Tab = { - id: string; + tabId: string; title: string; content: React.ReactNode; tab: { @@ -30,19 +30,19 @@ type Tab = { const TABS: Tab[] = [ { - id: 'alpha', + tabId: 'alpha', title: 'Alpha', content: 'Selected tab: Alpha', tab: { className: 'alpha-class' }, }, { - id: 'beta', + tabId: 'beta', title: 'Beta', content: 'Selected tab: Beta', tab: { className: 'beta-class' }, }, { - id: 'gamma', + tabId: 'gamma', title: 'Gamma', content: 'Selected tab: Gamma', tab: { className: 'gamma-class' }, @@ -52,7 +52,7 @@ const TABS: Tab[] = [ const TABS_WITH_DELTA: Tab[] = [ ...TABS, { - id: 'delta', + tabId: 'delta', title: 'Delta', content: 'Selected tab: Delta', tab: { className: 'delta-class' }, @@ -70,8 +70,8 @@ const UncontrolledTabs = ( { { tabs.map( ( tabObj ) => ( @@ -81,8 +81,8 @@ const UncontrolledTabs = ( { { tabs.map( ( tabObj ) => ( { tabObj.content } @@ -114,8 +114,8 @@ const ControlledTabs = ( { { tabs.map( ( tabObj ) => ( @@ -124,7 +124,7 @@ const ControlledTabs = ( { ) ) } { tabs.map( ( tabObj ) => ( - + { tabObj.content } ) ) } @@ -201,7 +201,7 @@ describe( 'Tabs', () => { } ); it( 'should not focus on the related TabPanel when pressing the Tab key if `focusable: false` is set', async () => { const TABS_WITH_ALPHA_FOCUSABLE_FALSE = TABS.map( ( tabObj ) => - tabObj.id === 'alpha' + tabObj.tabId === 'alpha' ? { ...tabObj, content: ( @@ -442,7 +442,7 @@ describe( 'Tabs', () => { const mockOnSelect = jest.fn(); const TABS_WITH_DELTA_DISABLED = TABS_WITH_DELTA.map( ( tabObj ) => - tabObj.id === 'delta' + tabObj.tabId === 'delta' ? { ...tabObj, tab: { @@ -604,7 +604,7 @@ describe( 'Tabs', () => { } ); it( 'should not load any tab if the active tab is removed and there are no enabled tabs', async () => { const TABS_WITH_BETA_GAMMA_DISABLED = TABS.map( ( tabObj ) => - tabObj.id !== 'alpha' + tabObj.tabId !== 'alpha' ? { ...tabObj, tab: { @@ -726,7 +726,7 @@ describe( 'Tabs', () => { expect( mockOnSelect ).toHaveBeenLastCalledWith( 'alpha' ); const TABS_WITH_ALPHA_DISABLED = TABS.map( ( tabObj ) => - tabObj.id === 'alpha' + tabObj.tabId === 'alpha' ? { ...tabObj, tab: { @@ -801,7 +801,7 @@ describe( 'Tabs', () => { const TABS_WITH_DELTA_DISABLED = TABS_WITH_DELTA.map( ( tabObj ) => - tabObj.id === 'delta' + tabObj.tabId === 'delta' ? { ...tabObj, tab: { @@ -849,7 +849,7 @@ describe( 'Tabs', () => { it( 'should select first enabled tab when the initial tab is disabled', async () => { const TABS_WITH_ALPHA_DISABLED = TABS.map( ( tabObj ) => - tabObj.id === 'alpha' + tabObj.tabId === 'alpha' ? { ...tabObj, tab: { @@ -878,7 +878,7 @@ describe( 'Tabs', () => { it( 'should select first enabled tab when the tab associated to `initialTabId` is disabled', async () => { const TABS_ONLY_GAMMA_ENABLED = TABS.map( ( tabObj ) => - tabObj.id !== 'gamma' + tabObj.tabId !== 'gamma' ? { ...tabObj, tab: { @@ -920,7 +920,7 @@ describe( 'Tabs', () => { expect( mockOnSelect ).toHaveBeenLastCalledWith( 'alpha' ); const TABS_WITH_ALPHA_DISABLED = TABS.map( ( tabObj ) => - tabObj.id === 'alpha' + tabObj.tabId === 'alpha' ? { ...tabObj, tab: { @@ -967,7 +967,7 @@ describe( 'Tabs', () => { expect( await getSelectedTab() ).toHaveTextContent( 'Gamma' ); const TABS_WITH_GAMMA_DISABLED = TABS.map( ( tabObj ) => - tabObj.id === 'gamma' + tabObj.tabId === 'gamma' ? { ...tabObj, tab: { @@ -1051,7 +1051,7 @@ describe( 'Tabs', () => { // Remove beta rerender( tab.id !== 'beta' ) } + tabs={ TABS.filter( ( tab ) => tab.tabId !== 'beta' ) } selectedTabId="beta" /> ); @@ -1085,7 +1085,7 @@ describe( 'Tabs', () => { it( 'should not render any tab if `selectedTabId` refers to a disabled tab', async () => { const TABS_WITH_DELTA_WITH_BETA_DISABLED = TABS_WITH_DELTA.map( ( tabObj ) => - tabObj.id === 'beta' + tabObj.tabId === 'beta' ? { ...tabObj, tab: { @@ -1122,7 +1122,7 @@ describe( 'Tabs', () => { expect( await getSelectedTab() ).toHaveTextContent( 'Beta' ); const TABS_WITH_BETA_DISABLED = TABS.map( ( tabObj ) => - tabObj.id === 'beta' + tabObj.tabId === 'beta' ? { ...tabObj, tab: { diff --git a/packages/components/src/tabs/types.ts b/packages/components/src/tabs/types.ts index 8b071937410919..389665b13357fe 100644 --- a/packages/components/src/tabs/types.ts +++ b/packages/components/src/tabs/types.ts @@ -78,8 +78,10 @@ export type TabListProps = { export type TabProps = { /** * The id of the tab, which is prepended with the `Tabs` instanceId. + * The value of this prop should match with the value of the `tabId` prop on + * the corresponding `Tabs.TabPanel` component. */ - id: string; + tabId: string; /** * The children elements, generally the text to display on the tab. */ @@ -103,9 +105,12 @@ export type TabPanelProps = { */ children?: React.ReactNode; /** - * A unique identifier for the tabpanel, which is used to generate a unique `id` for the underlying element. + * A unique identifier for the tabpanel, which is used to generate an + * instanced id for the underlying element. + * The value of this prop should match with the value of the `tabId` prop on + * the corresponding `Tabs.Tab` component. */ - id: string; + tabId: string; /** * Determines whether or not the tabpanel element should be focusable. * If `false`, pressing the tab key will skip over the tabpanel, and instead