← Back to team overview

sts-sponsors team mailing list archive

[Merge] ~petermakowski/maas-site-manager:settings-subnavigation-MAASENG-1508 into maas-site-manager:main

 

Peter Makowski has proposed merging ~petermakowski/maas-site-manager:settings-subnavigation-MAASENG-1508 into maas-site-manager:main.

Commit message:
add settings sub-nav MAASENG-1508
- fix baseline top margin MAASENG-1573
- fix invalid scss file import

Requested reviews:
  MAAS Committers (maas-committers)

For more details, see:
https://code.launchpad.net/~petermakowski/maas-site-manager/+git/site-manager/+merge/441449

QA Steps
Login go to settings
Ensure the secondary navigation is shown and you can use it to navigate to both Tokens and Requests
Ensure the secondary navigation is displayed and functional on mobile
-- 
Your team MAAS Committers is requested to review the proposed merge of ~petermakowski/maas-site-manager:settings-subnavigation-MAASENG-1508 into maas-site-manager:main.
diff --git a/frontend/src/App.scss b/frontend/src/App.scss
index 2d7748f..d61a372 100644
--- a/frontend/src/App.scss
+++ b/frontend/src/App.scss
@@ -1,6 +1,6 @@
 @use "sass:math";
 
-@import "@/settings.scss";
+@import "@/settings";
 @import "vanilla-framework";
 @include vf-base;
 
@@ -54,12 +54,13 @@
 // components styles
 @import "@/components/MainLayout/MainLayout";
 @import "@/components/Navigation/Navigation";
+@import "@/components/SecondaryNavigation/SecondaryNavigation";
 @import "@/components/TokensCreate/TokensCreate";
 @import "@/components/SitesList/SitesTable/SitesTableControls/SitesTableControls";
 @import "@/components/SitesList/SitesTable/SitesTableControls/ColumnsVisibilityControl/ColumnsVisibilityControl";
 @import "@/components/SitesList/SitesTable/SitesTable";
 @import "@/components/Placeholder/Placeholder";
-@import "@/components/TokensList/components/TokensTable/TokensTable.scss";
-@import "@/components/base/CopyButton/CopyButton.scss";
-@import "@/components/base/TablePagination/TablePagination.scss";
-@import "@/components/base/PaginationBar/PaginationBar.scss";
+@import "@/components/TokensList/components/TokensTable/TokensTable";
+@import "@/components/base/CopyButton/CopyButton";
+@import "@/components/base/TablePagination/TablePagination";
+@import "@/components/base/PaginationBar/PaginationBar";
diff --git a/frontend/src/_utils.scss b/frontend/src/_utils.scss
index 59a93da..387a22f 100644
--- a/frontend/src/_utils.scss
+++ b/frontend/src/_utils.scss
@@ -21,3 +21,6 @@
 .u-capitalize {
   text-transform: capitalize !important;
 }
+.u-padding-top--medium {
+  padding-top: $spv--medium !important;
+}
diff --git a/frontend/src/base/routesConfig.ts b/frontend/src/base/routesConfig.ts
index 0df83b8..a92033e 100644
--- a/frontend/src/base/routesConfig.ts
+++ b/frontend/src/base/routesConfig.ts
@@ -4,11 +4,11 @@ export const protectedRoutes = {
     title: "Regions",
   },
   requests: {
-    path: "/requests",
+    path: "/settings/requests",
     title: "Requests",
   },
   tokens: {
-    path: "/tokens",
+    path: "/settings/tokens",
     title: "Tokens",
   },
 };
diff --git a/frontend/src/components/MainLayout/MainLayout.test.tsx b/frontend/src/components/MainLayout/MainLayout.test.tsx
index d58173c..273ae0b 100644
--- a/frontend/src/components/MainLayout/MainLayout.test.tsx
+++ b/frontend/src/components/MainLayout/MainLayout.test.tsx
@@ -6,6 +6,8 @@ describe("MainLayout", () => {
   it("renders header", async () => {
     renderWithMemoryRouter(<MainLayout />);
 
-    await waitFor(() => expect(screen.getByRole("heading", { name: /MAAS Site Manager/i })).toBeInTheDocument());
+    await waitFor(() =>
+      expect(screen.getByRole("heading", { level: 1, name: /MAAS Site Manager/i })).toBeInTheDocument(),
+    );
   });
 });
