Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

✨ Add useLocalTableControlsWithUrlParams hook and update archetypes table to use it #1392

Merged
merged 4 commits into from
Sep 26, 2023
Merged

✨ Add useLocalTableControlsWithUrlParams hook and update archetypes table to use it #1392

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9c56d0c
:sparkles: Add useLocalTableControlsWithUrlParams hook and update arc…
mturley Sep 22, 2023
9386187
Merge branch 'main' into table-local-url-params
ibolton336 Sep 25, 2023
d855ef7
Merge branch 'main' into table-local-url-params
ibolton336 Sep 26, 2023
bacd72d
Merge branch 'main' into table-local-url-params
mturley Sep 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions client/src/app/Constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,4 +242,5 @@ export enum TableURLParamKeyPrefix {
issuesAffectedFiles = "if",
issuesRemainingIncidents = "ii",
dependencyApplications = "da",
archetypes = "ar",
}
92 changes: 87 additions & 5 deletions client/src/app/hooks/table-controls/useLocalTableControlState.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,21 +1,31 @@
import { useSelectionState } from "@migtools/lib-ui";
import { getLocalFilterDerivedState, useFilterState } from "./filtering";
import { useSortState, getLocalSortDerivedState } from "./sorting";
import {
getLocalFilterDerivedState,
useFilterState,
useFilterUrlParams,
} from "./filtering";
import {
useSortState,
getLocalSortDerivedState,
useSortUrlParams,
} from "./sorting";
import {
getLocalPaginationDerivedState,
usePaginationState,
usePaginationUrlParams,
} from "./pagination";
import { useExpansionState } from "./expansion";
import { useActiveRowState } from "./active-row";
import { useExpansionState, useExpansionUrlParams } from "./expansion";
import { useActiveRowState, useActiveRowUrlParams } from "./active-row";
import {
IExtraArgsForURLParamHooks,
IUseLocalTableControlStateArgs,
IUseTableControlPropsArgs,
} from "./types";

export const useLocalTableControlState = <
TItem,
TColumnKey extends string,
TSortableColumnKey extends TColumnKey
TSortableColumnKey extends TColumnKey,
>(
args: IUseLocalTableControlStateArgs<TItem, TColumnKey, TSortableColumnKey>
): IUseTableControlPropsArgs<TItem, TColumnKey, TSortableColumnKey> => {
Expand Down Expand Up @@ -77,3 +87,75 @@ export const useLocalTableControlState = <
currentPageItems: hasPagination ? currentPageItems : sortedItems,
};
};

// TODO refactor useUrlParams so it can be used conditionally (e.g. useStateOrUrlParams) so we don't have to duplicate all this.
// this would mean all use[Feature]UrlParams hooks could be consolidated into use[Feature]State with a boolean option for whether to use URL params.

export const useLocalTableControlUrlParams = <
TItem,
TColumnKey extends string,
TSortableColumnKey extends TColumnKey,
TURLParamKeyPrefix extends string = string,
>(
args: IUseLocalTableControlStateArgs<TItem, TColumnKey, TSortableColumnKey> &
IExtraArgsForURLParamHooks<TURLParamKeyPrefix>
): IUseTableControlPropsArgs<TItem, TColumnKey, TSortableColumnKey> => {
const {
items,
filterCategories = [],
sortableColumns = [],
getSortValues,
initialSort = null,
hasPagination = true,
initialItemsPerPage = 10,
idProperty,
initialSelected,
isItemSelectable,
} = args;

const filterState = useFilterUrlParams(args);
const { filteredItems } = getLocalFilterDerivedState({
items,
filterCategories,
filterState,
});

const selectionState = useSelectionState({
items: filteredItems,
isEqual: (a, b) => a[idProperty] === b[idProperty],
initialSelected,
isItemSelectable,
});

const sortState = useSortUrlParams({ ...args, sortableColumns, initialSort });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use a bit of trickery to get this as a single call .....

  const isUrlParamBased = !!urlParamKeyPrefix;

  const sortState = (isUrlParamBased ? useSortUrlParams : useSortState)({
    ...args, 
    sortableColumns,
    initialSort,
  });

Copy link
Collaborator Author

@mturley mturley Sep 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered this, but the eslint rules-of-hooks rule doesn't like it I think (if you re-render with a different value for isUrlParamBased it would switch up which hooks are called and screw up the render, not that we'd do that). If we added an eslint-ignore there I think it would be too easy to accidentally break it in a future change.

