From fd80a60eeb615b3c4464ade10cde7e26e1349e40 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 9 Jul 2024 10:45:07 -0500 Subject: [PATCH] feat(UnderlineNavItem): add support for icons as React.ReactElement (#4718) * feat(UnderlineNavItem): add support for icons as React.ReactElement * chore: add changeset * fix: update render logic for elements, update stories * chore: update typescript for storybook --------- Co-authored-by: Josh Black --- .changeset/bright-islands-kick.md | 5 ++++ .../UnderlineNav.features.stories.tsx | 20 +++++++------ .../src/UnderlineNav/UnderlineNav.stories.tsx | 7 +++-- .../src/UnderlineNav/UnderlineNav.test.tsx | 28 ++++++++++++++++++- .../src/UnderlineNav/UnderlineNavItem.tsx | 2 +- .../components/UnderlineTabbedInterface.tsx | 9 ++---- 6 files changed, 51 insertions(+), 20 deletions(-) create mode 100644 .changeset/bright-islands-kick.md diff --git a/.changeset/bright-islands-kick.md b/.changeset/bright-islands-kick.md new file mode 100644 index 00000000000..a1cce3b79d9 --- /dev/null +++ b/.changeset/bright-islands-kick.md @@ -0,0 +1,5 @@ +--- +'@primer/react': minor +--- + +Add support for providing icons as an element to UnderlineNavItem diff --git a/packages/react/src/UnderlineNav/UnderlineNav.features.stories.tsx b/packages/react/src/UnderlineNav/UnderlineNav.features.stories.tsx index 3e43bd6fbf7..05818b9749f 100644 --- a/packages/react/src/UnderlineNav/UnderlineNav.features.stories.tsx +++ b/packages/react/src/UnderlineNav/UnderlineNav.features.stories.tsx @@ -16,9 +16,11 @@ import type {Meta} from '@storybook/react' import {UnderlineNav} from './index' import {INITIAL_VIEWPORTS} from '@storybook/addon-viewport' -export default { +const meta = { title: 'Components/UnderlineNav/Features', -} as Meta +} satisfies Meta + +export default meta export const Default = () => { return ( @@ -33,17 +35,17 @@ export const Default = () => { export const WithIcons = () => { return ( - Code - + }>Code + } counter={6}> Issues - + }> Pull Requests - + } counter={7}> Discussions - Projects + }>Projects ) } @@ -51,10 +53,10 @@ export const WithIcons = () => { export const WithCounterLabels = () => { return ( - + } counter="11K"> Code - + } counter={12}> Issues diff --git a/packages/react/src/UnderlineNav/UnderlineNav.stories.tsx b/packages/react/src/UnderlineNav/UnderlineNav.stories.tsx index e225cb5807a..36d02d5e955 100644 --- a/packages/react/src/UnderlineNav/UnderlineNav.stories.tsx +++ b/packages/react/src/UnderlineNav/UnderlineNav.stories.tsx @@ -5,10 +5,9 @@ import {UnderlineNavItem} from './UnderlineNavItem' const excludedControlKeys = ['sx', 'as', 'variant', 'align', 'afterSelect'] -export default { +const meta: Meta = { title: 'Components/UnderlineNav', component: UnderlineNav, - subcomponents: {UnderlineNavItem}, parameters: { controls: { expanded: true, @@ -33,7 +32,9 @@ export default { 'aria-label': 'Repository', loadingCounters: false, }, -} as Meta +} + +export default meta export const Default: StoryFn = () => { const children = ['Code', 'Pull requests', 'Actions', 'Projects', 'Wiki'] diff --git a/packages/react/src/UnderlineNav/UnderlineNav.test.tsx b/packages/react/src/UnderlineNav/UnderlineNav.test.tsx index ae1383d5bab..bd1551a3922 100644 --- a/packages/react/src/UnderlineNav/UnderlineNav.test.tsx +++ b/packages/react/src/UnderlineNav/UnderlineNav.test.tsx @@ -1,5 +1,5 @@ import React from 'react' -import {render} from '@testing-library/react' +import {render, screen} from '@testing-library/react' import userEvent from '@testing-library/user-event' import type {IconProps} from '@primer/octicons-react' import { @@ -77,22 +77,26 @@ describe('UnderlineNav', () => { default: undefined, UnderlineNav, }) + it('renders aria-current attribute to be pages when an item is selected', () => { const {getByRole} = render() const selectedNavLink = getByRole('link', {name: 'Code'}) expect(selectedNavLink.getAttribute('aria-current')).toBe('page') }) + it('renders aria-label attribute correctly', () => { const {container, getByRole} = render() expect(container.getElementsByTagName('nav').length).toEqual(1) const nav = getByRole('navigation') expect(nav.getAttribute('aria-label')).toBe('Repository') }) + it('renders icons correctly', () => { const {getByRole} = render() const nav = getByRole('navigation') expect(nav.getElementsByTagName('svg').length).toEqual(7) }) + it('fires onSelect on click', async () => { const onSelect = jest.fn() const {getByRole} = render( @@ -107,6 +111,7 @@ describe('UnderlineNav', () => { await user.click(item) expect(onSelect).toHaveBeenCalledTimes(1) }) + it('fires onSelect on keypress', async () => { const onSelect = jest.fn() const {getByRole} = render( @@ -128,6 +133,7 @@ describe('UnderlineNav', () => { await user.keyboard(' ') // space expect(onSelect).toHaveBeenCalledTimes(3) }) + it('respects counter prop', () => { const {getByRole} = render() const item = getByRole('link', {name: 'Issues (120)'}) @@ -135,6 +141,7 @@ describe('UnderlineNav', () => { expect(counter.textContent).toBe('120') expect(counter).toHaveAttribute('aria-hidden', 'true') }) + it('renders the content of visually hidden span properly for screen readers', () => { const {getByRole} = render() const item = getByRole('link', {name: 'Issues (120)'}) @@ -142,6 +149,7 @@ describe('UnderlineNav', () => { // non breaking space unified code expect(counter.textContent).toBe('\u00A0(120)') }) + it('respects loadingCounters prop', () => { const {getByRole} = render() const item = getByRole('link', {name: 'Actions'}) @@ -149,6 +157,7 @@ describe('UnderlineNav', () => { expect(loadingCounter.className).toContain('LoadingCounter') expect(loadingCounter.textContent).toBe('') }) + it('renders a visually hidden h2 heading for screen readers when aria-label is present', () => { const {getByRole} = render() const heading = getByRole('heading', {name: 'Repository navigation'}) @@ -157,6 +166,7 @@ describe('UnderlineNav', () => { expect(heading.className).toContain('VisuallyHidden') expect(heading.textContent).toBe('Repository navigation') }) + it('throws an error when there are multiple items that have aria-current', () => { const spy = jest.spyOn(console, 'error').mockImplementation() expect(() => { @@ -186,6 +196,22 @@ describe('UnderlineNav', () => { // We are expecting a left value back, that way we know the `getAnchoredPosition` ran. expect(results).toEqual(expect.objectContaining({left: 0})) }) + + it('should support icons passed in as an element', () => { + render( + + }> + Page one + + }>Page two + }>Page three + , + ) + + expect(screen.getByLabelText('Page one icon')).toBeInTheDocument() + expect(screen.getByLabelText('Page two icon')).toBeInTheDocument() + expect(screen.getByLabelText('Page three icon')).toBeInTheDocument() + }) }) describe('Keyboard Navigation', () => { diff --git a/packages/react/src/UnderlineNav/UnderlineNavItem.tsx b/packages/react/src/UnderlineNav/UnderlineNavItem.tsx index f1b7e74f209..f63c38c270b 100644 --- a/packages/react/src/UnderlineNav/UnderlineNavItem.tsx +++ b/packages/react/src/UnderlineNav/UnderlineNavItem.tsx @@ -38,7 +38,7 @@ export type UnderlineNavItemProps = { /** * Icon before the text */ - icon?: React.FunctionComponent + icon?: React.FunctionComponent | React.ReactElement /** * Renders `UnderlineNav.Item` as given component i.e. react-router's Link **/ diff --git a/packages/react/src/internal/components/UnderlineTabbedInterface.tsx b/packages/react/src/internal/components/UnderlineTabbedInterface.tsx index 38acc6aafd5..b76b6e5c4c5 100644 --- a/packages/react/src/internal/components/UnderlineTabbedInterface.tsx +++ b/packages/react/src/internal/components/UnderlineTabbedInterface.tsx @@ -1,6 +1,7 @@ // Used for UnderlineNav and UnderlinePanels components import React, {forwardRef, type FC, type PropsWithChildren} from 'react' +import {isElement} from 'react-is' import type {IconProps} from '@primer/octicons-react' import styled, {keyframes} from 'styled-components' import CounterLabel from '../../CounterLabel' @@ -193,7 +194,7 @@ export type UnderlineItemProps = { iconsVisible?: boolean loadingCounters?: boolean counter?: number | string - icon?: FC + icon?: FC | React.ReactElement id?: string } & SxProp @@ -213,11 +214,7 @@ export const UnderlineItem = forwardRef( ) => { return ( - {iconsVisible && Icon && ( - - - - )} + {iconsVisible && Icon && {isElement(Icon) ? Icon : }} {children && ( {children}