diff --git a/.changeset/little-hairs-train.md b/.changeset/little-hairs-train.md deleted file mode 100644 index 1725b26816b..00000000000 --- a/.changeset/little-hairs-train.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@primer/react': minor ---- - -Allows up to 4 levels of nesting in the NavList component. diff --git a/docs/content/NavList.mdx b/docs/content/NavList.mdx index fc205fc6f57..3e12b73f7d6 100644 --- a/docs/content/NavList.mdx +++ b/docs/content/NavList.mdx @@ -161,36 +161,6 @@ If a `NavList.Item` contains a `NavList.SubNav`, the `NavList.Item` will render -### With nested sub-items - -```jsx live drafts - - Branches - Tags - - Actions - - - General - - - Runners - - Runner 1 - Runner 2 - - - - - -``` - - - -We only allow for up to 4 levels of nested subnavs. If more than 4 levels is required, it's a good sign that the navigation design needs to be rethought. - - - ### With a divider ```jsx live drafts diff --git a/src/NavList/NavList.stories.tsx b/src/NavList/NavList.stories.tsx index 9af62a887ee..f643302c1f8 100644 --- a/src/NavList/NavList.stories.tsx +++ b/src/NavList/NavList.stories.tsx @@ -64,45 +64,6 @@ export const WithSubItems: Story = () => ( ) -export const WithNestedSubItems: Story = () => ( - - - - Item 1 - - Item 2{/* NOTE: Don't nest SubNavs. For testing purposes only */} - - - Sub item 1 - - - Sub item 1.1 - - Sub item 1.1.1 - - Sub item 1.1.2 - - Sub item 1.1.2.1 - - Sub item 1.1.2.2 - - - - - - Sub item 1.2 - - - Sub item 2 - - - Item 3 - - - - -) - type ReactRouterLikeLinkProps = {to: string; children: React.ReactNode} const ReactRouterLikeLink = React.forwardRef(({to, ...props}, ref) => { // eslint-disable-next-line jsx-a11y/anchor-has-content diff --git a/src/NavList/NavList.test.tsx b/src/NavList/NavList.test.tsx index ee4fa595285..08fe311ae60 100644 --- a/src/NavList/NavList.test.tsx +++ b/src/NavList/NavList.test.tsx @@ -249,7 +249,7 @@ describe('NavList.Item with NavList.SubNav', () => { expect(container).toMatchSnapshot() }) - it('prevents more than 4 levels of nested SubNavs', () => { + it('prevents multiple levels of nested SubNavs', () => { const consoleSpy = jest .spyOn(console, 'error') // Suppress error message in test output @@ -257,45 +257,22 @@ describe('NavList.Item with NavList.SubNav', () => { const {getByRole} = render( - Item 1 - - Item 2{/* NOTE: Don't nest SubNavs. For testing purposes only */} + + Item - - Sub item 1 + + Sub item + {/* NOTE: Don't nest SubNavs. For testing purposes only */} - - Sub item 1.1 - - Sub item 1.1.1 - - Sub item 1.1.2 - - Sub item 1.1.2.1 - - Sub item 1.1.2.2 - - - Sub item 1.1.2.2.1 - - Sub item 1.1.2.2.2 - - - - - - - Sub item 1.2 + Sub sub item - Sub item 2 - Item 3 , ) - const item = getByRole('button', {name: 'Item 2'}) + const item = getByRole('button', {name: 'Item'}) fireEvent.click(item) expect(consoleSpy).toHaveBeenCalled() diff --git a/src/NavList/NavList.tsx b/src/NavList/NavList.tsx index 086ef324715..80550abc989 100644 --- a/src/NavList/NavList.tsx +++ b/src/NavList/NavList.tsx @@ -15,14 +15,6 @@ import {defaultSxProp} from '../utils/defaultSxProp' import {useId} from '../hooks/useId' import useIsomorphicLayoutEffect from '../utils/useIsomorphicLayoutEffect' -const getSubnavStyles = (depth: number) => { - return { - paddingLeft: depth > 0 ? depth + 2 : null, // Indent sub-items - fontSize: depth > 0 ? 0 : null, // Reduce font size of sub-items - fontWeight: depth > 0 ? 'normal' : null, // Sub-items don't get bolded - } -} - // ---------------------------------------------------------------------------- // NavList @@ -65,9 +57,9 @@ const Item = React.forwardRef( ) // Render ItemWithSubNav if SubNav is present - if (subNav && isValidElement(subNav)) { + if (subNav && isValidElement(subNav) && depth < 1) { return ( - + {childrenWithoutSubNav} ) @@ -78,7 +70,14 @@ const Item = React.forwardRef( ref={ref} aria-current={ariaCurrent} active={Boolean(ariaCurrent) && ariaCurrent !== 'false'} - sx={merge(getSubnavStyles(depth), sxProp)} + sx={merge( + { + paddingLeft: depth > 0 ? 5 : null, // Indent sub-items + fontSize: depth > 0 ? 0 : null, // Reduce font size of sub-items + fontWeight: depth > 0 ? 'normal' : null, // Sub-items don't get bolded + }, + sxProp, + )} {...props} > {children} @@ -95,7 +94,6 @@ Item.displayName = 'NavList.Item' type ItemWithSubNavProps = { children: React.ReactNode subNav: React.ReactNode - depth: number } & SxProp const ItemWithSubNavContext = React.createContext<{buttonId: string; subNavId: string; isOpen: boolean}>({ @@ -106,7 +104,7 @@ const ItemWithSubNavContext = React.createContext<{buttonId: string; subNavId: s // TODO: ref prop // TODO: Animate open/close transition -function ItemWithSubNav({children, subNav, depth, sx: sxProp = defaultSxProp}: ItemWithSubNavProps) { +function ItemWithSubNav({children, subNav, sx: sxProp = defaultSxProp}: ItemWithSubNavProps) { const buttonId = useId() const subNavId = useId() const [isOpen, setIsOpen] = React.useState(false) @@ -137,7 +135,6 @@ function ItemWithSubNav({children, subNav, depth, sx: sxProp = defaultSxProp}: I onClick={() => setIsOpen(open => !open)} sx={merge( { - ...getSubnavStyles(depth), fontWeight: containsCurrentItem ? 'bold' : null, // Parent item is bold if any of it's sub-items are current }, sxProp, @@ -181,10 +178,9 @@ const SubNav = ({children, sx: sxProp = defaultSxProp}: NavListSubNavProps) => { console.error('NavList.SubNav must be a child of a NavList.Item') } - if (depth > 3) { - // TODO: write a more informative error message that directs people to rethink their IA + if (depth > 0) { // eslint-disable-next-line no-console - console.error('NavList.SubNav only supports four levels of nesting') + console.error('NavList.SubNav only supports one level of nesting') return null } diff --git a/src/NavList/__snapshots__/NavList.test.tsx.snap b/src/NavList/__snapshots__/NavList.test.tsx.snap index 2fc235e8e21..443240545e4 100644 --- a/src/NavList/__snapshots__/NavList.test.tsx.snap +++ b/src/NavList/__snapshots__/NavList.test.tsx.snap @@ -1005,6 +1005,7 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav display: flex; padding-left: 8px; padding-right: 8px; + font-size: 14px; padding-top: 6px; padding-bottom: 6px; line-height: 20px; @@ -1078,7 +1079,7 @@ exports[`NavList.Item with NavList.SubNav does not have active styles if SubNav color: #0969da; -webkit-text-decoration: none; text-decoration: none; - padding-left: 16px; + padding-left: 32px; padding-right: 8px; padding-top: 6px; padding-bottom: 6px; @@ -1479,6 +1480,7 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t display: flex; padding-left: 8px; padding-right: 8px; + font-size: 14px; padding-top: 6px; padding-bottom: 6px; line-height: 20px; @@ -1558,7 +1560,7 @@ exports[`NavList.Item with NavList.SubNav has active styles if SubNav contains t color: #0969da; -webkit-text-decoration: none; text-decoration: none; - padding-left: 16px; + padding-left: 32px; padding-right: 8px; padding-top: 6px; padding-bottom: 6px;