What I'd like to do eventually (soon perhaps) is refactor useUrlParams (which is used inside useSortUrlParams and each of the other use[Feature]UrlParams hooks) into a useStateOrUrlParams so that it itself has a isUrlParamBased condition, and has its own useState internally that it either uses or ignores in favor of URL params depending on config. Then we could get rid of all the use[Feature]UrlParams hooks and instead have the URL params be an option in use[Feature]State, all the way up to useTableControlState.

const { sortedItems } = getLocalSortDerivedState({
sortState,
items: filteredItems,
getSortValues,
});

const paginationState = usePaginationUrlParams({
...args,
initialItemsPerPage,
});
const { currentPageItems } = getLocalPaginationDerivedState({
paginationState,
items: sortedItems,
});

const expansionState = useExpansionUrlParams<TColumnKey>(args);

const activeRowState = useActiveRowUrlParams(args);

return {
...args,
filterState,
expansionState,
selectionState,
sortState,
paginationState,
activeRowState,
totalItemCount: items.length,
currentPageItems: hasPagination ? currentPageItems : sortedItems,
};
};
22 changes: 19 additions & 3 deletions client/src/app/hooks/table-controls/useLocalTableControls.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,27 @@
import { useLocalTableControlState } from "./useLocalTableControlState";
import {
useLocalTableControlState,
useLocalTableControlUrlParams,
} from "./useLocalTableControlState";
import { useTableControlProps } from "./useTableControlProps";
import { IUseLocalTableControlStateArgs } from "./types";
import {
IExtraArgsForURLParamHooks,
IUseLocalTableControlStateArgs,
} from "./types";

export const useLocalTableControls = <
TItem,
TColumnKey extends string,
TSortableColumnKey extends TColumnKey
TSortableColumnKey extends TColumnKey,
>(
args: IUseLocalTableControlStateArgs<TItem, TColumnKey, TSortableColumnKey>
) => useTableControlProps(useLocalTableControlState(args));

export const useLocalTableControlsWithUrlParams = <
TItem,
TColumnKey extends string,
TSortableColumnKey extends TColumnKey,
TURLParamKeyPrefix extends string = string,
>(
args: IUseLocalTableControlStateArgs<TItem, TColumnKey, TSortableColumnKey> &
IExtraArgsForURLParamHooks<TURLParamKeyPrefix>
) => useTableControlProps(useLocalTableControlUrlParams(args));
2 changes: 1 addition & 1 deletion client/src/app/hooks/table-controls/useTableControlUrlParams.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export const useTableControlUrlParams = <
TColumnKey extends string,
TSortableColumnKey extends TColumnKey,
TFilterCategoryKey extends string = string,
TURLParamKeyPrefix extends string = string
TURLParamKeyPrefix extends string = string,
>(
args: ITableControlCommonArgs<
TItem,
Expand Down
6 changes: 4 additions & 2 deletions client/src/app/pages/archetypes/archetypes-page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ import {
TableHeaderContentWithControls,
TableRowContentWithControls,
} from "@app/components/TableControls";
import { useLocalTableControls } from "@app/hooks/table-controls";
import { useLocalTableControlsWithUrlParams } from "@app/hooks/table-controls";
import {
useDeleteArchetypeMutation,
useFetchArchetypes,
Expand All @@ -57,6 +57,7 @@ import { formatPath, getAxiosErrorMessage } from "@app/utils/utils";
import { AxiosError } from "axios";
import { Paths } from "@app/Paths";
import { SimplePagination } from "@app/components/SimplePagination";
import { TableURLParamKeyPrefix } from "@app/Constants";

const Archetypes: React.FC = () => {
const { t } = useTranslation();
Expand Down Expand Up @@ -96,7 +97,8 @@ const Archetypes: React.FC = () => {
onError
);

const tableControls = useLocalTableControls({
const tableControls = useLocalTableControlsWithUrlParams({
urlParamKeyPrefix: TableURLParamKeyPrefix.archetypes,
idProperty: "id",
items: archetypes,
isLoading: isFetching,
Expand Down