diff --git a/.changeset/little-hairs-train.md b/.changeset/little-hairs-train.md new file mode 100644 index 0000000000..1725b26816 --- /dev/null +++ b/.changeset/little-hairs-train.md @@ -0,0 +1,5 @@ +--- +'@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 3e12b73f7d..fc205fc6f5 100644 --- a/docs/content/NavList.mdx +++ b/docs/content/NavList.mdx @@ -161,6 +161,36 @@ 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 f643302c1f..9af62a887e 100644 --- a/src/NavList/NavList.stories.tsx +++ b/src/NavList/NavList.stories.tsx @@ -64,6 +64,45 @@ 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 08fe311ae6..ee4fa59528 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 multiple levels of nested SubNavs', () => { + it('prevents more than 4 levels of nested SubNavs', () => { const consoleSpy = jest .spyOn(console, 'error') // Suppress error message in test output @@ -257,22 +257,45 @@ describe('NavList.Item with NavList.SubNav', () => { const {getByRole} = render( - - Item + Item 1 + + Item 2{/* NOTE: Don't nest SubNavs. For testing purposes only */} - - Sub item - {/* NOTE: Don't nest SubNavs. For testing purposes only */} + + Sub item 1 - Sub sub item + + 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 item 2 + Item 3 , ) - const item = getByRole('button', {name: 'Item'}) + const item = getByRole('button', {name: 'Item 2'}) fireEvent.click(item) expect(consoleSpy).toHaveBeenCalled() diff --git a/src/NavList/NavList.tsx b/src/NavList/NavList.tsx index 80550abc98..086ef32471 100644 --- a/src/NavList/NavList.tsx +++ b/src/NavList/NavList.tsx @@ -15,6 +15,14 @@ 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 @@ -57,9 +65,9 @@ const Item = React.forwardRef( ) // Render ItemWithSubNav if SubNav is present - if (subNav && isValidElement(subNav) && depth < 1) { + if (subNav && isValidElement(subNav)) { return ( - + {childrenWithoutSubNav} ) @@ -70,14 +78,7 @@ const Item = React.forwardRef( ref={ref} aria-current={ariaCurrent} active={Boolean(ariaCurrent) && ariaCurrent !== 'false'} - 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, - )} + sx={merge(getSubnavStyles(depth), sxProp)} {...props} > {children} @@ -94,6 +95,7 @@ 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}>({ @@ -104,7 +106,7 @@ const ItemWithSubNavContext = React.createContext<{buttonId: string; subNavId: s // TODO: ref prop // TODO: Animate open/close transition -function ItemWithSubNav({children, subNav, sx: sxProp = defaultSxProp}: ItemWithSubNavProps) { +function ItemWithSubNav({children, subNav, depth, sx: sxProp = defaultSxProp}: ItemWithSubNavProps) { const buttonId = useId() const subNavId = useId() const [isOpen, setIsOpen] = React.useState(false) @@ -135,6 +137,7 @@ function ItemWithSubNav({children, subNav, sx: sxProp = defaultSxProp}: ItemWith 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, @@ -178,9 +181,10 @@ const SubNav = ({children, sx: sxProp = defaultSxProp}: NavListSubNavProps) => { console.error('NavList.SubNav must be a child of a NavList.Item') } - if (depth > 0) { + if (depth > 3) { + // TODO: write a more informative error message that directs people to rethink their IA // eslint-disable-next-line no-console - console.error('NavList.SubNav only supports one level of nesting') + console.error('NavList.SubNav only supports four levels of nesting') return null } diff --git a/src/NavList/__snapshots__/NavList.test.tsx.snap b/src/NavList/__snapshots__/NavList.test.tsx.snap index 443240545e..2fc235e8e2 100644 --- a/src/NavList/__snapshots__/NavList.test.tsx.snap +++ b/src/NavList/__snapshots__/NavList.test.tsx.snap @@ -1005,7 +1005,6 @@ 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; @@ -1079,7 +1078,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: 32px; + padding-left: 16px; padding-right: 8px; padding-top: 6px; padding-bottom: 6px; @@ -1480,7 +1479,6 @@ 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; @@ -1560,7 +1558,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: 32px; + padding-left: 16px; padding-right: 8px; padding-top: 6px; padding-bottom: 6px;