Revert "Allow up to 4 levels of nesting for the NavList (#3343)"

This reverts commit 786013e.
primer · Jun 12, 2023 · f316432 · f316432
1 parent bec5827
commit f316432
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 124 deletions.
diff --git a/.changeset/little-hairs-train.md b/.changeset/little-hairs-train.md
diff --git 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
 
 </Note>
 
-### With nested sub-items
-
-```jsx live drafts
-<NavList>
-  <NavList.Item href="/branches">Branches</NavList.Item>
-  <NavList.Item href="/tags">Tags</NavList.Item>
-  <NavList.Item>
-    Actions
-    <NavList.SubNav>
-      <NavList.Item href="/actions" aria-current="page">
-        General
-      </NavList.Item>
-      <NavList.Item href="/actions/runners">
-        Runners
-        <NavList.SubNav>
-          <NavList.Item href="/actions/runners/runner-1">Runner 1</NavList.Item>
-          <NavList.Item href="/actions/runners/runner-2">Runner 2</NavList.Item>
-        </NavList.SubNav>
-      </NavList.Item>
-    </NavList.SubNav>
-  </NavList.Item>
-</NavList>
-```
-
-<Note variant="warning">
-
-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.
-
-</Note>
-
 ### With a divider
 
 ```jsx live drafts

diff --git a/src/NavList/NavList.stories.tsx b/src/NavList/NavList.stories.tsx
@@ -64,45 +64,6 @@ export const WithSubItems: Story = () => (
   </PageLayout>
 )
 
-export const WithNestedSubItems: Story = () => (
-  <PageLayout>
-    <PageLayout.Pane position="start">
-      <NavList>
-        <NavList.Item href="#">Item 1</NavList.Item>
-        <NavList.Item href="#">
-          Item 2{/* NOTE: Don't nest SubNavs. For testing purposes only */}
-          <NavList.SubNav>
-            <NavList.Item href="#">
-              Sub item 1
-              <NavList.SubNav>
-                <NavList.Item href="#">
-                  Sub item 1.1
-                  <NavList.SubNav>
-                    <NavList.Item href="#">Sub item 1.1.1</NavList.Item>
-                    <NavList.Item href="#">
-                      Sub item 1.1.2
-                      <NavList.SubNav>
-                        <NavList.Item href="#">Sub item 1.1.2.1</NavList.Item>
-                        <NavList.Item href="#" aria-current="page">
-                          Sub item 1.1.2.2
-                        </NavList.Item>
-                      </NavList.SubNav>
-                    </NavList.Item>
-                  </NavList.SubNav>
-                </NavList.Item>
-                <NavList.Item href="#">Sub item 1.2</NavList.Item>
-              </NavList.SubNav>
-            </NavList.Item>
-            <NavList.Item href="#">Sub item 2</NavList.Item>
-          </NavList.SubNav>
-        </NavList.Item>
-        <NavList.Item href="#">Item 3</NavList.Item>
-      </NavList>
-    </PageLayout.Pane>
-    <PageLayout.Content></PageLayout.Content>
-  </PageLayout>
-)
-
 type ReactRouterLikeLinkProps = {to: string; children: React.ReactNode}
 const ReactRouterLikeLink = React.forwardRef<HTMLAnchorElement, ReactRouterLikeLinkProps>(({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
@@ -249,53 +249,30 @@ 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
       .mockImplementation(() => null)
 
     const {getByRole} = render(
       <NavList>
-        <NavList.Item href="#">Item 1</NavList.Item>
-        <NavList.Item href="#">
-          Item 2{/* NOTE: Don't nest SubNavs. For testing purposes only */}
+        <NavList.Item>
+          Item
           <NavList.SubNav>
-            <NavList.Item href="#">
-              Sub item 1
+            <NavList.Item>
+              Sub item
+              {/* NOTE: Don't nest SubNavs. For testing purposes only */}
               <NavList.SubNav>
-                <NavList.Item href="#">
-                  Sub item 1.1
-                  <NavList.SubNav>
-                    <NavList.Item href="#">Sub item 1.1.1</NavList.Item>
-                    <NavList.Item href="#">
-                      Sub item 1.1.2
-                      <NavList.SubNav>
-                        <NavList.Item href="#">Sub item 1.1.2.1</NavList.Item>
-                        <NavList.Item href="#">
-                          Sub item 1.1.2.2
-                          <NavList.SubNav>
-                            <NavList.Item href="#" aria-current="page">
-                              Sub item 1.1.2.2.1
-                            </NavList.Item>
-                            <NavList.Item href="#">Sub item 1.1.2.2.2</NavList.Item>
-                          </NavList.SubNav>
-                        </NavList.Item>
-                      </NavList.SubNav>
-                    </NavList.Item>
-                  </NavList.SubNav>
-                </NavList.Item>
-                <NavList.Item href="#">Sub item 1.2</NavList.Item>
+                <NavList.Item href="#">Sub sub item</NavList.Item>
               </NavList.SubNav>
             </NavList.Item>
-            <NavList.Item href="#">Sub item 2</NavList.Item>
           </NavList.SubNav>
         </NavList.Item>
-        <NavList.Item href="#">Item 3</NavList.Item>
       </NavList>,
     )
 
-    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
@@ -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<HTMLAnchorElement, NavListItemProps>(
     )
 
     // Render ItemWithSubNav if SubNav is present
-    if (subNav && isValidElement(subNav)) {
+    if (subNav && isValidElement(subNav) && depth < 1) {
       return (
-        <ItemWithSubNav subNav={subNav} depth={depth} sx={sxProp}>
+        <ItemWithSubNav subNav={subNav} sx={sxProp}>
           {childrenWithoutSubNav}
         </ItemWithSubNav>
       )
@@ -78,7 +70,14 @@ const Item = React.forwardRef<HTMLAnchorElement, NavListItemProps>(
         ref={ref}
         aria-current={ariaCurrent}
         active={Boolean(ariaCurrent) && ariaCurrent !== 'false'}
-        sx={merge<SxProp['sx']>(getSubnavStyles(depth), sxProp)}
+        sx={merge<SxProp['sx']>(
+          {
+            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<SxProp['sx']>(
             {
-              ...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
@@ -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;