sts-sponsors team mailing list archive
-
sts-sponsors team
-
Mailing list archive
-
Message #07285
[Merge] ~petermakowski/maas-site-manager:select-all-checkbox-1499 into maas-site-manager:main
Peter Makowski has proposed merging ~petermakowski/maas-site-manager:select-all-checkbox-1499 into maas-site-manager:main.
Commit message:
select all for all table views MAASENG-1499
- extract select all logic to SelectAllCheckbox component
- add custom handler for selection of all items
Requested reviews:
MAAS Committers (maas-committers)
For more details, see:
https://code.launchpad.net/~petermakowski/maas-site-manager/+git/site-manager/+merge/441550
QA Steps
Click the header row checkbox
Verify that everything on the current page has been selected
Go to the next page
Verify the header row checkbox is partially checked
Click the header row checkbox
Verify that all items on the current page are selected
Go to the previous page
Verify that all items are still selected
Click on header row checkbox
Verify that all items on all pages have been deselected
--
Your team MAAS Committers is requested to review the proposed merge of ~petermakowski/maas-site-manager:select-all-checkbox-1499 into maas-site-manager:main.
diff --git a/frontend/src/components/RequestsTable/RequestsTable.tsx b/frontend/src/components/RequestsTable/RequestsTable.tsx
index 0b88760..b4ccfc1 100644
--- a/frontend/src/components/RequestsTable/RequestsTable.tsx
+++ b/frontend/src/components/RequestsTable/RequestsTable.tsx
@@ -7,6 +7,7 @@ import type { EnrollmentRequest } from "@/api/types";
import docsUrls from "@/base/docsUrls";
import DateTime from "@/components/DateTime";
import ExternalLink from "@/components/ExternalLink";
+import SelectAllCheckbox from "@/components/SelectAllCheckbox";
import TableCaption from "@/components/TableCaption";
import { isDev } from "@/constants";
import { useAppContext } from "@/context";
@@ -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/SelectAllCheckbox/SelectAllCheckbox.test.tsx b/frontend/src/components/SelectAllCheckbox/SelectAllCheckbox.test.tsx
new file mode 100644
index 0000000..4c66f14
--- /dev/null
+++ b/frontend/src/components/SelectAllCheckbox/SelectAllCheckbox.test.tsx
@@ -0,0 +1,75 @@
+import type { ColumnDef } from "@tanstack/react-table";
+import { useReactTable, flexRender, getCoreRowModel } from "@tanstack/react-table";
+
+import SelectAllCheckbox from "./SelectAllCheckbox";
+
+import { render, screen } from "@/test-utils";
+
+type ColDef = ColumnDef<unknown, unknown>;
+
+const columns: ColDef[] = [
+ {
+ id: "select",
+ header: ({ table }) => <SelectAllCheckbox table={table} />,
+ cell: ({ row }) => {
+ return (
+ <input
+ className="p-checkbox__input"
+ type="checkbox"
+ {...{
+ checked: row.getIsSelected(),
+ disabled: !row.getCanSelect(),
+ onChange: row.getToggleSelectedHandler(),
+ }}
+ />
+ );
+ },
+ },
+];
+
+const SelectAllCheckboxWithTable = () => {
+ const table = useReactTable<unknown>({
+ data: Array.from({ length: 100 }, (_, i) => i + 1),
+ columns,
+ getCoreRowModel: getCoreRowModel(),
+ });
+ return (
+ <table aria-label="sites" className="sites-table">
+ <thead>
+ {table.getHeaderGroups().map((headerGroup) => (
+ <tr key={headerGroup.id}>
+ {headerGroup.headers.map((header) => {
+ return (
+ <th className={`${header.column.id}`} colSpan={header.colSpan} key={header.id}>
+ {flexRender(header.column.columnDef.header, header.getContext())}
+ </th>
+ );
+ })}
+ </tr>
+ ))}
+ </thead>
+ <tbody>
+ {table.getRowModel().rows.map((row) => {
+ return (
+ <tr key={row.id}>
+ {row.getVisibleCells().map((cell) => {
+ return (
+ <td className={`${cell.column.id}`} key={cell.id}>
+ {flexRender(cell.column.columnDef.cell, cell.getContext())}
+ </td>
+ );
+ })}
+ </tr>
+ );
+ })}
+ </tbody>
+ </table>
+ );
+};
+
+it("toggles select all checkbox on click", async () => {
+ render(<SelectAllCheckboxWithTable />);
+ await expect(screen.getByRole("checkbox", { name: /select all/i })).not.toBeChecked();
+ await screen.getByRole("checkbox", { name: /select all/i }).click();
+ await expect(screen.getByRole("checkbox", { name: /select all/i })).toBeChecked();
+});
diff --git a/frontend/src/components/SelectAllCheckbox/SelectAllCheckbox.tsx b/frontend/src/components/SelectAllCheckbox/SelectAllCheckbox.tsx
new file mode 100644
index 0000000..27fc9b2
--- /dev/null
+++ b/frontend/src/components/SelectAllCheckbox/SelectAllCheckbox.tsx
@@ -0,0 +1,49 @@
+import type { RowData, Table } from "@tanstack/react-table";
+
+import { useAppContext } from "@/context";
+
+type Props<T> = {
+ table: Table<T>;
+};
+
+// TODO: replace with table.getToggleAllPageRowsSelectedHandler once the issue below is fixed
+// https://github.com/TanStack/table/issues/4781
+const selectAllPageRows = <T extends RowData>(table: Table<T>) =>
+ table.setRowSelection((previousSelection) => {
+ return {
+ ...previousSelection,
+ ...Object.keys(table.getRowModel().rowsById).reduce((acc, id) => ({ ...acc, [id]: true }), {}),
+ };
+ });
+
+function SelectAllCheckbox<T>({ table }: Props<T>) {
+ // TODO: remove this workaround once the issue below is fixed
+ // https://github.com/TanStack/table/issues/4781
+ // manually check if some rows are selected as getIsSomePageRowsSelected
+ // returns false if there are any rows selected on other pages
+ const { rowSelection } = useAppContext();
+ const isSomeRowsSelected = !table.getIsAllPageRowsSelected() && Object.keys(rowSelection).length > 0;
+ return (
+ <label className="p-checkbox--inline">
+ <input
+ aria-checked={isSomeRowsSelected ? "mixed" : undefined}
+ aria-label="select all"
+ className="p-checkbox__input"
+ type="checkbox"
+ {...{
+ checked: isSomeRowsSelected || table.getIsAllPageRowsSelected(),
+ onChange: () => {
+ if (table.getIsAllPageRowsSelected()) {
+ table.resetRowSelection();
+ } else {
+ selectAllPageRows<T>(table);
+ }
+ },
+ }}
+ />
+ <span className="p-checkbox__label" />
+ </label>
+ );
+}
+
+export default SelectAllCheckbox;
diff --git a/frontend/src/components/SelectAllCheckbox/index.ts b/frontend/src/components/SelectAllCheckbox/index.ts
new file mode 100644
index 0000000..a7b05a3
--- /dev/null
+++ b/frontend/src/components/SelectAllCheckbox/index.ts
@@ -0,0 +1 @@
+export { default } from "./SelectAllCheckbox";
diff --git a/frontend/src/components/SitesList/SitesTable/SitesTable.tsx b/frontend/src/components/SitesList/SitesTable/SitesTable.tsx
index 0190168..e7e2e53 100644
--- a/frontend/src/components/SitesList/SitesTable/SitesTable.tsx
+++ b/frontend/src/components/SitesList/SitesTable/SitesTable.tsx
@@ -11,6 +11,7 @@ import SitesTableControls from "./SitesTableControls/SitesTableControls";
import type { SitesQueryResult } from "@/api/types";
import ExternalLink from "@/components/ExternalLink";
import NoRegions from "@/components/NoRegions";
+import SelectAllCheckbox from "@/components/SelectAllCheckbox";
import { isDev } from "@/constants";
import { useAppContext } from "@/context";
import type { UseSitesQueryResult } from "@/hooks/api";
@@ -49,24 +50,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..ae3b4eb 100644
--- a/frontend/src/components/TokensList/components/TokensTable/TokensTable.tsx
+++ b/frontend/src/components/TokensList/components/TokensTable/TokensTable.tsx
@@ -7,7 +7,9 @@ import { format, formatDistanceStrict } from "date-fns";
import pick from "lodash/fp/pick";
import type { Token } from "@/api/types";
+import SelectAllCheckbox from "@/components/SelectAllCheckbox";
import CopyButton from "@/components/base/CopyButton";
+import { useAppContext } from "@/context";
import type { useTokensQueryResult } from "@/hooks/api";
import { copyToClipboard } from "@/utils";
@@ -42,18 +44,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
@@ -61,9 +52,6 @@ const TokensTable = ({
disabled={!row.getCanSelect()}
onChange={row.getToggleSelectedHandler()}
type="checkbox"
- {...{
- indeterminate: row.getIsSomeSelected(),
- }}
/>
</div>
),
@@ -103,7 +91,9 @@ const TokensTable = ({
],
[handleTokenCopy, isTokenCopied],
);
- const [rowSelection, setRowSelection] = useState({});
+
+ const { rowSelection, setRowSelection } = useAppContext();
+
const noItems = useMemo<Token[]>(() => [], []);
const tokenTable = useReactTable<Token>({
@@ -112,9 +102,11 @@ const TokensTable = ({
state: {
rowSelection,
},
+ getRowId: (row) => row.name,
+ manualPagination: true,
enableRowSelection: true,
getCoreRowModel: getCoreRowModel(),
- columnResizeMode: "onChange",
+ enableMultiRowSelection: true,
onRowSelectionChange: setRowSelection,
});
return (
diff --git a/frontend/tests/components/table.spec.ts b/frontend/tests/components/table.spec.ts
new file mode 100644
index 0000000..ef8ea04
--- /dev/null
+++ b/frontend/tests/components/table.spec.ts
@@ -0,0 +1,72 @@
+import { test, expect } from "@playwright/test";
+
+import { adminAuthFile } from "../constants";
+import { routesConfig } from "@/base/routesConfig";
+
+test.use({ storageState: adminAuthFile });
+
+const pagesWithTable = [routesConfig.tokens, routesConfig.requests, routesConfig.sites];
+
+for (const pageWithTable of pagesWithTable) {
+ test(`${pageWithTable.title} - clicking unchecked 'select all' checkbox selects all items on the current page`, async ({
+ page,
+ }) => {
+ await page.goto(pageWithTable.path);
+ await page.getByRole("checkbox", { name: /select all/i }).click({ force: true });
+ const tableBodyCheckboxes = await page.locator("tbody").getByRole("checkbox").all();
+ for await (const checkbox of tableBodyCheckboxes) {
+ await expect(checkbox).toBeChecked();
+ }
+ await page.getByRole("checkbox", { name: /select all/i }).click({ force: true });
+ for await (const checkbox of tableBodyCheckboxes) {
+ await expect(checkbox).not.toBeChecked();
+ }
+ });
+
+ test(`${pageWithTable.title} - clicking partially checked 'select all' checkbox selects all items on the current page`, async ({
+ page,
+ }) => {
+ await page.goto(pageWithTable.path);
+ const selectAll = await page.getByRole("checkbox", { name: /select all/i });
+ const tableBodyCheckboxes = await page.locator("tbody").getByRole("checkbox").all();
+ // click a single item on the first page
+ await await page.locator("tbody").getByRole("checkbox").first().click({ force: true });
+ await page.getByRole("button", { name: "next page" }).click();
+ await expect(selectAll).toHaveAttribute("aria-checked", "mixed");
+ // click partially checked select all checkbox on the second page
+ await selectAll.click({ force: true });
+ await expect(selectAll).toBeChecked();
+ await expect(selectAll).not.toHaveAttribute("aria-checked", "mixed");
+ for await (const checkbox of tableBodyCheckboxes) {
+ await expect(checkbox).toBeChecked();
+ }
+ await page.getByRole("button", { name: "next page" }).click();
+ for await (const checkbox of tableBodyCheckboxes) {
+ await expect(checkbox).not.toBeChecked();
+ }
+ });
+
+ test(`${pageWithTable.title} - clicking checked select all checkbox deselects items on all pages`, async ({
+ page,
+ }) => {
+ await page.goto(pageWithTable.path);
+ const selectAll = await page.getByRole("checkbox", { name: /select all/i });
+ const tableBodyCheckboxes = await page.locator("tbody").getByRole("checkbox").all();
+ // select all items on the first page
+ await selectAll.click({ force: true });
+ await page.getByRole("button", { name: "next page" }).click();
+ await expect(selectAll).toHaveAttribute("aria-checked", "mixed");
+ // select all items on the second page
+ await selectAll.click({ force: true });
+ await expect(selectAll).toBeChecked();
+ // click selected 'select all' checkbox on the second page
+ await selectAll.click({ force: true });
+ for await (const checkbox of tableBodyCheckboxes) {
+ await expect(checkbox).not.toBeChecked();
+ }
+ await page.getByRole("button", { name: "previous page" }).click();
+ for await (const checkbox of tableBodyCheckboxes) {
+ await expect(checkbox).not.toBeChecked();
+ }
+ });
+}
Follow ups
