← Back to team overview

sts-sponsors team mailing list archive

[Merge] ~petermakowski/maas-site-manager:add-side-panel-MAASENG-1487 into maas-site-manager:main

 

Peter Makowski has proposed merging ~petermakowski/maas-site-manager:add-side-panel-MAASENG-1487 into maas-site-manager:main.

Commit message:
feat: add side panel MAASENG-1487
- move token generation form to the side panel
- refactor navigation to use location state to display the active side panel
- add end to end test for creation of tokens

Requested reviews:
  MAAS Committers (maas-committers)

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

QA Steps:
- Go to /sites
- Click on Settings in the navigation to go to Tokens
- click “Generate tokens 
- Go back in browser history e.g. using the “back” button in the browser
- Go forward in browser history, e.g. by using the “forward” button in the browser
- Verify the Generate tokens form remains open as you navigate back to the tokens page
- Click on “sites in the navigation
- Click on tokens in the navigation
- Make sure the token generation form is closed
—
- Click on generate tokens
- Click on “cancel
- Make sure the form has been closed
- Click on generate tokens
- Fill out the form and submit
- Make sure the form has been closed after successful submission
—
- verify the side panel is opened, closed and displayed correctly and all content visible on mobile
-- 
Your team MAAS Committers is requested to review the proposed merge of ~petermakowski/maas-site-manager:add-side-panel-MAASENG-1487 into maas-site-manager:main.
diff --git a/frontend/src/components/MainLayout/MainLayout.scss b/frontend/src/components/MainLayout/MainLayout.scss
index 12ae5c8..26594be 100644
--- a/frontend/src/components/MainLayout/MainLayout.scss
+++ b/frontend/src/components/MainLayout/MainLayout.scss
@@ -1,3 +1,7 @@
 .l-main.is-maas-site-manager {
-  margin-top: 1.5rem;
+  margin-top: $sph--large;
+}
+.l-aside.is-maas-site-manager {
+  background: $color-x-light;
+  padding-top: $sph--large;
 }
diff --git a/frontend/src/components/MainLayout/MainLayout.tsx b/frontend/src/components/MainLayout/MainLayout.tsx
index 3f37f7c..0032450 100644
--- a/frontend/src/components/MainLayout/MainLayout.tsx
+++ b/frontend/src/components/MainLayout/MainLayout.tsx
@@ -1,20 +1,41 @@
-import { Outlet } from "react-router-dom";
+import { Col, Row } from "@canonical/react-components";
+import classNames from "classnames";
+import { Outlet, useLocation } from "react-router-dom";
 
 import "./MainLayout.scss";
 import Navigation from "@/components/Navigation";
+import TokensCreate from "@/pages/tokens/create";
 
-const MainLayout: React.FC = () => (
-  <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 />
-        </div>
-      </div>
-    </main>
-  </div>
+const Aside = ({ children, isOpen }: { children: React.ReactNode; isOpen: boolean }) => (
+  <aside
+    aria-label="Add machine"
+    className={classNames("l-aside", "is-maas-site-manager", { "is-collapsed": !isOpen })}
+    data-testid="section-header-content"
+    id="aside-panel"
+  >
+    <Row>
+      <Col size={12}>{children}</Col>
+    </Row>
+  </aside>
 );
 
+const MainLayout: React.FC = () => {
+  const { state, pathname } = useLocation();
+
+  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 />
+          </div>
+        </div>
+      </main>
+      <Aside isOpen={!!state?.sidebar}>{pathname === "/tokens" ? <TokensCreate /> : null}</Aside>
+    </div>
+  );
+};
+
 export default MainLayout;
