Peter Makowski has proposed merging
~petermakowski/maas-site-manager:separate-context-providers-for-different-views-MAASENG-1549
into maas-site-manager:main.
Commit message:
separate context for different views MAASENG-1549
- extract RowSelectionContextProviders
- move app context consumer logic to Aside
Requested reviews:
MAAS Lander (maas-lander)
MAAS Committers (maas-committers)
For more details, see:
https://code.launchpad.net/~petermakowski/maas-site-manager/+git/site-manager/+merge/442736
QA Steps
Verify that selection of table items for all tables (sites, requests, tokens)
works correctly and selection is cleared on route change
--
Your team MAAS Committers is requested to review the proposed merge of
~petermakowski/maas-site-manager:separate-context-providers-for-different-views-MAASENG-1549
into maas-site-manager:main.
diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx
index 1bfb4ba..fead2d5 100644
--- a/frontend/src/App.tsx
+++ b/frontend/src/App.tsx
@@ -1,6 +1,8 @@
import "./App.scss";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
+import { RowSelectionContextProviders } from "./context/RowSelectionContext";
+
import apiClient from "@/api";
import { AppContextProvider, AuthContextProvider } from "@/context";
import { createBrowserRouter, RouterProvider } from "@/router";
@@ -14,7 +16,9 @@ const App: React.FC = () => {
<QueryClientProvider client={queryClient}>
<AppContextProvider>
<AuthContextProvider apiClient={apiClient}>
- <RouterProvider router={router} />
+ <RowSelectionContextProviders>
+ <RouterProvider router={router} />
+ </RowSelectionContextProviders>
</AuthContextProvider>
</AppContextProvider>
</QueryClientProvider>
diff --git a/frontend/src/components/EnrollmentActions/EnrollmentActions.tsx b/frontend/src/components/EnrollmentActions/EnrollmentActions.tsx
index ed4f073..25f3361 100644
--- a/frontend/src/components/EnrollmentActions/EnrollmentActions.tsx
+++ b/frontend/src/components/EnrollmentActions/EnrollmentActions.tsx
@@ -2,11 +2,11 @@ import { Button, Notification } from "@canonical/react-components";
import EnrollmentNotification from "./EnrollmentNotification";
-import { useAppContext } from "@/context";
+import { useRowSelectionContext } from "@/context/RowSelectionContext";
import { useEnrollmentRequestsMutation } from "@/hooks/react-query";
const EnrollmentActions: React.FC = () => {
- const { rowSelection, setRowSelection } = useAppContext();
+ const { rowSelection, setRowSelection } = useRowSelectionContext("requests");
const selectedIds = Object.keys(rowSelection).map((id) => id);
const enrollmentRequestsMutation = useEnrollmentRequestsMutation({ onSuccess: () => setRowSelection({}) });
const isActionDisabled = Object.keys(rowSelection).length === 0 || enrollmentRequestsMutation.isLoading;
diff --git a/frontend/src/components/MainLayout/MainLayout.tsx b/frontend/src/components/MainLayout/MainLayout.tsx
index 9be3fca..3fe61c0 100644
--- a/frontend/src/components/MainLayout/MainLayout.tsx
+++ b/frontend/src/components/MainLayout/MainLayout.tsx
@@ -1,4 +1,3 @@
-import type { PropsWithChildren } from "react";
import { useEffect } from "react";
import { Col, Row, useOnEscapePressed, usePrevious } from "@canonical/react-components";
@@ -19,21 +18,39 @@ export const sidebarLabels: Record<"removeRegions" | "createToken", string> = {
createToken: "Generate tokens",
};
-type AsideProps = PropsWithChildren<Pick<ReturnType<typeof useAppContext>, "sidebar" | "setSidebar">>;
-const Aside = ({ children, sidebar, setSidebar, ...props }: AsideProps) => {
+const Aside = () => {
+ const { pathname } = useLocation();
+ const previousPathname = usePrevious(pathname);
+ const { sidebar, setSidebar } = useAppContext();
+
+ // close any open panels on route change
+ useEffect(() => {
+ if (pathname !== previousPathname) {
+ setSidebar(null);
+ }
+ }, [pathname, previousPathname, setSidebar]);
+
useOnEscapePressed(() => {
setSidebar(null);
});
+
return (
<aside
aria-hidden={!sidebar}
+ aria-label={sidebar ? sidebarLabels[sidebar] : undefined}
className={classNames("l-aside is-maas-site-manager u-padding-top--medium", { "is-collapsed": !sidebar })}
id="aside-panel"
role="dialog"
- {...props}
>
<Row>
- <Col size={12}>{children}</Col>
+ <Col size={12}>
+ {/* use context based on the route? */}
+ {!!sidebar && sidebar === "createToken" ? (
+ <TokensCreate />
+ ) : !!sidebar && sidebar === "removeRegions" ? (
+ <RemoveRegions />
+ ) : null}
+ </Col>
</Row>
</aside>
);
@@ -45,48 +62,29 @@ const getPageTitle = (pathname: RoutePath) => {
};
const MainLayout: React.FC = () => {
- const { sidebar, setSidebar } = useAppContext();
const { pathname } = useLocation();
- const previousPathname = usePrevious(pathname);
const { status } = useAuthContext();
const isLoggedIn = status === "authenticated";
-
- // close any open panels on route change
- useEffect(() => {
- if (pathname !== previousPathname) {
- setSidebar(null);
- }
- }, [pathname, previousPathname, setSidebar]);
-
const isSideNavVisible = matchPath("/settings/*", pathname);
return (
- <>
- <div className="l-application">
- <Navigation isLoggedIn={isLoggedIn} />
- <main className="l-main is-maas-site-manager">
- <h1 className="u-visually-hidden">{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 className="l-application">
+ <Navigation isLoggedIn={isLoggedIn} />
+ <main className="l-main is-maas-site-manager">
+ <h1 className="u-visually-hidden">{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>
- </main>
- <Aside aria-label={sidebar ? sidebarLabels[sidebar] : undefined} setSidebar={setSidebar} sidebar={sidebar}>
- {!!sidebar && sidebar === "createToken" ? (
- <TokensCreate />
- ) : !!sidebar && sidebar === "removeRegions" ? (
- <RemoveRegions />
- ) : null}
- </Aside>
- </div>
- </>
+ </div>
+ </main>
+ <Aside />
+ </div>
);
};
diff --git a/frontend/src/components/RemoveRegions/RemoveRegions.test.tsx b/frontend/src/components/RemoveRegions/RemoveRegions.test.tsx
index 80b268d..54207ca 100644
--- a/frontend/src/components/RemoveRegions/RemoveRegions.test.tsx
+++ b/frontend/src/components/RemoveRegions/RemoveRegions.test.tsx
@@ -2,14 +2,18 @@ import RemoveRegions from "./index";
import { render, screen, userEvent } from "@/test-utils";
-vi.mock("@/context", () => ({
- useAppContext: () => ({
- rowSelection: {
- "1": true,
- "2": true,
- },
- }),
-}));
+vi.mock("@/context", async () => {
+ const actual = await vi.importActual("@/context");
+ return {
+ ...actual!,
+ useRowSelectionContext: () => ({
+ rowSelection: {
+ "1": true,
+ "2": true,
+ },
+ }),
+ };
+});
it("submit button should not be disabled when something has been typed", async () => {
render(<RemoveRegions />);
diff --git a/frontend/src/components/RemoveRegions/RemoveRegions.tsx b/frontend/src/components/RemoveRegions/RemoveRegions.tsx
index 0b1c195..303f7a2 100644
--- a/frontend/src/components/RemoveRegions/RemoveRegions.tsx
+++ b/frontend/src/components/RemoveRegions/RemoveRegions.tsx
@@ -6,7 +6,7 @@ import { Field, Form, Formik } from "formik";
import pluralize from "pluralize";
import * as Yup from "yup";
-import { useAppContext } from "@/context";
+import { useAppContext, useRowSelectionContext } from "@/context";
import { useSiteQueryData } from "@/hooks/react-query";
const initialValues = {
@@ -34,7 +34,7 @@ const createHandleValidate =
};
const RemoveRegions = () => {
- const { rowSelection } = useAppContext();
+ const { rowSelection } = useRowSelectionContext("sites");
const { setSidebar } = useAppContext();
const regionsCount = rowSelection && Object.keys(rowSelection).length;
const id = useId();
diff --git a/frontend/src/components/RequestsTable/RequestsTable.tsx b/frontend/src/components/RequestsTable/RequestsTable.tsx
index 20887c4..e81f6b1 100644
--- a/frontend/src/components/RequestsTable/RequestsTable.tsx
+++ b/frontend/src/components/RequestsTable/RequestsTable.tsx
@@ -10,7 +10,7 @@ import ExternalLink from "@/components/ExternalLink";
import SelectAllCheckbox from "@/components/SelectAllCheckbox";
import TableCaption from "@/components/TableCaption";
import { isDev } from "@/constants";
-import { useAppContext } from "@/context";
+import { useRowSelectionContext } from "@/context/RowSelectionContext";
import type { UseEnrollmentRequestsQueryResult } from "@/hooks/react-query";
export type EnrollmentRequestsColumnDef = ColumnDef<EnrollmentRequest, EnrollmentRequest[keyof EnrollmentRequest]>;
@@ -23,7 +23,7 @@ const RequestsTable = ({
isFetchedAfterMount,
isLoading,
}: Pick<UseEnrollmentRequestsQueryResult, "data" | "isLoading" | "isFetchedAfterMount">) => {
- const { rowSelection, setRowSelection } = useAppContext();
+ const { rowSelection, setRowSelection } = useRowSelectionContext("requests");
// clear selection on unmount
useEffect(() => {
@@ -34,7 +34,7 @@ const RequestsTable = ({
() => [
columnHelper.accessor("name", {
id: "select",
- header: ({ table }) => <SelectAllCheckbox table={table} />,
+ header: ({ table }) => <SelectAllCheckbox table={table} tableId="requests" />,
cell: ({ row, getValue }) => {
return (
<label className="p-checkbox--inline">
diff --git a/frontend/src/components/SelectAllCheckbox/SelectAllCheckbox.test.tsx b/frontend/src/components/SelectAllCheckbox/SelectAllCheckbox.test.tsx
index 4c66f14..86b50b4 100644
--- a/frontend/src/components/SelectAllCheckbox/SelectAllCheckbox.test.tsx
+++ b/frontend/src/components/SelectAllCheckbox/SelectAllCheckbox.test.tsx
@@ -10,7 +10,7 @@ type ColDef = ColumnDef<unknown, unknown>;
const columns: ColDef[] = [
{
id: "select",
- header: ({ table }) => <SelectAllCheckbox table={table} />,
+ header: ({ table }) => <SelectAllCheckbox table={table} tableId="sites" />,
cell: ({ row }) => {
return (
<input
diff --git a/frontend/src/components/SelectAllCheckbox/SelectAllCheckbox.tsx b/frontend/src/components/SelectAllCheckbox/SelectAllCheckbox.tsx
index 27fc9b2..aca48f1 100644
--- a/frontend/src/components/SelectAllCheckbox/SelectAllCheckbox.tsx
+++ b/frontend/src/components/SelectAllCheckbox/SelectAllCheckbox.tsx
@@ -1,6 +1,7 @@
import type { RowData, Table } from "@tanstack/react-table";
-import { useAppContext } from "@/context";
+import type { TableId } from "@/context/RowSelectionContext";
+import { useRowSelectionContext } from "@/context/RowSelectionContext";
type Props<T> = {
table: Table<T>;
@@ -16,12 +17,12 @@ const selectAllPageRows = <T extends RowData>(table: Table<T>) =>
};
});
-function SelectAllCheckbox<T>({ table }: Props<T>) {
+function SelectAllCheckbox<T>({ table, tableId }: Props<T> & { tableId: TableId }) {
// 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 { rowSelection } = useRowSelectionContext(tableId);
const isSomeRowsSelected = !table.getIsAllPageRowsSelected() && Object.keys(rowSelection).length > 0;
return (
<label className="p-checkbox--inline">
diff --git a/frontend/src/components/SitesList/SitesTable/SitesTable.tsx b/frontend/src/components/SitesList/SitesTable/SitesTable.tsx
index 32439cd..b236346 100644
--- a/frontend/src/components/SitesList/SitesTable/SitesTable.tsx
+++ b/frontend/src/components/SitesList/SitesTable/SitesTable.tsx
@@ -18,7 +18,7 @@ import type { PaginationBarProps } from "@/components/base/PaginationBar/Paginat
import PaginationBar from "@/components/base/PaginationBar/PaginationBar";
import TooltipButton from "@/components/base/TooltipButton/TooltipButton";
import { isDev } from "@/constants";
-import { useAppContext } from "@/context";
+import { useRowSelectionContext } from "@/context/RowSelectionContext";
import type { UseSitesQueryResult } from "@/hooks/react-query";
import { getAllMachines, getCountryName, getTimezoneUTCString, getTimeInTimezone } from "@/utils";
@@ -44,8 +44,7 @@ const SitesTable = ({
const [columnVisibility, setColumnVisibility] = useLocalStorageState("sitesTableColumnVisibility", {
defaultValue: {},
});
-
- const { rowSelection, setRowSelection } = useAppContext();
+ const { rowSelection, setRowSelection } = useRowSelectionContext("sites");
// clear selection on unmount
useEffect(() => {
@@ -57,7 +56,7 @@ const SitesTable = ({
{
id: "select",
accessorKey: "name",
- header: ({ table }) => <SelectAllCheckbox table={table} />,
+ header: ({ table }) => <SelectAllCheckbox table={table} tableId="sites" />,
cell: ({ row, getValue }: { row: Row<Site>; getValue: Getter<Site["name"]> }) => {
return (
<label className="p-checkbox--inline">
diff --git a/frontend/src/components/SitesList/SitesTable/SitesTableControls/SitesTableControls.tsx b/frontend/src/components/SitesList/SitesTable/SitesTableControls/SitesTableControls.tsx
index 1ff3107..2ca0e3b 100644
--- a/frontend/src/components/SitesList/SitesTable/SitesTableControls/SitesTableControls.tsx
+++ b/frontend/src/components/SitesList/SitesTable/SitesTableControls/SitesTableControls.tsx
@@ -4,7 +4,8 @@ import ColumnsVisibilityControl from "./ColumnsVisibilityControl";
import SitesCount from "./SitesCount";
import type { SitesColumn } from "@/components/SitesList/SitesTable/SitesTable";
-import { useAppContext } from "@/context";
+import { useAppContext } from "@/context/AppContext";
+import { useRowSelectionContext } from "@/context/RowSelectionContext";
import type { UseSitesQueryResult } from "@/hooks/react-query";
const SitesTableControls = ({
@@ -19,7 +20,8 @@ const SitesTableControls = ({
const handleSearchInput = (inputValue: string) => {
setSearchText(inputValue);
};
- const { rowSelection, setSidebar } = useAppContext();
+ const { setSidebar } = useAppContext();
+ const { rowSelection } = useRowSelectionContext("sites");
const isRemoveDisabled = Object.keys(rowSelection).length <= 0;
return (
diff --git a/frontend/src/components/TokensList/TokensList.tsx b/frontend/src/components/TokensList/TokensList.tsx
index b805a11..04d7567 100644
--- a/frontend/src/components/TokensList/TokensList.tsx
+++ b/frontend/src/components/TokensList/TokensList.tsx
@@ -9,13 +9,15 @@ import docsUrls from "@/base/docsUrls";
import ExternalLink from "@/components/ExternalLink";
import PaginationBar from "@/components/base/PaginationBar";
import { useAppContext } from "@/context";
+import { useRowSelectionContext } from "@/context/RowSelectionContext";
import { useDeleteTokensMutation, useTokensQuery } from "@/hooks/react-query";
import usePagination from "@/hooks/usePagination";
const DEFAULT_PAGE_SIZE = 50;
const TokensList = () => {
- const { setSidebar, rowSelection, setRowSelection } = useAppContext();
+ const { setSidebar } = useAppContext();
+ const { rowSelection, setRowSelection } = useRowSelectionContext("tokens");
const [totalDataCount, setTotalDataCount] = useState(0);
const [deleteNotification, setDeleteNotification] = useState("");
const { page, debouncedPage, size, handleNextClick, handlePreviousClick, handlePageSizeChange, setPage } =
diff --git a/frontend/src/components/TokensList/components/TokensTable/TokensTable.tsx b/frontend/src/components/TokensList/components/TokensTable/TokensTable.tsx
index 3c4d33a..50896a6 100644
--- a/frontend/src/components/TokensList/components/TokensTable/TokensTable.tsx
+++ b/frontend/src/components/TokensList/components/TokensTable/TokensTable.tsx
@@ -8,7 +8,7 @@ import type { Token } from "@/api/types";
import SelectAllCheckbox from "@/components/SelectAllCheckbox";
import CopyButton from "@/components/base/CopyButton";
import TooltipButton from "@/components/base/TooltipButton";
-import { useAppContext } from "@/context";
+import { useRowSelectionContext } from "@/context/RowSelectionContext";
import type { useTokensQueryResult } from "@/hooks/react-query";
import { copyToClipboard, formatDistanceToNow, formatUTCDateString } from "@/utils";
@@ -26,7 +26,8 @@ const TokensTable = ({
isLoading,
}: Pick<useTokensQueryResult, "data" | "isLoading" | "isFetchedAfterMount">) => {
const [copiedText, setCopiedText] = useState("");
- const { rowSelection, setRowSelection } = useAppContext();
+
+ const { rowSelection, setRowSelection } = useRowSelectionContext("tokens");
const isTokenCopied = useCallback((token: string) => token === copiedText, [copiedText]);
// clear table selection on unmount
@@ -50,7 +51,7 @@ const TokensTable = ({
{
id: "select",
accessorKey: "value",
- header: ({ table }) => <SelectAllCheckbox table={table} />,
+ header: ({ table }) => <SelectAllCheckbox table={table} tableId="tokens" />,
cell: ({ row, getValue }: { row: Row<Token>; getValue: Getter<Token["value"]> }) => (
<label className="p-checkbox--inline">
<input
diff --git a/frontend/src/context/AppContext.tsx b/frontend/src/context/AppContext.tsx
index c7c41ec..6a63165 100644
--- a/frontend/src/context/AppContext.tsx
+++ b/frontend/src/context/AppContext.tsx
@@ -1,26 +1,17 @@
import { createContext, useContext, useState } from "react";
-import type { OnChangeFn, RowSelectionState } from "@tanstack/react-table";
-
export const AppContext = createContext<{
- rowSelection: RowSelectionState;
- setRowSelection: OnChangeFn<RowSelectionState>;
sidebar: "removeRegions" | "createToken" | null;
setSidebar: (sidebar: "removeRegions" | "createToken" | null) => void;
}>({
- rowSelection: {},
- setRowSelection: () => ({}),
sidebar: null,
setSidebar: () => null,
});
export const AppContextProvider = ({ children }: { children: React.ReactNode }) => {
- const [rowSelection, setRowSelection] = useState<RowSelectionState>({});
const [sidebar, setSidebar] = useState<"removeRegions" | "createToken" | null>(null);
- return (
- <AppContext.Provider value={{ rowSelection, setRowSelection, sidebar, setSidebar }}>{children}</AppContext.Provider>
- );
+ return <AppContext.Provider value={{ sidebar, setSidebar }}>{children}</AppContext.Provider>;
};
export const useAppContext = () => useContext(AppContext);
diff --git a/frontend/src/context/RowSelectionContext.tsx b/frontend/src/context/RowSelectionContext.tsx
new file mode 100644
index 0000000..5ce565d
--- /dev/null
+++ b/frontend/src/context/RowSelectionContext.tsx
@@ -0,0 +1,51 @@
+import type { PropsWithChildren } from "react";
+import React, { createContext, useContext, useState } from "react";
+
+import type { OnChangeFn, RowSelectionState } from "@tanstack/react-table";
+
+export type TableId = "sites" | "requests" | "tokens";
+type RowSelectionContextValue = { rowSelection: RowSelectionState; setRowSelection: OnChangeFn<RowSelectionState> };
+const ContextsCache = new Map<TableId, React.Context<RowSelectionContextValue>>();
+
+const getRowSelectionContext = (id: TableId): React.Context<RowSelectionContextValue> => {
+ if (!ContextsCache.has(id)) {
+ ContextsCache.set(
+ id,
+ createContext<RowSelectionContextValue>({
+ rowSelection: {},
+ setRowSelection: () => ({}),
+ }),
+ );
+ }
+ return ContextsCache.get(id)!;
+};
+
+export const useRowSelectionContext = (id: TableId): RowSelectionContextValue => {
+ const RowSelectionContext = getRowSelectionContext(id);
+ return useContext(RowSelectionContext);
+};
+
+export const getRowSelectionContextProvider =
+ (id: TableId) =>
+ ({ children }: PropsWithChildren) => {
+ const RowSelectionContext = getRowSelectionContext(id);
+ const [rowSelection, setRowSelection] = useState({});
+
+ return (
+ <RowSelectionContext.Provider value={{ rowSelection, setRowSelection }}>{children}</RowSelectionContext.Provider>
+ );
+ };
+
+const SitesRowSelectionContextProvider = getRowSelectionContextProvider("sites");
+const RequestsRowSelectionContextProvider = getRowSelectionContextProvider("requests");
+const TokensRowSelectionContextProvider = getRowSelectionContextProvider("tokens");
+
+export const RowSelectionContextProviders = ({ children }: PropsWithChildren) => {
+ return (
+ <SitesRowSelectionContextProvider>
+ <RequestsRowSelectionContextProvider>
+ <TokensRowSelectionContextProvider>{children}</TokensRowSelectionContextProvider>
+ </RequestsRowSelectionContextProvider>
+ </SitesRowSelectionContextProvider>
+ );
+};
diff --git a/frontend/src/context/index.ts b/frontend/src/context/index.ts
index d778afb..9073ecf 100644
--- a/frontend/src/context/index.ts
+++ b/frontend/src/context/index.ts
@@ -1,2 +1,3 @@
export { AuthContext, AuthContextProvider, useAuthContext } from "./AuthContext";
export { AppContext, AppContextProvider, useAppContext } from "./AppContext";
+export { RowSelectionContextProviders, useRowSelectionContext } from "./RowSelectionContext";
diff --git a/frontend/src/test-utils.tsx b/frontend/src/test-utils.tsx
index b7f52c0..de43cdc 100644
--- a/frontend/src/test-utils.tsx
+++ b/frontend/src/test-utils.tsx
@@ -6,7 +6,7 @@ import type { RenderOptions, RenderResult } from "@testing-library/react";
import { screen, render } from "@testing-library/react";
import apiClient from "@/api";
-import { AppContextProvider, AuthContextProvider } from "@/context";
+import { AppContextProvider, AuthContextProvider, RowSelectionContextProviders } from "@/context";
import type { MemoryRouterProps } from "@/router";
import { MemoryRouter } from "@/router";
@@ -30,7 +30,9 @@ const makeProvidersWithMemoryRouter =
<Providers>
<MemoryRouter {...memoryRouterProps}>
<AppContextProvider>
- <AuthContextProvider apiClient={apiClient}>{children}</AuthContextProvider>
+ <AuthContextProvider apiClient={apiClient}>
+ <RowSelectionContextProviders>{children}</RowSelectionContextProviders>
+ </AuthContextProvider>
</AppContextProvider>
</MemoryRouter>
</Providers>
--
Mailing list: https://launchpad.net/~sts-sponsors
Post to : sts-sponsors@lists.launchpad.net
Unsubscribe : https://launchpad.net/~sts-sponsors
More help : https://help.launchpad.net/ListHelp
