diff --git a/packages/ui-components/src/components/SideNavigation/SideNavigation.stories.tsx b/packages/ui-components/src/components/SideNavigation/SideNavigation.stories.tsx index 8eb7749f22..1fa72a9e1d 100644 --- a/packages/ui-components/src/components/SideNavigation/SideNavigation.stories.tsx +++ b/packages/ui-components/src/components/SideNavigation/SideNavigation.stories.tsx @@ -60,20 +60,18 @@ export const NavigationWithGroups: Story = { render: (args) => ( - - - - - - + + + + + - - + - + - + diff --git a/packages/ui-components/src/components/SideNavigation/sidenavigation.css b/packages/ui-components/src/components/SideNavigation/sidenavigation.css index f87bda78f1..c58370238b 100644 --- a/packages/ui-components/src/components/SideNavigation/sidenavigation.css +++ b/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; diff --git a/packages/ui-components/src/components/SideNavigationGroup/SideNavigationGroup.component.tsx b/packages/ui-components/src/components/SideNavigationGroup/SideNavigationGroup.component.tsx index 326d231046..e4766c0649 100644 --- a/packages/ui-components/src/components/SideNavigationGroup/SideNavigationGroup.component.tsx +++ b/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 = () => ( - {label} + {label} ) @@ -124,9 +122,9 @@ export const SideNavigationGroup = ({ children, label = "", open = false }: Side } return ( - <> +
  • {renderGroup()} - {isOpen && {children}} - + {isOpen && hasChildren && } +
  • ) } diff --git a/packages/ui-components/src/components/SideNavigationGroup/SideNavigationGroup.stories.tsx b/packages/ui-components/src/components/SideNavigationGroup/SideNavigationGroup.stories.tsx index ef85f5892e..d3bbb44443 100644 --- a/packages/ui-components/src/components/SideNavigationGroup/SideNavigationGroup.stories.tsx +++ b/packages/ui-components/src/components/SideNavigationGroup/SideNavigationGroup.stories.tsx @@ -35,11 +35,16 @@ type Story = StoryObj export const Default: Story = { args: { label: "Group", + children: [ + , + , + , + ], }, 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: ( <> - - - - - + + + + + diff --git a/packages/ui-components/src/components/SideNavigationGroup/SideNavigationGroup.test.tsx b/packages/ui-components/src/components/SideNavigationGroup/SideNavigationGroup.test.tsx index 31345887e8..cb9d2bf733 100644 --- a/packages/ui-components/src/components/SideNavigationGroup/SideNavigationGroup.test.tsx +++ b/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( - - - - - + ) - 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( - - - + ) - expect(screen.getByText("Leaf")).toHaveClass("level-3") + expect(screen.getByText("Leaf")).toHaveClass("level-1") + }) + + test("renders as a
  • so it is a valid direct child of a
      ", () => { + const { container } = render() + const root = container.firstElementChild + expect(root?.tagName).toBe("LI") + }) + + test("wraps expanded children in a nested
        with only
      • direct children", () => { + const { container } = render( + + + + + ) + + 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
          when the group has no children", () => { + const { container } = render() + expect(container.querySelector("ul")).toBeNull() }) }) diff --git a/packages/ui-components/src/components/SideNavigationItem/SideNavigationItem.component.tsx b/packages/ui-components/src/components/SideNavigationItem/SideNavigationItem.component.tsx index b80f50b3cb..2e841b02ac 100644 --- a/packages/ui-components/src/components/SideNavigationItem/SideNavigationItem.component.tsx +++ b/packages/ui-components/src/components/SideNavigationItem/SideNavigationItem.component.tsx @@ -42,7 +42,7 @@ const disabledStyles = ` export interface SideNavigationItemProps extends HTMLAttributes { /** 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 | ReactElement[] | string /** Marks the item as non-interactive if set to true. */ disabled?: boolean @@ -149,38 +149,40 @@ export const SideNavigationItem = ({ return ( -
          - {href ? ( - - {renderLeft()} - - ) : ( - - )} - {renderExpandButton()} -
          - {isOpen && typeof children !== "string" && children} +
        • +
          + {href ? ( + + {renderLeft()} + + ) : ( + + )} + {renderExpandButton()} +
          + {isOpen && typeof children !== "string" && children && Children.count(children) > 0 &&
            {children}
          } +
        • ) } diff --git a/packages/ui-components/src/components/SideNavigationItem/SideNavigationItem.test.tsx b/packages/ui-components/src/components/SideNavigationItem/SideNavigationItem.test.tsx index 62bc6dcf3f..fe1ebeb19e 100644 --- a/packages/ui-components/src/components/SideNavigationItem/SideNavigationItem.test.tsx +++ b/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
        • so it is a valid direct child of a
            ", () => { + const { container } = render() + const root = container.firstElementChild + expect(root?.tagName).toBe("LI") + }) + + it("wraps expanded children in a nested
              with only
            • direct children", () => { + const { container } = render( + + + + + ) + + 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
                when expanded without children", () => { + const { container } = render() + expect(container.querySelector("ul")).toBeNull() + }) }) diff --git a/packages/ui-components/src/components/SideNavigationList/SideNavigationList.component.tsx b/packages/ui-components/src/components/SideNavigationList/SideNavigationList.component.tsx index 668cca615c..d52b194a70 100644 --- a/packages/ui-components/src/components/SideNavigationList/SideNavigationList.component.tsx +++ b/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
                  {children}
                + return
                  {children}
                } diff --git a/packages/ui-components/src/components/SideNavigationList/SideNavigationList.test.tsx b/packages/ui-components/src/components/SideNavigationList/SideNavigationList.test.tsx index f3d0bcf11d..4a9d34a063 100644 --- a/packages/ui-components/src/components/SideNavigationList/SideNavigationList.test.tsx +++ b/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", () => {