diff --git a/frontend/src/components/MainLayout/MainLayout.tsx b/frontend/src/components/MainLayout/MainLayout.tsx
index 4ab076f..93364ab 100644
--- a/frontend/src/components/MainLayout/MainLayout.tsx
+++ b/frontend/src/components/MainLayout/MainLayout.tsx
@@ -3,7 +3,7 @@ import { useEffect } from "react";
 
 import { Col, Row, Strip, useOnEscapePressed, usePrevious } from "@canonical/react-components";
 import classNames from "classnames";
-import { Outlet, useLocation } from "react-router-dom";
+import { matchPath, Outlet, useLocation } from "react-router-dom";
 
 import { routesConfig } from "@/base/routesConfig";
 import type { RoutePath } from "@/base/routesConfig";
@@ -11,6 +11,7 @@ import DocumentTitle from "@/components/DocumentTitle/DocumentTitle";
 import Navigation from "@/components/Navigation";
 import NavigationBanner from "@/components/Navigation/NavigationBanner";
 import RemoveRegions from "@/components/RemoveRegions";
+import SecondaryNavigation from "@/components/SecondaryNavigation";
 import { useAppContext } from "@/context";
 import TokensCreate from "@/pages/tokens/create";
 
@@ -27,7 +28,7 @@ const Aside = ({ children, sidebar, setSidebar, ...props }: AsideProps) => {
   return (
     <aside
       aria-hidden={!sidebar}
-      className={classNames("l-aside", "is-maas-site-manager", { "is-collapsed": !sidebar })}
+      className={classNames("l-aside is-maas-site-manager u-padding-top--medium", { "is-collapsed": !sidebar })}
       id="aside-panel"
       role="dialog"
       {...props}
@@ -45,6 +46,7 @@ const getPageTitle = (pathname: RoutePath) => {
 };
 
 const LoginLayout: React.FC = () => {
+  const { pathname } = useLocation();
   return (
     <div className="l-application">
       <header className="l-navigation-bar is-pinned">
@@ -55,7 +57,8 @@ const LoginLayout: React.FC = () => {
         </div>
       </header>
       <main className="l-main">
-        <div>
+        <h1 className="u-hide">{getPageTitle(pathname as RoutePath)}</h1>
+        <div className="l-main__content">
           <Strip element="section" includeCol={false} shallow>
             <Col size={12}>
               <Outlet />
@@ -79,15 +82,23 @@ const MainLayout: React.FC = () => {
     }
   }, [pathname, previousPathname, setSidebar]);
 
+  const isSideNavVisible = matchPath("settings/*", pathname);
+
   return (
     <>
       <div className="l-application">
         <Navigation />
         <main className="l-main is-maas-site-manager">
-          <div className="row">
-            <div className="col-12">
-              <h1 className="u-hide">MAAS Site Manager</h1>
-              <Outlet />
+          <h1 className="u-hide">{getPageTitle(pathname as RoutePath)}</h1>
+          <div className={classNames("l-main__nav", { "is-open": isSideNavVisible })}>
+            <SecondaryNavigation isOpen={!!isSideNavVisible} />
+          </div>
+
+          <div className="l-main__content u-padding-top--medium">
+            <div className="row">
+              <div className="col-12">
+                <Outlet />
+              </div>
             </div>
           </div>
         </main>
diff --git a/frontend/src/components/MainLayout/_MainLayout.scss b/frontend/src/components/MainLayout/_MainLayout.scss
index 26594be..3390122 100644
--- a/frontend/src/components/MainLayout/_MainLayout.scss
+++ b/frontend/src/components/MainLayout/_MainLayout.scss
@@ -1,7 +1,23 @@
 .l-main.is-maas-site-manager {
-  margin-top: $sph--large;
+  display: flex;
+  flex-direction: column;
+  overflow: hidden;
+  @media (min-width: $breakpoint-small) {
+    flex-direction: row;
+  }
+  .l-main__nav {
+    flex-shrink: 0;
+
+    @media (min-width: $breakpoint-small) {
+      height: 100vh;
+    }
+
+
+  }
+  .l-main__content {
+    overflow-y: auto;
+  }
 }
 .l-aside.is-maas-site-manager {
   background: $color-x-light;
-  padding-top: $sph--large;
 }
diff --git a/frontend/src/components/Navigation/Navigation.test.tsx b/frontend/src/components/Navigation/Navigation.test.tsx
index e0609fc..a997571 100644
--- a/frontend/src/components/Navigation/Navigation.test.tsx
+++ b/frontend/src/components/Navigation/Navigation.test.tsx
@@ -1,6 +1,6 @@
 import { MemoryRouter } from "react-router-dom";
 
-import Navigation, { navItems, navBottomItems } from "./Navigation";
+import Navigation, { navItems, navItemsBottom } from "./Navigation";
 
 import { render, screen, userEvent } from "@/test-utils";
 
@@ -14,7 +14,7 @@ describe("Navigation", () => {
     expect(screen.getByRole("navigation")).toBeInTheDocument();
   });
 
-  [...navItems, ...navBottomItems].forEach(({ label, url }) => {
+  [...navItems, ...navItemsBottom].forEach(({ label, url }) => {
     it(`highlights ${label} navigation item when active`, () => {
       render(
         <MemoryRouter initialEntries={[{ pathname: url, key: "testKey" }]}>
diff --git a/frontend/src/components/Navigation/Navigation.tsx b/frontend/src/components/Navigation/Navigation.tsx
index a61ee34..1036927 100644
--- a/frontend/src/components/Navigation/Navigation.tsx
+++ b/frontend/src/components/Navigation/Navigation.tsx
@@ -6,7 +6,7 @@ import useLocalStorageState from "use-local-storage-state";
 import NavigationBanner from "./NavigationBanner";
 import NavigationCollapseToggle from "./NavigationCollapseToggle";
 import NavigationList from "./NavigationList";
-import type { NavGroup, NavLink } from "./types";
+import type { NavLink } from "./types";
 
 export const navItems: NavLink[] = [
   {
@@ -16,16 +16,11 @@ export const navItems: NavLink[] = [
   },
 ];
 
-export const navBottomItems: NavLink[] = [
-  { label: "Tokens", url: "/tokens" },
-  { label: "Requests", url: "/requests" },
-];
-
-const navItemsBottom: NavGroup[] = [
+export const navItemsBottom: NavLink[] = [
   {
-    groupTitle: "Enrolment",
-    groupIcon: "settings",
-    navLinks: navBottomItems,
+    label: "Settings",
+    url: "/settings",
+    icon: "settings",
   },
 ];
 
diff --git a/frontend/src/components/Navigation/_Navigation.scss b/frontend/src/components/Navigation/_Navigation.scss
index 706e28d..b006f4a 100644
--- a/frontend/src/components/Navigation/_Navigation.scss
+++ b/frontend/src/components/Navigation/_Navigation.scss
@@ -3,6 +3,8 @@
   &:hover,
   &:focus-within,
   &.is-pinned {
+      grid-area: nav;
+      position: static;
     .l-navigation__controls {
       opacity: 1;
       visibility: visible;
diff --git a/frontend/src/components/SecondaryNavigation/SecondaryNavigation.test.tsx b/frontend/src/components/SecondaryNavigation/SecondaryNavigation.test.tsx
new file mode 100644
index 0000000..c579458
--- /dev/null
+++ b/frontend/src/components/SecondaryNavigation/SecondaryNavigation.test.tsx
@@ -0,0 +1,19 @@
+import SecondaryNavigation from "./SecondaryNavigation";
+
+import { routesConfig } from "@/base/routesConfig";
+import { renderWithMemoryRouter, screen } from "@/test-utils";
+
+it("renders settings secondary navigation", async () => {
+  renderWithMemoryRouter(<SecondaryNavigation />);
+
+  expect(screen.getByRole("heading", { level: 2, name: /Settings/i })).toBeInTheDocument();
+  expect(screen.getByRole("link", { name: /Tokens/i })).toBeInTheDocument();
+  expect(screen.getByRole("link", { name: /Requests/i })).toBeInTheDocument();
+});
+
+it("highlights an active navigation item correctly settings secondary navigation", async () => {
+  renderWithMemoryRouter(<SecondaryNavigation />, { initialEntries: [routesConfig.requests.path] });
+
+  expect(screen.getByRole("heading", { level: 2, name: /Settings/i })).toBeInTheDocument();
+  expect(screen.getByRole("link", { current: "page" })).toHaveTextContent(routesConfig.requests.title);
+});
diff --git a/frontend/src/components/SecondaryNavigation/SecondaryNavigation.tsx b/frontend/src/components/SecondaryNavigation/SecondaryNavigation.tsx
new file mode 100644
index 0000000..669d917
--- /dev/null
+++ b/frontend/src/components/SecondaryNavigation/SecondaryNavigation.tsx
@@ -0,0 +1,112 @@
+import classNames from "classnames";
+import type { Location } from "react-router-dom";
+import { matchPath, Link, useLocation } from "react-router-dom";
+
+export type NavItem = {
+  label: string;
+  path?: string;
+  items?: NavItem[];
+};
+
+type ItemProps = { item: NavItem };
+
+const SideNavigationLink = ({ item }: ItemProps) => {
+  const location = useLocation();
+  const isActive = getIsActive({ item, location });
+  if (!item.path) {
+    return null;
+  }
+  return (
+    <Link
+      aria-current={isActive ? "page" : undefined}
+      className={classNames("p-side-navigation__link", {
+        "is-active": isActive,
+      })}
+      to={item.path}
+    >
+      {item.label}
+    </Link>
+  );
+};
+const SubNavItem = ({ item }: ItemProps) => {
+  return (
+    <li className="p-side-navigation__item" key={item.path}>
+      <SideNavigationLink item={item} />
+    </li>
+  );
+};
+
+const SubNavigation = ({ items }: { items: NavItem["items"] }) => {
+  if (!items || !items.length) return null;
+
+  return (
+    <ul className="p-side-navigation__list">
+      {items.map((item) => (
+        <SubNavItem item={item} key={item.path} />
+      ))}
+    </ul>
+  );
+};
+
+const getIsActive = ({ item, location }: ItemProps & { location: Location }) => {
+  const path = item.path;
+
+  if (!path) {
+    return false;
+  }
+
+  if (!item.items) {
+    return location.pathname.startsWith(path);
+  }
+
+  return matchPath(path, location.pathname);
+};
+
+const SideNavigationItem = ({ item }: ItemProps) => {
+  const location = useLocation();
+  const isActive = getIsActive({ item, location });
+  const itemClassName = classNames("p-side-navigation__item--title", {
+    "is-active": isActive,
+  });
+
+  return (
+    <li className={itemClassName} key={item.path || item.label}>
+      {item.path ? (
+        <SideNavigationLink item={item} />
+      ) : (
+        <span className="p-side-navigation__text p-side-navigation__item">{item.label}</span>
+      )}
+      {item.items ? <SubNavigation items={item.items} /> : null}
+    </li>
+  );
+};
+
+export const secondaryNavItems = [
+  {
+    label: "Enrollment",
+    items: [
+      { path: "/settings/tokens", label: "Tokens" },
+      {
+        path: "/settings/requests",
+        label: "Requests",
+      },
+    ],
+  },
+];
+
+export const SecondaryNavigation = ({ isOpen }: { isOpen?: boolean }) => {
+  return (
+    <div className={classNames("p-side-navigation is-maas-site-manager is-dark", { "is-open": isOpen })}>
+      <nav className="p-side-navigation__drawer u-padding-top--medium">
+        <h2 className="p-side-navigation__title p-heading--4 p-panel__logo-name">Settings</h2>
+        <ul className="p-side-navigation__list">
+          {secondaryNavItems.map((item) => (
+            <SideNavigationItem item={item} key={item.label} />
+          ))}
+        </ul>
+      </nav>
+    </div>
+  );
+};
+
+export default SecondaryNavigation;
diff --git a/frontend/src/components/SecondaryNavigation/_SecondaryNavigation.scss b/frontend/src/components/SecondaryNavigation/_SecondaryNavigation.scss
new file mode 100644
index 0000000..1533ba7
--- /dev/null
+++ b/frontend/src/components/SecondaryNavigation/_SecondaryNavigation.scss
@@ -0,0 +1,37 @@
+.p-side-navigation.is-maas-site-manager {
+  transition-duration: 0.165s;
+  transition-property: width, box-shadow, background;
+  transition-timing-function: cubic-bezier(0.215, 0.61, 0.355, 1);
+  width: 0;
+  visibility: hidden;
+
+  &.is-open {
+    visibility: visible;
+    width: 100%;
+    @media (min-width: $breakpoint-small) {
+      width: $application-layout--side-nav-width-expanded;
+    }
+  }
+
+  &,
+  .p-side-navigation__drawer {
+    background: #444444;
+    height: 100%;
+  }
+  .p-side-navigation__drawer {
+    transform: none;
+    position: static;
+    max-width: 100%;
+  }
+  .p-side-navigation__title {
+    @extend %vf-heading-4;
+    padding-left: 1.5rem;
+    margin-bottom: 2rem !important;
+  }
+  .p-side-navigation__text,
+  .p-side-navigation__item--title .p-side-navigation__item .p-side-navigation__link {
+    @media (min-width: $breakpoint-small) {
+      padding-left: 1.5rem;
+    }
+  }
+}
diff --git a/frontend/src/components/SecondaryNavigation/index.ts b/frontend/src/components/SecondaryNavigation/index.ts
new file mode 100644
index 0000000..1a07b4d
--- /dev/null
+++ b/frontend/src/components/SecondaryNavigation/index.ts
@@ -0,0 +1 @@
+export { default, secondaryNavItems } from "./SecondaryNavigation";
diff --git a/frontend/src/components/TokensList/TokensList.tsx b/frontend/src/components/TokensList/TokensList.tsx
index a8f878e..2a05b01 100644
--- a/frontend/src/components/TokensList/TokensList.tsx
+++ b/frontend/src/components/TokensList/TokensList.tsx
@@ -2,10 +2,9 @@ import { useState, useEffect } from "react";
 
 import { Button, Col, Row } from "@canonical/react-components";
 
-import PaginationBar from "../base/PaginationBar";
-
 import TokensTable from "./components/TokensTable/TokensTable";
 
+import PaginationBar from "@/components/base/PaginationBar";
 import { useAppContext } from "@/context";
 import { useTokensQuery } from "@/hooks/api";
 import usePagination from "@/hooks/usePagination";
@@ -29,11 +28,6 @@ const TokensList = () => {
   return (
     <section>
       <Row>
-        <Col size={2}>
-          <h2 className="p-heading--4">Tokens</h2>
-        </Col>
-      </Row>
-      <Row>
         <Col size={12}>
           <div className="u-flex u-flex--justify-end">
             <Button>Export</Button>
diff --git a/frontend/src/components/base/PaginationBar/_PaginationBar.scss.scss b/frontend/src/components/base/PaginationBar/_PaginationBar.scss
similarity index 100%
rename from frontend/src/components/base/PaginationBar/_PaginationBar.scss.scss
rename to frontend/src/components/base/PaginationBar/_PaginationBar.scss
diff --git a/frontend/src/pages/settings/index.tsx b/frontend/src/pages/settings/index.tsx
new file mode 100644
index 0000000..478f99e
--- /dev/null
+++ b/frontend/src/pages/settings/index.tsx
@@ -0,0 +1,7 @@
+import { Outlet } from "react-router-dom";
+
+const Settings = () => {
+  return <Outlet />;
+};
+
+export default Settings;
diff --git a/frontend/src/routes.test.tsx b/frontend/src/routes.test.tsx
index 8607e87..418038f 100644
--- a/frontend/src/routes.test.tsx
+++ b/frontend/src/routes.test.tsx
@@ -4,7 +4,7 @@ import { allResolvers } from "./mocks/resolvers";
 
 import { routesConfig } from "@/base/routesConfig";
 import routes from "@/routes";
-import { render, waitFor, setupServer } from "@/test-utils";
+import { render, screen, waitFor, setupServer } from "@/test-utils";
 
 const mockServer = setupServer(...allResolvers);
 
@@ -18,6 +18,7 @@ describe("router", () => {
   });
   afterAll(() => {
     mockServer.close();
+    localStorage.removeItem("jwtToken");
   });
 
   it("redirects to the default route", async () => {
@@ -34,5 +35,10 @@ describe("router", () => {
       render(<RouterProvider router={router} />);
       expect(document.title).toBe(`${title} | MAAS Site Manager`);
     });
+    it(`displays correct heading for ${title} page`, async () => {
+      const router = createMemoryRouter(routes, { initialEntries: [path], initialIndex: 0 });
+      render(<RouterProvider router={router} />);
+      expect(screen.getByRole("heading", { level: 1, name: `${title} | MAAS Site Manager` })).toBeInTheDocument();
+    });
   });
 });
diff --git a/frontend/src/routes/routes.tsx b/frontend/src/routes/routes.tsx
index b1ca0de..740cc44 100644
--- a/frontend/src/routes/routes.tsx
+++ b/frontend/src/routes/routes.tsx
@@ -6,6 +6,7 @@ import MainLayout from "@/components/MainLayout";
 import Login from "@/pages/login";
 import Logout from "@/pages/logout";
 import Requests from "@/pages/requests";
+import Settings from "@/pages/settings";
 import SitesList from "@/pages/sites";
 import Tokens from "@/pages/tokens/tokens";
 
@@ -25,19 +26,15 @@ export const routes = createRoutesFromElements(
     <Route
       element={
         <RequireLogin>
-          <Requests />
+          <Settings />
         </RequireLogin>
       }
-      path="requests"
-    />
-    <Route
-      element={
-        <RequireLogin>
-          <Tokens />
-        </RequireLogin>
-      }
-      path="tokens"
-    />
+      path="settings"
+    >
+      <Route index loader={() => redirect("tokens")} />
+      <Route element={<Tokens />} path="tokens" />
+      <Route element={<Requests />} path="requests" />
+    </Route>
     <Route path="users" />
   </Route>,
 );

References