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 all 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 @@
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> => {

Check warning on line 102 in client/src/app/hooks/table-controls/useLocalTableControlState.ts

View check run for this annotation

Codecov / codecov/patch

client/src/app/hooks/table-controls/useLocalTableControlState.ts#L102 

Added line #L102 was not covered by tests
const {
items,
filterCategories = [],
sortableColumns = [],
getSortValues,
initialSort = null,
hasPagination = true,
initialItemsPerPage = 10,
idProperty,
initialSelected,
isItemSelectable,
} = args;

Check warning on line 114 in client/src/app/hooks/table-controls/useLocalTableControlState.ts

View check run for this annotation

Codecov / codecov/patch

client/src/app/hooks/table-controls/useLocalTableControlState.ts#L114 

Added line #L114 was not covered by tests

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

Check warning on line 117 in client/src/app/hooks/table-controls/useLocalTableControlState.ts

View check run for this annotation

Codecov / codecov/patch

client/src/app/hooks/table-controls/useLocalTableControlState.ts#L116-L117 

Added lines #L116 - L117 were not covered by tests
items,
filterCategories,
filterState,
});

const selectionState = useSelectionState({

Check warning on line 123 in client/src/app/hooks/table-controls/useLocalTableControlState.ts

View check run for this annotation

Codecov / codecov/patch

client/src/app/hooks/table-controls/useLocalTableControlState.ts#L123 

Added line #L123 was not covered by tests
items: filteredItems,
isEqual: (a, b) => a[idProperty] === b[idProperty],

Check warning on line 125 in client/src/app/hooks/table-controls/useLocalTableControlState.ts

View check run for this annotation

Codecov / codecov/patch

client/src/app/hooks/table-controls/useLocalTableControlState.ts#L125 

Added line #L125 was not covered by tests
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({

Check warning on line 131 in client/src/app/hooks/table-controls/useLocalTableControlState.ts

View check run for this annotation

Codecov / codecov/patch

client/src/app/hooks/table-controls/useLocalTableControlState.ts#L130-L131 

Added lines #L130 - L131 were not covered by tests
sortState,
items: filteredItems,
getSortValues,
});

const paginationState = usePaginationUrlParams({

Check warning on line 137 in client/src/app/hooks/table-controls/useLocalTableControlState.ts

View check run for this annotation

Codecov / codecov/patch

client/src/app/hooks/table-controls/useLocalTableControlState.ts#L137 

Added line #L137 was not covered by tests
...args,
initialItemsPerPage,
});
const { currentPageItems } = getLocalPaginationDerivedState({

Check warning on line 141 in client/src/app/hooks/table-controls/useLocalTableControlState.ts

View check run for this annotation

Codecov / codecov/patch

client/src/app/hooks/table-controls/useLocalTableControlState.ts#L141 

Added line #L141 was not covered by tests
paginationState,
items: sortedItems,
});

const expansionState = useExpansionUrlParams<TColumnKey>(args);

Check warning on line 146 in client/src/app/hooks/table-controls/useLocalTableControlState.ts

View check run for this annotation

Codecov / codecov/patch

client/src/app/hooks/table-controls/useLocalTableControlState.ts#L146 

Added line #L146 was not covered by tests

const activeRowState = useActiveRowUrlParams(args);

Check warning on line 148 in client/src/app/hooks/table-controls/useLocalTableControlState.ts

View check run for this annotation

Codecov / codecov/patch

client/src/app/hooks/table-controls/useLocalTableControlState.ts#L148 

Added line #L148 was not covered by tests

return {

Check warning on line 150 in client/src/app/hooks/table-controls/useLocalTableControlState.ts

View check run for this annotation

Codecov / codecov/patch

client/src/app/hooks/table-controls/useLocalTableControlState.ts#L150 

Added line #L150 was not covered by tests
...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));

Check warning on line 27 in client/src/app/hooks/table-controls/useLocalTableControls.ts

View check run for this annotation

Codecov / codecov/patch

client/src/app/hooks/table-controls/useLocalTableControls.ts#L27 

Added line #L27 was not covered by tests
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
Loading