sts-sponsors team mailing list archive

[Jones Ogolo <mp+440737@xxxxxxxxxxxxxxxxxx>]

Jones Ogolo has proposed merging ~jonesogolo/maas-site-manager:1499-implement-select/deselect-all-table-list into maas-site-manager:main.

Commit message:
Create a select/deselect all component

Requested reviews:
  MAAS Committers (maas-committers)

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

Extracted select/deselect table data logic to separate component and Replaced the inline checkbox implementations for all tables

Steps to QA:
1. Goto /requests
2. Click the checkbox on the table head
3. Ensure all checkboxes are checked
4. Deselect any of the checkboxes in the table body
5. Ensure the checkbox icon at the table head is 'indeterminate'.
-- 
Your team MAAS Committers is requested to review the proposed merge of ~jonesogolo/maas-site-manager:1499-implement-select/deselect-all-table-list into maas-site-manager:main.

diff --git a/frontend/src/components/RequestsTable/RequestsTable.tsx b/frontend/src/components/RequestsTable/RequestsTable.tsx
index 0b88760..eaffdc6 100644
--- a/frontend/src/components/RequestsTable/RequestsTable.tsx
+++ b/frontend/src/components/RequestsTable/RequestsTable.tsx
@@ -8,6 +8,7 @@ import docsUrls from "@/base/docsUrls";
 import DateTime from "@/components/DateTime";
 import ExternalLink from "@/components/ExternalLink";
 import TableCaption from "@/components/TableCaption";
+import SelectAllCheckbox from "@/components/base/SelectAllCheckbox";
 import { isDev } from "@/constants";
 import { useAppContext } from "@/context";
 import type { UseEnrollmentRequestsQueryResult } from "@/hooks/api";
