Skip to content

Commit

Permalink
feat(Tabs): add renderIcon prop (#13427)
Browse files Browse the repository at this point in the history
* feat(Tabs): add renderIcon prop

* fix(Tabs): line height issue with icon only tab

* test: update public API snapshot

* fix(Tabs): use sentence case for tabs naming

* fix(Tabs): fix 1px icon issue and simplify tab icon code

* test(Tabs): remove unnecessary test

* fix(tabs): re-add line-height zero for icon

* fix: merge with main

---------

Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
  • Loading branch information
francinelucca and kodiakhq[bot] authored Apr 11, 2023
1 parent 8fefe14 commit e3eff64
Show file tree
Hide file tree
Showing 6 changed files with 269 additions and 53 deletions.
28 changes: 27 additions & 1 deletion e2e/components/Tabs/Tabs-test.e2e.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,33 @@ test.describe('Tabs', () => {
test('contained with secondary labels @vrt', async ({ page }) => {
await snapshotStory(page, {
component: 'Tabs',
id: 'components-tabs--contained-with-secondary-label',
id: 'components-tabs--contained-with-secondary-labels',
theme,
});
});

test('with icons @vrt', async ({ page }) => {
await snapshotStory(page, {
component: 'Tabs',
id: 'components-tabs--with-icons',
theme,
});
});

test('contained with icons @vrt', async ({ page }) => {
await snapshotStory(page, {
component: 'Tabs',
id: 'components-tabs--contained-with-icons',
theme,
});
});

test('contained with secondary labels and icons @vrt', async ({
page,
}) => {
await snapshotStory(page, {
component: 'Tabs',
id: 'components-tabs--contained-with-secondary-labels-and-icons',
theme,
});
});
Expand Down
16 changes: 13 additions & 3 deletions packages/react/__tests__/__snapshots__/PublicAPI-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -7193,9 +7193,6 @@ Map {
"disabled": Object {
"type": "bool",
},
"hasSecondaryLabel": Object {
"type": "bool",
},
"onClick": Object {
"type": "func",
},
Expand All @@ -7205,6 +7202,19 @@ Map {
"renderButton": Object {
"type": "func",
},
"renderIcon": Object {
"args": Array [
Array [
Object {
"type": "func",
},
Object {
"type": "object",
},
],
],
"type": "oneOfType",
},
"secondaryLabel": Object {
"type": "string",
},
Expand Down
48 changes: 38 additions & 10 deletions packages/react/src/components/Tabs/Tabs-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ describe('Tabs', () => {
<Tabs defaultSelectedIndex={1}>
<TabList aria-label="List of tabs">
<Tab>Tab Label 1</Tab>
<Tab>Tab Label 2</Tab>
<Tab data-testid="tab-testid">Tab Label 2</Tab>
<Tab>Tab Label 3</Tab>
</TabList>
<TabPanels>
Expand All @@ -21,7 +21,7 @@ describe('Tabs', () => {
</Tabs>
);

expect(screen.getByText('Tab Label 2').parentElement).toHaveAttribute(
expect(screen.getByTestId('tab-testid')).toHaveAttribute(
'aria-selected',
'true'
);
Expand Down Expand Up @@ -56,7 +56,9 @@ describe('Tab', () => {
<Tabs>
<TabList aria-label="List of tabs">
<Tab disabled>Tab Label 1</Tab>
<Tab className="custom-class">Tab Label 2</Tab>
<Tab data-testid="tab-testid" className="custom-class">
Tab Label 2
</Tab>
<Tab>Tab Label 3</Tab>
</TabList>
<TabPanels>
Expand All @@ -67,17 +69,17 @@ describe('Tab', () => {
</Tabs>
);

expect(screen.getByText('Tab Label 2').parentElement).toHaveClass(
'custom-class'
);
expect(screen.getByTestId('tab-testid')).toHaveClass('custom-class');
});

it('should not select a disabled tab and select next tab', () => {
render(
<Tabs>
<TabList aria-label="List of tabs">
<Tab disabled>Tab Label 1</Tab>
<Tab>Tab Label 2</Tab>
<Tab data-testid="tab-testid-1" disabled>
Tab Label 1
</Tab>
<Tab data-testid="tab-testid-2">Tab Label 2</Tab>
<Tab>Tab Label 3</Tab>
</TabList>
<TabPanels>
Expand All @@ -88,13 +90,13 @@ describe('Tab', () => {
</Tabs>
);

expect(screen.getByText('Tab Label 1').parentElement).toHaveAttribute(
expect(screen.getByTestId('tab-testid-1')).toHaveAttribute(
'aria-selected',
'false'
);

// By default, if a Tab is disabled, the next Tab should be selected
expect(screen.getByText('Tab Label 2').parentElement).toHaveAttribute(
expect(screen.getByTestId('tab-testid-2')).toHaveAttribute(
'aria-selected',
'true'
);
Expand Down Expand Up @@ -158,6 +160,32 @@ describe('Tab', () => {
expect(screen.queryByText('test-secondary-label')).not.toBeInTheDocument();
});

it('should display an icon from renderIcon prop', async () => {
render(
<Tabs>
<TabList aria-label="List of tabs">
<Tab renderIcon={() => <svg data-testid="svg" />} disabled>
Tab Label 1
</Tab>
<Tab data-testid="tab-testid" className="custom-class">
Tab Label 2
</Tab>
<Tab>Tab Label 3</Tab>
</TabList>
<TabPanels>
<TabPanel>Tab Panel 1</TabPanel>
<TabPanel>Tab Panel 2</TabPanel>
<TabPanel>Tab Panel 3</TabPanel>
</TabPanels>
</Tabs>
);

expect(screen.getByTestId('svg')).toBeInTheDocument();
expect(screen.getByTestId('svg').parentElement).toHaveClass(
'cds--tabs__nav-item--icon'
);
});

it('should call onClick from props if provided', async () => {
const onClick = jest.fn();
render(
Expand Down
33 changes: 22 additions & 11 deletions packages/react/src/components/Tabs/Tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -345,12 +345,12 @@ function TabList({
onKeyDown={onKeyDown}>
{React.Children.map(children, (child, index) => {
return (
<TabContext.Provider value={index}>
<TabContext.Provider
value={{ index, hasSecondaryLabel: hasSecondaryLabelTabs }}>
{React.cloneElement(child, {
ref: (node) => {
tabs.current[index] = node;
},
hasSecondaryLabel: hasSecondaryLabelTabs,
})}
</TabContext.Provider>
);
Expand Down Expand Up @@ -489,15 +489,15 @@ const Tab = React.forwardRef(function Tab(
onClick,
onKeyDown,
secondaryLabel,
hasSecondaryLabel,
renderIcon: Icon,
...rest
},
ref
) {
const prefix = usePrefix();
const { selectedIndex, setSelectedIndex, baseId } =
React.useContext(TabsContext);
const index = React.useContext(TabContext);
const { index, hasSecondaryLabel } = React.useContext(TabContext);
const id = `${baseId}-tab-${index}`;
const panelId = `${baseId}-tabpanel-${index}`;
const className = cx(
Expand Down Expand Up @@ -533,7 +533,14 @@ const Tab = React.forwardRef(function Tab(
onKeyDown={onKeyDown}
tabIndex={selectedIndex === index ? '0' : '-1'}
type="button">
<span className={`${prefix}--tabs__nav-item-label`}>{children}</span>
<div className={`${prefix}--tabs__nav-item-label-wrapper`}>
<span className={`${prefix}--tabs__nav-item-label`}>{children}</span>
{Icon && (
<div className={`${prefix}--tabs__nav-item--icon`}>
<Icon size={16} />
</div>
)}
</div>
{hasSecondaryLabel && (
<div className={`${prefix}--tabs__nav-item-secondary-label`}>
{secondaryLabel}
Expand Down Expand Up @@ -564,11 +571,6 @@ Tab.propTypes = {
*/
disabled: PropTypes.bool,

/*
* Internal use only, determines wether a tab should render as a secondary label tab
**/
hasSecondaryLabel: PropTypes.bool,

/**
* Provide a handler that is invoked when a user clicks on the control
*/
Expand All @@ -586,6 +588,12 @@ Tab.propTypes = {
**/
renderButton: PropTypes.func,

/**
* Optional prop to render an icon next to the label.
* Can be a React component class
*/
renderIcon: PropTypes.oneOfType([PropTypes.func, PropTypes.object]),

/*
* An optional label to render under the primary tab label.
/* This prop is only useful for conained tabs
Expand All @@ -607,7 +615,10 @@ const IconTab = React.forwardRef(function IconTab(
) {
const prefix = usePrefix();

const classNames = cx(`${prefix}--tabs__nav-item--icon`, customClassName);
const classNames = cx(
`${prefix}--tabs__nav-item--icon-only`,
customClassName
);
return (
<Tooltip
align="bottom"
Expand Down
Loading

0 comments on commit e3eff64

Please sign in to comment.