fix(ui): fix invalid HTML structure in SideNavigation component tree

packages/ui-components/src/components/SideNavigation/SideNavigation.stories.tsx

@@ -60,20 +60,18 @@ export const NavigationWithGroups: Story = {
   render: (args) => (
     <SideNavigation {...args}>
       <SideNavigationList>
-        <SideNavigationItem label="Item 1" selected={true} href="#" />
-        <SideNavigationGroup label="Item 2">
-          <SideNavigationItem label="Sub-Child 1" />
-          <SideNavigationGroup label="Sub-Child 2">
-            <SideNavigationItem label="Sub-Child 3" />
-          </SideNavigationGroup>
+        <SideNavigationGroup label="Group 1" open>
+          <SideNavigationItem label="Grouped-Item 1" />
+          <SideNavigationItem label="Grouped-Item 2">
+            <SideNavigationItem label="Grouped-Item 3" />
+          </SideNavigationItem>
         </SideNavigationGroup>
-        <SideNavigationItem label="Item 3" href="#" />
-        <SideNavigationGroup label="Group Example">
+        <SideNavigationGroup label="Group 2" open>
           <SideNavigationItem label="Grouped Item 1" />
-          <SideNavigationGroup label="Grouped Item 2">
+          <SideNavigationItem label="Grouped Item 2">
             <SideNavigationItem label="Sub-Child 1" />
             <SideNavigationItem label="Sub-Child 2" />
-          </SideNavigationGroup>
+          </SideNavigationItem>
         </SideNavigationGroup>
       </SideNavigationList>
     </SideNavigation>

packages/ui-components/src/components/SideNavigation/sidenavigation.css

@@ -19,6 +19,21 @@
 
 /* SideNavigationItem */
 
+/* This selector addresses the actual list element of a group and should become .juno-sidenavigation-item once we have renamed the below classes for the buttons */
+
+.juno-sidenavigation-group-element:not(:first-child) {
+  margin-top: 1rem;
+}
+
+/* Nested lists inside items/groups must not show default list styling. */
+.juno-sidenavigation ul {
+  list-style: none;
+  padding: 0;
+  margin: 0;
+}
+
+/* The below styles should go to the button element and renamed accordingly: */
+
 .juno-sidenavigation-item {
   border-radius: 0.25rem;
   border-left: 0.25rem solid transparent;

packages/ui-components/src/components/SideNavigationGroup/SideNavigationGroup.component.tsx

@@ -3,10 +3,9 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-import React, { Children, ReactElement, ReactNode, useContext, useEffect, useState, MouseEvent } from "react"
+import React, { Children, ReactElement, ReactNode, useEffect, useState, MouseEvent } from "react"
 import { Icon } from "../Icon"
 import { SideNavigationItemProps } from "../SideNavigationItem"
-import { LevelContext } from "../SideNavigation/levelContext"
 import "../SideNavigation/sidenavigation.css"
 
 const sideNavGroupStyles = `
@@ -19,6 +18,7 @@ const sideNavGroupStyles = `
   jn:rounded
   jn:border-l-[0.25rem]
   jn:border-transparent
+  jn:text-sm
 `
 
 const interactiveGroupStyles = `
@@ -68,8 +68,6 @@ export interface SideNavigationGroupProps {
 
 export const SideNavigationGroup = ({ children, label = "", open = false }: SideNavigationGroupProps): ReactNode => {
   const [isOpen, setIsOpen] = useState(open)
-  const level = useContext(LevelContext)
-  const levelClassName = `level-${level + 1}`
 
   // Sync internal state with external prop changes
   useEffect(() => {
@@ -94,7 +92,7 @@ export const SideNavigationGroup = ({ children, label = "", open = false }: Side
 
   const renderLabel = () => (
     <span className={labelContainerStyles}>
-      <span className={`${labelClampStyles} ${levelClassName}`}>{label}</span>
+      <span className={`${labelClampStyles}`}>{label}</span>
     </span>
   )
 
@@ -124,9 +122,9 @@ export const SideNavigationGroup = ({ children, label = "", open = false }: Side
   }
 
   return (
-    <>
+    <li className="juno-sidenavigation-group-element">
       {renderGroup()}
-      {isOpen && <LevelContext.Provider value={level + 1}>{children}</LevelContext.Provider>}
-    </>
+      {isOpen && hasChildren && <ul>{children}</ul>}
+    </li>
   )
 }

packages/ui-components/src/components/SideNavigationGroup/SideNavigationGroup.stories.tsx

@@ -35,11 +35,16 @@ type Story = StoryObj<typeof meta>
 export const Default: Story = {
   args: {
     label: "Group",
+    children: [
+      <SideNavigationItem key="1" label="Item 1" href="#" />,
+      <SideNavigationItem key="2" label="Item 2" href="#" />,
+      <SideNavigationItem key="3" label="Item 3" href="#" />,
+    ],
   },
   parameters: {
     docs: {
       description: {
-        story: "Displays a simple SideNavigationGroup without children, useful for organizing items.",
+        story: "Displays a simple SideNavigationGroup with a few items, useful for organizing items.",
       },
     },
   },
@@ -50,11 +55,11 @@ export const Expandable: Story = {
     label: "Expandable Group",
     children: (
       <>
-        <SideNavigationItem label="1st Level Item" href="#" icon="addCircle" />
-        <SideNavigationItem label="Nested" icon="addCircle">
-          <SideNavigationItem label="2nd Level Item" icon="addCircle">
-            <SideNavigationItem label="3rd Level Item" href="#" icon="addCircle" />
-            <SideNavigationItem label="4th Level Item" href="#" icon="addCircle" />
+        <SideNavigationItem label="1st Level Item" href="#" />
+        <SideNavigationItem label="Nested">
+          <SideNavigationItem label="2nd Level Item">
+            <SideNavigationItem label="3rd Level Item" href="#" />
+            <SideNavigationItem label="4th Level Item" href="#" />
           </SideNavigationItem>
         </SideNavigationItem>
       </>

packages/ui-components/src/components/SideNavigationGroup/SideNavigationGroup.test.tsx

@@ -103,31 +103,52 @@ describe("SideNavigationGroup", () => {
     expect(group).not.toHaveAttribute("title")
   })
 
-  test("indents the group label based on its nesting level", () => {
+  test("does not apply a level-* class to the group label", () => {
     render(
       <SideNavigationGroup label="Top" open>
-        <SideNavigationGroup label="Middle" open>
-          <SideNavigationGroup label="Inner" open>
-            <SideNavigationItem label="Leaf" />
-          </SideNavigationGroup>
-        </SideNavigationGroup>
+        <SideNavigationItem label="Leaf" />
       </SideNavigationGroup>
     )
 
-    expect(screen.getByText("Top")).toHaveClass("level-1")
-    expect(screen.getByText("Middle")).toHaveClass("level-2")
-    expect(screen.getByText("Inner")).toHaveClass("level-3")
+    const label = screen.getByText("Top")
+    expect(label.className).not.toMatch(/\blevel-\d+\b/)
   })
 
-  test("propagates its level so child SideNavigationItems indent correctly", () => {
+  test("does not increment the level for its children (groups are top-level only)", () => {
     render(
       <SideNavigationGroup label="Outer" open>
-        <SideNavigationGroup label="Inner" open>
-          <SideNavigationItem label="Leaf" />
-        </SideNavigationGroup>
+        <SideNavigationItem label="Leaf" />
       </SideNavigationGroup>
     )
 
-    expect(screen.getByText("Leaf")).toHaveClass("level-3")
+    expect(screen.getByText("Leaf")).toHaveClass("level-1")
+  })
+
+  test("renders as a <li> so it is a valid direct child of a <ul>", () => {
+    const { container } = render(<SideNavigationGroup label="Group" />)
+    const root = container.firstElementChild
+    expect(root?.tagName).toBe("LI")
+  })
+
+  test("wraps expanded children in a nested <ul> with only <li> direct children", () => {
+    const { container } = render(
+      <SideNavigationGroup label="Group" open>
+        <SideNavigationItem label="Child A" />
+        <SideNavigationItem label="Child B" />
+      </SideNavigationGroup>
+    )
+
+    const nestedUls = container.querySelectorAll("ul")
+    expect(nestedUls.length).toBe(1)
+    for (const ul of nestedUls) {
+      for (const child of Array.from(ul.children)) {
+        expect(child.tagName).toBe("LI")
+      }
+    }
+  })
+
+  test("does not render a nested <ul> when the group has no children", () => {
+    const { container } = render(<SideNavigationGroup label="Childless Group" open />)
+    expect(container.querySelector("ul")).toBeNull()
   })
 })

packages/ui-components/src/components/SideNavigationItem/SideNavigationItem.component.tsx

@@ -42,7 +42,7 @@ const disabledStyles = `
 export interface SideNavigationItemProps extends HTMLAttributes<HTMLElement> {
   /** Provides an accessibility label for the navigation item. */
   ariaLabel?: string
-  /** Represents nested components. If a string is passed, it will be treated as a label.*/
+  /** Nested SideNavigationItem components rendered as a sub-list when expanded. A string may be passed instead and will be treated as a label. */
   children?: ReactElement<SideNavigationItemProps> | ReactElement<SideNavigationItemProps>[] | string
   /** Marks the item as non-interactive if set to true. */
   disabled?: boolean
@@ -149,38 +149,40 @@ export const SideNavigationItem = ({
 
   return (
     <LevelContext.Provider value={level + 1}>
-      <div className="jn:flex jn:w-full jn:items-start jn:justify-between">
-        {href ? (
-          <a
-            aria-current={selected ? "page" : undefined}
-            aria-label={ariaLabel ? ariaLabel : undefined}
-            className={combinedClassNames}
-            href={!disabled ? href : undefined}
-            onClick={!disabled ? handleMainClick : undefined}
-            aria-disabled={disabled || undefined}
-            tabIndex={disabled ? -1 : undefined}
-            title={titleText}
-            {...props}
-          >
-            {renderLeft()}
-          </a>
-        ) : (
-          <button
-            type="button"
-            aria-current={selected ? "page" : undefined}
-            aria-label={ariaLabel ? ariaLabel : undefined}
-            className={combinedClassNames}
-            onClick={!disabled ? handleMainClick : undefined}
-            disabled={disabled}
-            title={titleText}
-            {...props}
-          >
-            {renderLeft()}
-          </button>
-        )}
-        {renderExpandButton()}
-      </div>
-      {isOpen && typeof children !== "string" && children}
+      <li className="jn:flex jn:w-full jn:flex-col">
+        <div className="jn:flex jn:w-full jn:items-start jn:justify-between">
+          {href ? (
+            <a
+              aria-current={selected ? "page" : undefined}
+              aria-label={ariaLabel ? ariaLabel : undefined}
+              className={combinedClassNames}
+              href={!disabled ? href : undefined}
+              onClick={!disabled ? handleMainClick : undefined}
+              aria-disabled={disabled || undefined}
+              tabIndex={disabled ? -1 : undefined}
+              title={titleText}
+              {...props}
+            >
+              {renderLeft()}
+            </a>
+          ) : (
+            <button
+              type="button"
+              aria-current={selected ? "page" : undefined}
+              aria-label={ariaLabel ? ariaLabel : undefined}
+              className={combinedClassNames}
+              onClick={!disabled ? handleMainClick : undefined}
+              disabled={disabled}
+              title={titleText}
+              {...props}
+            >
+              {renderLeft()}
+            </button>
+          )}
+          {renderExpandButton()}
+        </div>
+        {isOpen && typeof children !== "string" && children && Children.count(children) > 0 && <ul>{children}</ul>}
+      </li>
     </LevelContext.Provider>
   )
 }

packages/ui-components/src/components/SideNavigationItem/SideNavigationItem.test.tsx

@@ -118,4 +118,32 @@ describe("SideNavigationItem", () => {
     )
     expect(screen.getByText("Child")).toHaveClass("level-2")
   })
+
+  it("renders as a <li> so it is a valid direct child of a <ul>", () => {
+    const { container } = render(<SideNavigationItem label="Item" />)
+    const root = container.firstElementChild
+    expect(root?.tagName).toBe("LI")
+  })
+
+  it("wraps expanded children in a nested <ul> with only <li> direct children", () => {
+    const { container } = render(
+      <SideNavigationItem label="Parent" open>
+        <SideNavigationItem label="Child A" />
+        <SideNavigationItem label="Child B" />
+      </SideNavigationItem>
+    )
+
+    const nestedUls = container.querySelectorAll("ul")
+    expect(nestedUls.length).toBe(1)
+    for (const ul of nestedUls) {
+      for (const child of Array.from(ul.children)) {
+        expect(child.tagName).toBe("LI")
+      }
+    }
+  })
+
+  it("does not render a nested <ul> when expanded without children", () => {
+    const { container } = render(<SideNavigationItem label="Lonely" open />)
+    expect(container.querySelector("ul")).toBeNull()
+  })
 })

packages/ui-components/src/components/SideNavigationList/SideNavigationList.component.tsx

@@ -29,5 +29,5 @@ export interface SideNavigationListProps {
  * @see {@link SideNavigationListProps}
  */
 export const SideNavigationList = ({ children }: SideNavigationListProps): ReactNode => {
-  return <ul className={`list-none p-0 m-0 ${sideNavListStyles}`}>{children}</ul>
+  return <ul className={`juno-sidenavigation-list ${sideNavListStyles}`}>{children}</ul>
 }

packages/ui-components/src/components/SideNavigationList/SideNavigationList.test.tsx

@@ -22,11 +22,7 @@ describe("SideNavigationList", () => {
 
     const list = screen.getByRole("list")
     expect(list).toBeInTheDocument()
-    expect(list).toHaveClass("list-none")
-    expect(list).toHaveClass("p-0")
-    expect(list).toHaveClass("m-0")
-    expect(list).toHaveClass("jn:bg-theme-sidenav-list")
-    expect(list).toHaveClass("jn:space-y-[0.25rem]")
+    expect(list).toHaveClass("juno-sidenavigation-list")
   })
 
   test("renders children within the SideNavigationList", () => {