@@ -34,24 +35,7 @@ const RequestsTable = ({
       {
         id: "select",
         accessorKey: "name",
-        header: ({ table }) => (
-          <label className="p-checkbox--inline">
-            <input
-              aria-checked={table.getIsSomeRowsSelected() || table.getIsSomePageRowsSelected() ? "mixed" : undefined}
-              aria-label="select all"
-              className="p-checkbox__input"
-              type="checkbox"
-              {...{
-                checked:
-                  table.getIsSomePageRowsSelected() ||
-                  table.getIsSomeRowsSelected() ||
-                  table.getIsAllPageRowsSelected(),
-                onChange: table.getToggleAllPageRowsSelectedHandler(),
-              }}
-            />
-            <span className="p-checkbox__label" />
-          </label>
-        ),
+        header: ({ table }) => <SelectAllCheckbox table={table} />,
         cell: ({ row, getValue }: { row: Row<EnrollmentRequest>; getValue: Getter<EnrollmentRequest["name"]> }) => {
           return (
             <label className="p-checkbox--inline">
diff --git a/frontend/src/components/SitesList/SitesTable/SitesTable.test.tsx b/frontend/src/components/SitesList/SitesTable/SitesTable.test.tsx
index cdab9f3..ef65b7c 100644
--- a/frontend/src/components/SitesList/SitesTable/SitesTable.test.tsx
+++ b/frontend/src/components/SitesList/SitesTable/SitesTable.test.tsx
@@ -111,3 +111,16 @@ it("displays full name of the country", () => {
 
   expect(screen.getByText("United Kingdom")).toBeInTheDocument();
 });
+
+it("renders a SelectAllCheckbox", () => {
+  const items = siteFactory.buildList(1);
+  render(
+    <SitesTable
+      data={sitesQueryResultFactory.build({ items, total: 1, page: 1, size: 1 })}
+      isFetchedAfterMount={true}
+      isLoading={false}
+      setSearchText={() => {}}
+    />,
+  );
+  expect(screen.getByRole("checkbox", { name: /select all/i })).toBeInTheDocument();
+});
diff --git a/frontend/src/components/SitesList/SitesTable/SitesTable.tsx b/frontend/src/components/SitesList/SitesTable/SitesTable.tsx
index e73b97a..255e727 100644
--- a/frontend/src/components/SitesList/SitesTable/SitesTable.tsx
+++ b/frontend/src/components/SitesList/SitesTable/SitesTable.tsx
@@ -9,7 +9,11 @@ import ConnectionInfo from "./ConnectionInfo/ConnectionInfo";
 import SitesTableControls from "./SitesTableControls/SitesTableControls";
 
 import type { SitesQueryResult } from "@/api/types";
+<<<<<<< frontend/src/components/SitesList/SitesTable/SitesTable.tsx
 import NoRegions from "@/components/NoRegions";
+=======
+import SelectAllCheckbox from "@/components/base/SelectAllCheckbox/SelectAllCheckbox";
+>>>>>>> frontend/src/components/SitesList/SitesTable/SitesTable.tsx
 import { isDev } from "@/constants";
 import { useAppContext } from "@/context";
 import type { UseSitesQueryResult } from "@/hooks/api";
@@ -48,24 +52,7 @@ const SitesTable = ({
       {
         id: "select",
         accessorKey: "name",
-        header: ({ table }) => (
-          <label className="p-checkbox--inline">
-            <input
-              aria-checked={table.getIsSomeRowsSelected() || table.getIsSomePageRowsSelected() ? "mixed" : undefined}
-              aria-label="select all"
-              className="p-checkbox__input"
-              type="checkbox"
-              {...{
-                checked:
-                  table.getIsSomePageRowsSelected() ||
-                  table.getIsSomeRowsSelected() ||
-                  table.getIsAllPageRowsSelected(),
-                onChange: table.getToggleAllPageRowsSelectedHandler(),
-              }}
-            />
-            <span className="p-checkbox__label" />
-          </label>
-        ),
+        header: ({ table }) => <SelectAllCheckbox table={table} />,
         cell: ({ row, getValue }: { row: Row<Site>; getValue: Getter<Site["name"]> }) => {
           return (
             <label className="p-checkbox--inline">
diff --git a/frontend/src/components/TokensList/components/TokensTable/TokensTable.tsx b/frontend/src/components/TokensList/components/TokensTable/TokensTable.tsx
index 6de9df3..100ae6f 100644
--- a/frontend/src/components/TokensList/components/TokensTable/TokensTable.tsx
+++ b/frontend/src/components/TokensList/components/TokensTable/TokensTable.tsx
@@ -8,6 +8,7 @@ import pick from "lodash/fp/pick";
 
 import type { Token } from "@/api/types";
 import CopyButton from "@/components/base/CopyButton";
+import SelectAllCheckbox from "@/components/base/SelectAllCheckbox/SelectAllCheckbox";
 import type { useTokensQueryResult } from "@/hooks/api";
 import { copyToClipboard } from "@/utils";
 
@@ -42,18 +43,7 @@ const TokensTable = ({
     () => [
       {
         id: "select",
-        header: ({ table }) => (
-          <div>
-            <Input
-              checked={table.getIsAllRowsSelected()}
-              onChange={table.getToggleAllRowsSelectedHandler()}
-              type="checkbox"
-              {...{
-                indeterminate: table.getIsSomeRowsSelected(),
-              }}
-            />
-          </div>
-        ),
+        header: ({ table }) => <SelectAllCheckbox table={table} />,
         cell: ({ row }) => (
           <div>
             <Input
diff --git a/frontend/src/components/TokensList/components/TokensTable/_TokensTable.scss b/frontend/src/components/TokensList/components/TokensTable/_TokensTable.scss
index 4215dfb..42824a1 100644
--- a/frontend/src/components/TokensList/components/TokensTable/_TokensTable.scss
+++ b/frontend/src/components/TokensList/components/TokensTable/_TokensTable.scss
@@ -1,4 +1,5 @@
 .tokens-table {
+  margin-top: $spv--medium;
   thead th:first-child {
     width: 3rem;
   }
diff --git a/frontend/src/components/base/SelectAllCheckbox/SelectAllCheckbox.tsx b/frontend/src/components/base/SelectAllCheckbox/SelectAllCheckbox.tsx
new file mode 100644
index 0000000..5f2050c
--- /dev/null
+++ b/frontend/src/components/base/SelectAllCheckbox/SelectAllCheckbox.tsx
@@ -0,0 +1,26 @@
+import type { Table } from "@tanstack/react-table";
+
+type Props<T> = {
+  table: Table<T>;
+};
+
+function SelectAllCheckbox<T>({ table }: Props<T>) {
+  return (
+    <label className="p-checkbox--inline">
+      <input
+        aria-checked={table.getIsSomeRowsSelected() || table.getIsSomePageRowsSelected() ? "mixed" : undefined}
+        aria-label="select all"
+        className="p-checkbox__input"
+        type="checkbox"
+        {...{
+          checked:
+            table.getIsSomePageRowsSelected() || table.getIsSomeRowsSelected() || table.getIsAllPageRowsSelected(),
+          onChange: table.getToggleAllPageRowsSelectedHandler(),
+        }}
+      />
+      <span className="p-checkbox__label" />
+    </label>
+  );
+}
+
+export default SelectAllCheckbox;
diff --git a/frontend/src/components/base/SelectAllCheckbox/index.ts b/frontend/src/components/base/SelectAllCheckbox/index.ts
new file mode 100644
index 0000000..a7b05a3
--- /dev/null
+++ b/frontend/src/components/base/SelectAllCheckbox/index.ts
@@ -0,0 +1 @@
+export { default } from "./SelectAllCheckbox";
diff --git a/frontend/tests/requests.spec.ts b/frontend/tests/requests.spec.ts
index ec0163c..594fe9f 100644
--- a/frontend/tests/requests.spec.ts
+++ b/frontend/tests/requests.spec.ts
@@ -12,3 +12,12 @@ test("goes to the regions page if the user clicks on the regions link", async ({
   await page.getByRole("button", { name: /Go to Regions/i }).click();
   await expect(page.getByRole("heading", { name: /regions/i })).toBeVisible();
 });
+
+test("Clicking 'select all' checkbox selects all regions", async ({ page }) => {
+  await page.getByRole("checkbox", { name: /select all/i }).click({ force: true });
+  const checkboxes = await page.$$('input[type="checkbox"]');
+  for (const checkbox of checkboxes) {
+    const isChecked = await checkbox.isChecked();
+    expect(isChecked).toBe(true);
+  }
+});