From dbac3c6405bcfede25aead10e1786ee36281ce74 Mon Sep 17 00:00:00 2001
From: Ingrid do Rio Schulze <ingrid.dorioschulze@digitalservice.bund.de>
Date: Thu, 23 Jan 2025 11:38:56 +0100
Subject: [PATCH 1/4] feat(sidebar navigation): update sidebar navigation
 styling

---
 app/components/navigation/FlowNavigation.tsx |  5 ++-
 app/components/navigation/NavItem.tsx        | 39 +++++++++-----------
 2 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/app/components/navigation/FlowNavigation.tsx b/app/components/navigation/FlowNavigation.tsx
index d71779dd3..de589cec6 100644
--- a/app/components/navigation/FlowNavigation.tsx
+++ b/app/components/navigation/FlowNavigation.tsx
@@ -3,7 +3,10 @@ import { NavigationList } from "~/components/navigation/NavigationList";
 
 export default function FlowNavigation(props: FlowNavigationProps) {
   return (
-    <nav aria-label={props.a11yLabels?.menuLabel} className="bg-white">
+    <nav
+      aria-label={props.a11yLabels?.menuLabel}
+      className="bg-white border-[1px] border-blue-400"
+    >
       <NavigationList {...props} />
     </nav>
   );
diff --git a/app/components/navigation/NavItem.tsx b/app/components/navigation/NavItem.tsx
index c25ee71b3..74d698029 100644
--- a/app/components/navigation/NavItem.tsx
+++ b/app/components/navigation/NavItem.tsx
@@ -1,4 +1,4 @@
-import CheckIcon from "@digitalservicebund/icons/Check";
+import CheckCircle from "@digitalservicebund/icons/CheckCircle";
 import ExpandLessIcon from "@digitalservicebund/icons/ExpandLess";
 import ExpandMoreIcon from "@digitalservicebund/icons/ExpandMore";
 import classNames from "classnames";
@@ -25,9 +25,9 @@ export const StateIcon: FC<{
   id: string;
   a11yLabels?: NavigationA11yLabels;
 }> = ({ id, a11yLabels }) => (
-  <CheckIcon
+  <CheckCircle
     id={id}
-    className="shrink-0"
+    className="shrink-0 fill-green-700"
     aria-label={a11yLabels?.itemFinished}
   />
 );
@@ -49,21 +49,21 @@ export function NavItem({
   const isDone = stateIsDone(state);
   const collapse = useCollapse({ defaultExpanded: isCurrent });
 
-  // Transparent left borders to avoid layout shifts
-  const liClassNames = classNames("list-none border-l-[1px] min-w-full", {
-    "text-gray-600 curser-not-allowed hover:font-normal pointer-events-none":
-      isDisabled,
-    "mb-1": !isChild, // margin instead of bottom border to avoid diagonal corners colors
-    "border-l-blue-800": isCurrent,
-    "border-l-blue-100": !isCurrent,
-  });
+  // Transparent last: borders to avoid layout shifts
+  const liClassNames = classNames(
+    "list-none border-b-[1px] border-blue-400 last:border-0 min-w-full",
+    {
+      "text-gray-600 curser-not-allowed hover:font-normal pointer-events-none":
+        isDisabled,
+      "border-transparent last:border-transparent": isChild,
+    },
+  );
 
   const itemClassNames = classNames(
-    "bg-blue-100 w-full ds-label-02-reg p-16 border-l-[3px] border-transparent flex gap-x-4 items-center hover:underline hover:bg-blue-300 active:bg-white focus-visible:shadow-[inset_0px_0px_0px_4px] focus:shadow-blue-800",
+    "w-full ds-label-02-reg p-16 flex justify-between items-center hover:underline hover:bg-blue-400 active:bg-blue-300 focus-visible:shadow-[inset_0px_0px_0px_4px] focus:shadow-blue-300",
     {
-      "ds-label-02-bold bg-blue-500 border-l-blue-800":
-        isCurrent && !hasSubflows,
-      "pl-40": isChild,
+      "ds-label-02-bold bg-blue-400": isCurrent && !hasSubflows,
+      "pl-24": isChild,
     },
   );
   const iconId = useId();
@@ -79,18 +79,15 @@ export function NavItem({
             {...collapse.getToggleProps()}
             aria-describedby={iconId}
           >
-            {isDone && <StateIcon id={iconId} a11yLabels={a11yLabels} />}
             {label}
             {collapse.isExpanded ? (
               <ExpandLessIcon className="ml-auto" />
             ) : (
               <ExpandMoreIcon className="ml-auto" />
             )}
+            {isDone && <StateIcon id={iconId} a11yLabels={a11yLabels} />}
           </button>
-          <section
-            className="border-t-[1px] border-white"
-            {...collapse.getCollapseProps()}
-          >
+          <section {...collapse.getCollapseProps()}>
             <NavigationList
               navItems={visibleChildItems}
               a11yLabels={a11yLabels}
@@ -106,8 +103,8 @@ export function NavItem({
           aria-current={isCurrent}
           aria-describedby={iconId}
         >
-          {isDone && <StateIcon id={iconId} a11yLabels={a11yLabels} />}
           {label}
+          {isDone && <StateIcon id={iconId} a11yLabels={a11yLabels} />}
         </a>
       )}
     </li>

From a4ca3554da047522e1bc99301a6e65d929809579 Mon Sep 17 00:00:00 2001
From: Ingrid do Rio Schulze <ingrid.dorioschulze@digitalservice.bund.de>
Date: Fri, 24 Jan 2025 13:25:35 +0100
Subject: [PATCH 2/4] test(sidebar navigation): add new test for NavItem and
 NavigationList, update FlowNavigation test

---
 .../__test__/FlowNavigation.test.tsx          |  79 +++-------
 app/components/__test__/NavItem.test.tsx      | 137 ++++++++++++++++++
 .../__test__/NavigationList.test.tsx          |  18 +++
 app/components/navigation/FlowNavigation.tsx  |   1 +
 4 files changed, 173 insertions(+), 62 deletions(-)
 create mode 100644 app/components/__test__/NavItem.test.tsx
 create mode 100644 app/components/__test__/NavigationList.test.tsx

diff --git a/app/components/__test__/FlowNavigation.test.tsx b/app/components/__test__/FlowNavigation.test.tsx
index 0e00926c4..bb7011b70 100644
--- a/app/components/__test__/FlowNavigation.test.tsx
+++ b/app/components/__test__/FlowNavigation.test.tsx
@@ -1,68 +1,23 @@
-import {
-  fireEvent,
-  render,
-  screen,
-  type RenderResult,
-} from "@testing-library/react";
+import { render, screen } from "@testing-library/react";
 import FlowNavigation from "~/components/navigation/FlowNavigation";
 import { NavState } from "~/services/navigation/navState";
 
 describe("FlowNavigation", () => {
-  let component: RenderResult;
-
-  afterEach(() => {
-    component.unmount();
-  });
-
-  it("renders single navigation item", () => {
-    const destination = "/destination";
-    const label = "navLabel";
-    const state = NavState.Current;
-
-    component = render(
-      <FlowNavigation navItems={[{ destination, label, state }]} />,
-    );
-
-    const links = screen.getAllByRole<HTMLAnchorElement>("link");
-    expect(links[0]).toHaveTextContent(label);
-    expect(links[0].href).toContain(destination);
-    expect(links[0].parentNode).toBeInstanceOf(HTMLLIElement);
-  });
-
-  it("renders disabled navigation item", () => {
-    const label = "navLabel";
-    component = render(
-      <FlowNavigation
-        navItems={[{ destination: "/a", label, state: NavState.Disabled }]}
-      />,
-    );
-    screen
-      .getAllByRole<HTMLAnchorElement>("link")
-      .forEach((link) => expect(link).toHaveAttribute("aria-disabled", "true"));
-  });
-
-  it("renders nested collapsible navigation", () => {
-    const label = "navLabel";
-    component = render(
-      <FlowNavigation
-        navItems={[
-          {
-            destination: "/a",
-            label,
-            state: NavState.Open,
-            subflows: [
-              { destination: "/c", label: "sublabel", state: NavState.Open },
-            ],
-          },
-        ]}
-      />,
-    );
-
-    const liElement = screen.getByRole<HTMLLIElement>("listitem");
-
-    expect(liElement.children[1]).not.toBeVisible();
-    fireEvent.click(liElement.children[0]);
-    expect(liElement.children[1]).toBeVisible();
-    expect(liElement.children[1]).toHaveTextContent("sublabel");
+  it("renders a <nav> element with a <NavigationList> component", () => {
+    const navItems = [
+      {
+        destination: "/destination",
+        label: "navLabel",
+        state: NavState.Current,
+      },
+    ];
+
+    render(<FlowNavigation navItems={navItems} />);
+
+    const nav = screen.getByRole("navigation");
+    expect(nav).toHaveClass("bg-white border-[1px] border-blue-400");
+
+    const list = screen.getByRole("list");
+    expect(list).toHaveClass("pl-0");
   });
 });
diff --git a/app/components/__test__/NavItem.test.tsx b/app/components/__test__/NavItem.test.tsx
new file mode 100644
index 000000000..eec23c716
--- /dev/null
+++ b/app/components/__test__/NavItem.test.tsx
@@ -0,0 +1,137 @@
+import { render, screen } from "@testing-library/react";
+import { NavState } from "~/services/navigation/navState";
+import { NavItem } from "../navigation/NavItem";
+
+describe("NavigationItem", () => {
+  const destination = "/destination";
+  const label = "navLabel";
+
+  it("renders one navigation item", () => {
+    render(
+      <NavItem
+        destination={destination}
+        label={label}
+        state={NavState.Current}
+      />,
+    );
+
+    expect(screen.getByRole("link")).toHaveTextContent(label);
+    expect(screen.getByRole("link")).toHaveAttribute("href", destination);
+    expect(screen.getByRole("listitem")).toBeInstanceOf(HTMLLIElement);
+  });
+  it("renders navigation item with a checkmark icon when state is done", () => {
+    render(
+      <NavItem destination={destination} label={label} state={NavState.Done} />,
+    );
+
+    const checkCircle = screen.getByTestId("CheckCircleIcon");
+    expect(checkCircle).toHaveClass("shrink-0 fill-green-700");
+  });
+
+  it("renders navigation item with the correct classNames when state is disabled", () => {
+    render(
+      <NavItem
+        destination={destination}
+        label={label}
+        state={NavState.Disabled}
+      />,
+    );
+
+    screen
+      .getAllByRole<HTMLAnchorElement>("link")
+      .forEach((link) => expect(link).toHaveAttribute("aria-disabled", "true"));
+    expect(screen.getByRole("listitem")).toHaveClass(
+      "text-gray-600 curser-not-allowed hover:font-normal pointer-events-none",
+    );
+  });
+
+  it("renders navigation item with correct classNames when state is current", () => {
+    render(
+      <NavItem
+        destination={destination}
+        label={label}
+        state={NavState.Current}
+      />,
+    );
+
+    expect(screen.getByRole("listitem")).toHaveClass(
+      "border-b-[1px] border-blue-400 last:border-0 min-w-full",
+    );
+  });
+
+  it("renders navigation item with correct classNames when isChild is true", () => {
+    render(
+      <NavItem
+        destination={destination}
+        label={label}
+        state={NavState.Current}
+        isChild={true}
+      />,
+    );
+
+    expect(screen.getByRole("listitem")).toHaveClass(
+      "border-transparent last:border-transparent",
+    );
+  });
+
+  it("renders navigation item with active subflows", () => {
+    const subflows = [
+      {
+        destination: "/subflow1",
+        label: "subflowLabel1",
+        state: NavState.Current,
+      },
+      {
+        destination: "/subflow2",
+        label: "subflowLabel2",
+        state: NavState.Disabled,
+      },
+    ];
+    render(
+      <NavItem
+        destination={destination}
+        label={label}
+        state={NavState.Done}
+        subflows={subflows}
+      />,
+    );
+
+    const subflowLink = screen.getAllByRole("link");
+    expect(subflowLink[0]).toHaveTextContent("subflowLabel1");
+    expect(subflowLink[0]).toHaveAttribute("href", "/subflow1");
+    expect(subflowLink).toHaveLength(1); // check that disabled subflow is not rendered
+    expect(subflowLink[0].parentNode).toBeInstanceOf(HTMLLIElement);
+  });
+
+  it("renders subItem with correct classNames when isChild is true", () => {
+    const subflows = [
+      {
+        destination: "/subflow",
+        label: "subflowLabel",
+        state: NavState.Open,
+      },
+    ];
+    render(
+      <NavItem
+        destination={destination}
+        label={label}
+        state={NavState.Current}
+        subflows={subflows}
+      />,
+    );
+    expect(screen.getByText("subflowLabel")).toHaveClass("pl-24");
+  });
+
+  it("renders items with correct classNames when the state is current and item doesn't have subflows", () => {
+    render(
+      <NavItem
+        destination={destination}
+        label={label}
+        state={NavState.Current}
+      />,
+    );
+    expect(screen.getByRole("link")).toHaveClass(
+      "ds-label-02-bold bg-blue-400",
+    );
+  });
+});
diff --git a/app/components/__test__/NavigationList.test.tsx b/app/components/__test__/NavigationList.test.tsx
new file mode 100644
index 000000000..61470c35d
--- /dev/null
+++ b/app/components/__test__/NavigationList.test.tsx
@@ -0,0 +1,18 @@
+import { render, screen } from "@testing-library/react";
+import { NavState } from "~/services/navigation/navState";
+import { NavigationList } from "../navigation/NavigationList";
+
+describe("NavigationList", () => {
+  it("renders a <ul> element with one navigation item", () => {
+    const destination = "/destination";
+    const label = "navLabel";
+    const state = NavState.Current;
+
+    render(<NavigationList navItems={[{ destination, label, state }]} />);
+
+    const links = screen.getAllByRole<HTMLAnchorElement>("link");
+    expect(links[0]).toHaveTextContent(label);
+    expect(links[0].href).toContain(destination);
+    expect(links[0].parentNode).toBeInstanceOf(HTMLLIElement);
+  });
+});
diff --git a/app/components/navigation/FlowNavigation.tsx b/app/components/navigation/FlowNavigation.tsx
index de589cec6..97ab2e857 100644
--- a/app/components/navigation/FlowNavigation.tsx
+++ b/app/components/navigation/FlowNavigation.tsx
@@ -4,6 +4,7 @@ import { NavigationList } from "~/components/navigation/NavigationList";
 export default function FlowNavigation(props: FlowNavigationProps) {
   return (
     <nav
+      role="navigation"
       aria-label={props.a11yLabels?.menuLabel}
       className="bg-white border-[1px] border-blue-400"
     >

From 8798a673920a57be4267f6decac55d16f4d98a2d Mon Sep 17 00:00:00 2001
From: Ingrid do Rio Schulze <ingrid.dorioschulze@digitalservice.bund.de>
Date: Fri, 24 Jan 2025 18:43:45 +0100
Subject: [PATCH 3/4] test(sidebar navigation): update NavItem navigation with
 subflows assertion

---
 app/components/__test__/NavItem.test.tsx | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/app/components/__test__/NavItem.test.tsx b/app/components/__test__/NavItem.test.tsx
index eec23c716..6c9bb9e91 100644
--- a/app/components/__test__/NavItem.test.tsx
+++ b/app/components/__test__/NavItem.test.tsx
@@ -74,12 +74,12 @@ describe("NavigationItem", () => {
     );
   });
 
-  it("renders navigation item with active subflows", () => {
+  it("renders navigation item with subflows", () => {
     const subflows = [
       {
         destination: "/subflow1",
         label: "subflowLabel1",
-        state: NavState.Current,
+        state: NavState.Open,
       },
       {
         destination: "/subflow2",
@@ -91,16 +91,15 @@ describe("NavigationItem", () => {
       <NavItem
         destination={destination}
         label={label}
-        state={NavState.Done}
+        state={NavState.Current}
         subflows={subflows}
       />,
     );
 
-    const subflowLink = screen.getAllByRole("link");
-    expect(subflowLink[0]).toHaveTextContent("subflowLabel1");
-    expect(subflowLink[0]).toHaveAttribute("href", "/subflow1");
-    expect(subflowLink).toHaveLength(1); // check that disabled subflow is not rendered
-    expect(subflowLink[0].parentNode).toBeInstanceOf(HTMLLIElement);
+    const subflow1 = screen.getByText("subflowLabel1");
+    expect(subflow1).toHaveAttribute("href", "/subflow1");
+    const subflow2 = screen.queryByText("subflowLabel2");
+    expect(subflow2).not.toBeInTheDocument();
   });
 
   it("renders subItem with correct classNames when isChild is true", () => {

From 7ec38f6c3c85cc2620ce1a5b9ead8684665d78e7 Mon Sep 17 00:00:00 2001
From: Ingrid do Rio Schulze <ingrid.dorioschulze@digitalservice.bund.de>
Date: Mon, 27 Jan 2025 10:31:24 +0100
Subject: [PATCH 4/4] test(sidebar navigation): move tests to correct path, add
 assertion to check if ul is being rendered and change assertions testing
 child components functionality

---
 .../__test__/NavigationList.test.tsx          | 18 ---------------
 .../__test__/FlowNavigation.test.tsx          |  5 +----
 .../__test__/NavItem.test.tsx                 |  2 +-
 .../__test__/NavigationList.test.tsx          | 22 +++++++++++++++++++
 4 files changed, 24 insertions(+), 23 deletions(-)
 delete mode 100644 app/components/__test__/NavigationList.test.tsx
 rename app/components/{ => navigation}/__test__/FlowNavigation.test.tsx (77%)
 rename app/components/{ => navigation}/__test__/NavItem.test.tsx (98%)
 create mode 100644 app/components/navigation/__test__/NavigationList.test.tsx

diff --git a/app/components/__test__/NavigationList.test.tsx b/app/components/__test__/NavigationList.test.tsx
deleted file mode 100644
index 61470c35d..000000000
--- a/app/components/__test__/NavigationList.test.tsx
+++ /dev/null
@@ -1,18 +0,0 @@
-import { render, screen } from "@testing-library/react";
-import { NavState } from "~/services/navigation/navState";
-import { NavigationList } from "../navigation/NavigationList";
-
-describe("NavigationList", () => {
-  it("renders a <ul> element with one navigation item", () => {
-    const destination = "/destination";
-    const label = "navLabel";
-    const state = NavState.Current;
-
-    render(<NavigationList navItems={[{ destination, label, state }]} />);
-
-    const links = screen.getAllByRole<HTMLAnchorElement>("link");
-    expect(links[0]).toHaveTextContent(label);
-    expect(links[0].href).toContain(destination);
-    expect(links[0].parentNode).toBeInstanceOf(HTMLLIElement);
-  });
-});
diff --git a/app/components/__test__/FlowNavigation.test.tsx b/app/components/navigation/__test__/FlowNavigation.test.tsx
similarity index 77%
rename from app/components/__test__/FlowNavigation.test.tsx
rename to app/components/navigation/__test__/FlowNavigation.test.tsx
index bb7011b70..31bbf583c 100644
--- a/app/components/__test__/FlowNavigation.test.tsx
+++ b/app/components/navigation/__test__/FlowNavigation.test.tsx
@@ -3,7 +3,7 @@ import FlowNavigation from "~/components/navigation/FlowNavigation";
 import { NavState } from "~/services/navigation/navState";
 
 describe("FlowNavigation", () => {
-  it("renders a <nav> element with a <NavigationList> component", () => {
+  it("renders a navigation element", () => {
     const navItems = [
       {
         destination: "/destination",
@@ -16,8 +16,5 @@ describe("FlowNavigation", () => {
 
     const nav = screen.getByRole("navigation");
     expect(nav).toHaveClass("bg-white border-[1px] border-blue-400");
-
-    const list = screen.getByRole("list");
-    expect(list).toHaveClass("pl-0");
   });
 });
diff --git a/app/components/__test__/NavItem.test.tsx b/app/components/navigation/__test__/NavItem.test.tsx
similarity index 98%
rename from app/components/__test__/NavItem.test.tsx
rename to app/components/navigation/__test__/NavItem.test.tsx
index 6c9bb9e91..aa01eb9e0 100644
--- a/app/components/__test__/NavItem.test.tsx
+++ b/app/components/navigation/__test__/NavItem.test.tsx
@@ -1,6 +1,6 @@
 import { render, screen } from "@testing-library/react";
 import { NavState } from "~/services/navigation/navState";
-import { NavItem } from "../navigation/NavItem";
+import { NavItem } from "../NavItem";
 
 describe("NavigationItem", () => {
   const destination = "/destination";
diff --git a/app/components/navigation/__test__/NavigationList.test.tsx b/app/components/navigation/__test__/NavigationList.test.tsx
new file mode 100644
index 000000000..1044e8031
--- /dev/null
+++ b/app/components/navigation/__test__/NavigationList.test.tsx
@@ -0,0 +1,22 @@
+import { render, screen } from "@testing-library/react";
+import { NavState } from "~/services/navigation/navState";
+import { NavigationList } from "../NavigationList";
+
+describe("NavigationList", () => {
+  it("renders a navigation list with one navigation item", () => {
+    const destination = "/destination";
+    const label = "navLabel";
+    const state = NavState.Current;
+
+    render(<NavigationList navItems={[{ destination, label, state }]} />);
+
+    const list = screen.getByRole("list");
+    expect(list).toBeInstanceOf(HTMLUListElement);
+    expect(list).toHaveClass("pl-0");
+
+    const item = screen.getAllByRole<HTMLAnchorElement>("link");
+    expect(item[0]).toHaveTextContent(label);
+    expect(item[0].href).toContain(destination);
+    expect(item[0].parentNode).toBeInstanceOf(HTMLLIElement);
+  });
+});