diff --git a/frontend/src/components/Navigation/Navigation.tsx b/frontend/src/components/Navigation/Navigation.tsx
index f2b3a13..1849bba 100644
--- a/frontend/src/components/Navigation/Navigation.tsx
+++ b/frontend/src/components/Navigation/Navigation.tsx
@@ -33,7 +33,7 @@ const Navigation = (): JSX.Element => {
 
   return (
     <>
-      <header className="l-navigation-bar">
+      <header aria-label="navigation" className="l-navigation-bar">
         <div className="p-panel is-dark">
           <div className="p-panel__header">
             <NavigationBanner />
@@ -52,7 +52,7 @@ const Navigation = (): JSX.Element => {
         </div>
       </header>
       <nav
-        aria-label="main navigation"
+        aria-label="main"
         className={classNames("l-navigation", { "is-collapsed": isCollapsed, "is-pinned": !isCollapsed })}
       >
         <div className="l-navigation__drawer">
diff --git a/frontend/src/components/TokensCreate/TokensCreate.scss b/frontend/src/components/TokensCreate/TokensCreate.scss
new file mode 100644
index 0000000..3fc3031
--- /dev/null
+++ b/frontend/src/components/TokensCreate/TokensCreate.scss
@@ -0,0 +1,17 @@
+.tokens-create {
+  .tokens-create__heading {
+    margin-bottom: $sph--x-large;
+  }
+  hr.tokens-create__separator {
+    margin-bottom: $sph--large;
+  }
+  .tokens-create__buttons {
+    display: flex;
+    justify-content: flex-end;
+    width: 100%;
+    a,
+    button {
+      margin-left: $sph--large;
+    }
+  }
+}
diff --git a/frontend/src/components/TokensCreate/TokensCreate.test.tsx b/frontend/src/components/TokensCreate/TokensCreate.test.tsx
index b2a82f7..da95903 100644
--- a/frontend/src/components/TokensCreate/TokensCreate.test.tsx
+++ b/frontend/src/components/TokensCreate/TokensCreate.test.tsx
@@ -5,7 +5,7 @@ import { vi } from "vitest";
 import TokensCreate from "./TokensCreate";
 
 import urls from "@/api/urls";
-import { render, screen, userEvent } from "@/test-utils";
+import { renderWithMemoryRouter, screen, userEvent } from "@/test-utils";
 
 const postTokensEndpointMock = vi.fn();
 const mockServer = setupServer(
@@ -28,12 +28,12 @@ afterAll(() => {
 
 describe("TokensCreate", () => {
   it("renders the form", async () => {
-    render(<TokensCreate />);
+    renderWithMemoryRouter(<TokensCreate />);
     expect(screen.getByRole("form", { name: /Generate new enrollment tokens/i })).toBeInTheDocument();
   });
 
   it("if not all required fields have been entered the submit button is disabled", async () => {
-    render(<TokensCreate />);
+    renderWithMemoryRouter(<TokensCreate />);
     const amount = screen.getByLabelText(/Amount of tokens to generate/i);
     const expires = screen.getByLabelText(/Expiration time/i);
     expect(screen.getByRole("button", { name: /Generate tokens/i })).toBeDisabled();
@@ -43,7 +43,7 @@ describe("TokensCreate", () => {
   });
 
   it("displays an error for invalid expiration value", async () => {
-    render(<TokensCreate />);
+    renderWithMemoryRouter(<TokensCreate />);
     const expires = screen.getByLabelText(/Expiration time/i);
     await userEvent.type(expires, "2");
     await userEvent.tab();
@@ -53,7 +53,7 @@ describe("TokensCreate", () => {
   });
 
   it("can generate enrolment tokens", async () => {
-    render(<TokensCreate />);
+    renderWithMemoryRouter(<TokensCreate />);
     const amount = screen.getByLabelText(/Amount of tokens to generate/i);
     const expires = screen.getByLabelText(/Expiration time/i);
     expect(screen.getByRole("button", { name: /Generate tokens/i })).toBeDisabled();
diff --git a/frontend/src/components/TokensCreate/TokensCreate.tsx b/frontend/src/components/TokensCreate/TokensCreate.tsx
index cf8d963..f347540 100644
--- a/frontend/src/components/TokensCreate/TokensCreate.tsx
+++ b/frontend/src/components/TokensCreate/TokensCreate.tsx
@@ -3,8 +3,10 @@ import { useId } from "react";
 import { Button, Input, Label, Notification } from "@canonical/react-components";
 import { useMutation } from "@tanstack/react-query";
 import { Field, Formik, Form } from "formik";
+import { Link, useNavigate, useNavigation } from "react-router-dom";
 import * as Yup from "yup";
 
+import "./TokensCreate.scss";
 import { humanIntervalToISODuration } from "./utils";
 
 import { postTokens } from "@/api/handlers";
@@ -40,6 +42,7 @@ const TokensCreate = () => {
   const headingId = useId();
   const expiresId = useId();
   const amountId = useId();
+  const navigate = useNavigate();
   const mutation = useMutation(postTokens);
   const handleSubmit = async (
     { amount, expires }: TokensCreateFormValues,
@@ -49,15 +52,18 @@ const TokensCreate = () => {
     // TODO: update the tokens list once fetching tokens from API is implemented
     // https://warthogs.atlassian.net/browse/MAASENG-1474
     setSubmitting(false);
+    navigate("/tokens", { state: { sidebar: false } });
   };
 
   return (
-    <div>
-      <h3 id={headingId}>Generate new enrollment tokens</h3>
+    <div className="tokens-create">
+      <h3 className="tokens-create__heading p-heading--4" id={headingId}>
+        Generate new enrollment tokens
+      </h3>
       {mutation.isError && <Notification severity="negative">There was an error generating the token(s).</Notification>}
       <Formik initialValues={initialValues} onSubmit={handleSubmit} validationSchema={TokensCreateSchema}>
         {({ isSubmitting, errors, touched, isValid, dirty }) => (
-          <Form aria-labelledby={headingId} noValidate>
+          <Form aria-labelledby={headingId} className="tokens-create" noValidate>
             <Label htmlFor={amountId}>Amount of tokens to generate</Label>
             <Field
               as={Input}
@@ -77,17 +83,22 @@ const TokensCreate = () => {
               type="text"
             />
             <p className="u-text--muted">
-              Use this token once to request an enrolment in the specified timeframe. <br />
-              Allowed time units are seconds, minutes, hours, days and weeks.
+              Use this token once to request an enrolment in the specified timeframe. Allowed time units are seconds,
+              minutes, hours, days and weeks.
             </p>
-            <Button type="button">Cancel</Button>
-            <Button
-              appearance="positive"
-              disabled={!dirty || !isValid || mutation.isLoading || isSubmitting}
-              type="submit"
-            >
-              Generate tokens
-            </Button>
+            <hr className="tokens-create__separator" />
+            <div className="tokens-create__buttons">
+              <Link className="p-button" role="button" state={{ sidebar: false }} to="tokens">
+                Cancel
+              </Link>
+              <Button
+                appearance="positive"
+                disabled={!dirty || !isValid || mutation.isLoading || isSubmitting}
+                type="submit"
+              >
+                Generate tokens
+              </Button>
+            </div>
           </Form>
         )}
       </Formik>
diff --git a/frontend/src/components/TokensList/TokensList.tsx b/frontend/src/components/TokensList/TokensList.tsx
index 4125a07..a366f95 100644
--- a/frontend/src/components/TokensList/TokensList.tsx
+++ b/frontend/src/components/TokensList/TokensList.tsx
@@ -3,25 +3,25 @@ import { Outlet, Link } from "react-router-dom";
 
 const TokensList = () => {
   return (
-    <section>
-      <Row>
-        <Col size={2}>
-          <h2 className="p-heading--4">Tokens</h2>
-        </Col>
-      </Row>
-      <Row>
-        <Col size={12}>
-          <Button>Export</Button>
-          <Button appearance="negative">Delete</Button>
-          <Link to="create">
-            <Button appearance="positive" element="a">
+    <>
+      <section>
+        <Row>
+          <Col size={2}>
+            <h2 className="p-heading--4">Tokens</h2>
+          </Col>
+        </Row>
+        <Row>
+          <Col size={12}>
+            <Button>Export</Button>
+            <Button appearance="negative">Delete</Button>
+            <Link className="p-button--positive" role="button" state={{ sidebar: true }} to="">
               Generate tokens
-            </Button>
-          </Link>
-        </Col>
-      </Row>
+            </Link>
+          </Col>
+        </Row>
+      </section>
       <Outlet />
-    </section>
+    </>
   );
 };
 
diff --git a/frontend/src/routes.tsx b/frontend/src/routes.tsx
index 5009017..2cd8876 100644
--- a/frontend/src/routes.tsx
+++ b/frontend/src/routes.tsx
@@ -2,7 +2,6 @@ import { createRoutesFromElements, Route, redirect } from "react-router-dom";
 
 import MainLayout from "@/components/MainLayout";
 import SitesList from "@/pages/sites";
-import TokensCreate from "@/pages/tokens/create";
 import Tokens from "@/pages/tokens/tokens";
 
 export const routes = createRoutesFromElements(
@@ -16,9 +15,7 @@ export const routes = createRoutesFromElements(
     <Route path="logout" />
     <Route element={<SitesList />} path="sites" />
     <Route path="requests" />
-    <Route element={<Tokens />} path="tokens">
-      <Route element={<TokensCreate />} path="create" />
-    </Route>
+    <Route element={<Tokens />} path="tokens" />
     <Route path="users" />
   </Route>,
 );
diff --git a/frontend/tests/tokens.spec.ts b/frontend/tests/tokens.spec.ts
new file mode 100644
index 0000000..1230a72
--- /dev/null
+++ b/frontend/tests/tokens.spec.ts
@@ -0,0 +1,47 @@
+import { test, expect } from "@playwright/test";
+
+test.beforeEach(async ({ page }) => {
+  await page.goto("/tokens");
+});
+
+test("can open and close token generate form", async ({ page }) => {
+  await page.getByRole("button", { name: /Generate tokens/i }).click();
+  expect(page.getByRole("form", { name: /Generate new enrollment tokens/i })).toBeVisible();
+  await page.getByRole("button", { name: /Cancel/i }).click();
+  await expect(page.getByRole("form", { name: /Generate new enrollment tokens/i })).toBeHidden();
+});
+
+test("token create form persists its open state when navigating back", async ({ page }) => {
+  await page.getByRole("button", { name: /Generate tokens/i }).click();
+  await expect(page.getByRole("form", { name: /Generate new enrollment tokens/i })).toBeVisible();
+
+  const mobileBanner = await page.getByRole("banner", { name: /navigation/i }).isVisible();
+  if (mobileBanner) {
+    await page
+      .getByRole("banner", { name: /navigation/ })
+      .getByRole("button", { name: /menu/i })
+      .click();
+  } else {
+    await page.getByRole("navigation", { name: /main/i }).hover();
+  }
+  await page
+    .getByRole("navigation", { name: /main/i })
+    .getByRole("link", { name: /Overview/ })
+    .click();
+
+  await page.goBack();
+  await expect(page.getByRole("form", { name: /Generate new enrollment tokens/i })).toBeVisible();
+});
+
+test("closes the form after creating the token", async ({ page }) => {
+  await page.getByRole("button", { name: /Generate tokens/i }).click();
+
+  await page.getByRole("textbox", { name: /Amount of tokens to generate/i }).type("1");
+  await page.getByRole("textbox", { name: /Expiration time/i }).type("1 week");
+  await page
+    .getByRole("form", { name: /Generate new enrollment tokens/i })
+    .getByRole("button", { name: /Generate tokens/i })
+    .click();
+  await page.waitForResponse((resp) => resp.url().includes("/tokens") && resp.status() === 200);
+  await expect(page.getByRole("form", { name: /Generate new enrollment tokens/i })).toBeHidden();
+});

Follow ups