fix: table scrolling when clicking show more

- fix: don't reset Table keys unless sort has changed - refactor: remove double Table implementation (reuse one styled table) refs: TILA-3576
City-of-Helsinki · Oct 8, 2024 · 01955e3 · 01955e3
1 parent 0ee87c5
commit 01955e3
Show file tree

Hide file tree

Showing 3 changed files with 79 additions and 141 deletions.
diff --git a/apps/admin-ui/src/component/Table.tsx b/apps/admin-ui/src/component/Table.tsx
@@ -45,18 +45,22 @@ const StyledTable = styled(Table)<TableWrapperProps>`
 type Props = Omit<TableProps, "onSort"> & {
   setSort?: (col: string) => void;
   isLoading?: boolean;
+  enableFrontendSorting?: boolean;
 };
 
 // @param isLoading - if true, table is rendered with a loading overlay
 // TODO overlay and spinner for loading would be preferable over colour switching
 export function CustomTable({
   isLoading,
   setSort,
+  enableFrontendSorting,
   ...props
 }: Props): JSX.Element {
+  const [keyOverride, setKeyOverride] = React.useState<number>(0);
   const onSort = (order: "asc" | "desc", colKey: string) => {
     const field = order === "asc" ? colKey : `-${colKey}`;
     if (setSort) {
+      setKeyOverride((prev) => prev + 1);
       setSort(field);
     }
   };
@@ -65,11 +69,11 @@ export function CustomTable({
     <StyledTable
       {...props}
       variant="light"
-      onSort={onSort}
-      // NOTE have to unmount on data changes because there is a bug in the Table component
-      // removing this and using sort leaves ghost elements in the table.
-      // also search indicators are not correct when search changes (initial direction / key)
-      key={JSON.stringify(props.rows, replacer)}
+      onSort={!enableFrontendSorting ? onSort : undefined}
+      // NOTE when using backend sorting we need to unmount the table
+      // otherwise the table header is not updated
+      // unmounting on other data changes is not necessary and causes other bugs like automatic scrolling.
+      key={`custom-table-${keyOverride}`}
       $tableBackground={
         isLoading ? "var(--color-black-10)" : "var(--color-white)"
       }
@@ -79,18 +83,3 @@ export function CustomTable({
     />
   );
 }
-
-// React elements can't be serialized into json
-function replacer(_key: string, value: unknown) {
-  if (typeof value === "object" && value != null) {
-    if (
-      "$$typeof" in value &&
-      value.$$typeof != null &&
-      // eslint-disable-next-line @typescript-eslint/no-base-to-string
-      value.$$typeof.toString() === "Symbol(react.element)"
-    ) {
-      return undefined;
-    }
-  }
-  return value;
-}
diff --git a/apps/admin-ui/src/spa/application-rounds/CustomTable.tsx b/apps/admin-ui/src/spa/application-rounds/CustomTable.tsx
diff --git a/apps/admin-ui/src/spa/application-rounds/index.tsx b/apps/admin-ui/src/spa/application-rounds/index.tsx
@@ -19,8 +19,9 @@ import { Container } from "@/styles/layout";
 import { truncate } from "@/helpers";
 import Loader from "@/component/Loader";
 import { ApplicationRoundCard } from "./ApplicationRoundCard";
-import { StyledHDSTable } from "./CustomTable";
 import { TableLink } from "@/styles/util";
+import { CustomTable } from "@/component/Table";
+import Error404 from "@/common/Error404";
 
 const AccordionContainer = styled.div`
   display: flex;
@@ -83,7 +84,7 @@ function AllApplicationRounds(): JSX.Element | null {
   const { t } = useTranslation();
 
   // TODO pagination
-  const { data, loading } = useApplicationRoundsQuery({
+  const { data, loading, error } = useApplicationRoundsQuery({
     onError: (err: ApolloError) => {
       errorToast({ text: err.message });
     },
@@ -93,12 +94,13 @@ function AllApplicationRounds(): JSX.Element | null {
     data?.applicationRounds?.edges?.map((ar) => ar?.node)
   );
 
-  if (loading) {
+  if (loading && allApplicationRounds == null) {
     return <Loader />;
   }
 
-  if (!allApplicationRounds) {
-    return null;
+  if (allApplicationRounds == null || error != null) {
+    // TODO should be a different error page
+    return <Error404 />;
   }
 
   const currentApplicationRounds = allApplicationRounds.filter(
@@ -117,6 +119,51 @@ function AllApplicationRounds(): JSX.Element | null {
     (ar) => ar.status === ApplicationRoundStatusChoice.Handled
   );
 
+  const cols = [
+    {
+      isSortable: true,
+      headerName: t("ApplicationRound.headings.name"),
+      transform: (applicationRound: ApplicationRoundNode) => (
+        <TableLink to={getApplicationRoundUrl(applicationRound.pk)}>
+          <span title={applicationRound.nameFi ?? ""}>
+            {truncate(applicationRound.nameFi ?? "", 50)}
+          </span>
+        </TableLink>
+      ),
+      key: "nameFi",
+    },
+    {
+      isSortable: true,
+      headerName: t("ApplicationRound.headings.reservationUnitCount"),
+      transform: (applicationRound: ApplicationRoundNode) =>
+        String(applicationRound.applicationsCount),
+      key: "applicationsCount",
+    },
+    {
+      isSortable: true,
+      headerName: t("ApplicationRound.headings.applicationCount"),
+      transform: (applicationRound: ApplicationRoundNode) =>
+        String(applicationRound.reservationUnitCount),
+      key: "reservationUnitCount",
+    },
+    {
+      isSortable: true,
+      headerName: t("ApplicationRound.headings.sent"),
+      transform: (applicationRound: ApplicationRoundNode) =>
+        formatDate(applicationRound.statusTimestamp || null) || "-",
+      key: "statusTimestampSort",
+    },
+  ];
+
+  const rows = orderBy(
+    sentApplicationRounds,
+    ["statusTimestamp"],
+    ["desc"]
+  ).map((a) => ({
+    ...a,
+    statusTimestampSort: new Date(a.statusTimestamp || "").getTime(),
+  }));
+
   return (
     <Container>
       <div>
@@ -127,13 +174,11 @@ function AllApplicationRounds(): JSX.Element | null {
         initiallyOpen
         hideIfEmpty
         name={t("ApplicationRound.groupLabel.handling")}
-        rounds={
-          orderBy(
-            currentApplicationRounds,
-            ["status", "applicationPeriodEnd"],
-            ["asc", "asc"]
-          ) || []
-        }
+        rounds={orderBy(
+          currentApplicationRounds,
+          ["status", "applicationPeriodEnd"],
+          ["asc", "asc"]
+        )}
       />
       <RoundsAccordion
         name={t("ApplicationRound.groupLabel.notSent")}
@@ -143,22 +188,21 @@ function AllApplicationRounds(): JSX.Element | null {
       />
       <RoundsAccordion
         name={t("ApplicationRound.groupLabel.open")}
-        rounds={
-          orderBy(openApplicationRounds, ["applicationPeriodEnd", "asc"], []) ||
+        rounds={orderBy(
+          openApplicationRounds,
+          ["applicationPeriodEnd", "asc"],
           []
-        }
+        )}
         hideIfEmpty
         initiallyOpen
       />
       <RoundsAccordion
         name={t("ApplicationRound.groupLabel.opening")}
-        rounds={
-          orderBy(
-            upcomingApplicationRounds,
-            ["applicationPeriodBegin"],
-            ["asc"]
-          ) || []
-        }
+        rounds={orderBy(
+          upcomingApplicationRounds,
+          ["applicationPeriodBegin"],
+          ["asc"]
+        )}
         emptyContent={
           <div>
             <div>{t("ApplicationRound.noUpcoming")}</div>
@@ -169,58 +213,13 @@ function AllApplicationRounds(): JSX.Element | null {
         heading={t("ApplicationRound.groupLabel.previousRounds")}
         className="previous-rounds"
       >
-        <StyledHDSTable
-          ariaLabelSortButtonAscending="Sorted in ascending order"
-          ariaLabelSortButtonDescending="Sorted in descending order"
-          ariaLabelSortButtonUnset="Not sorted"
+        <CustomTable
+          enableFrontendSorting
           initialSortingColumnKey="applicantSort"
           initialSortingOrder="asc"
-          cols={[
-            {
-              isSortable: true,
-              headerName: t("ApplicationRound.headings.name"),
-              transform: (applicationRound: ApplicationRoundNode) => (
-                <TableLink to={getApplicationRoundUrl(applicationRound.pk)}>
-                  <span title={applicationRound.nameFi ?? ""}>
-                    {truncate(applicationRound.nameFi ?? "", 50)}
-                  </span>
-                </TableLink>
-              ),
-              key: "nameFi",
-            },
-            {
-              isSortable: true,
-              headerName: t("ApplicationRound.headings.reservationUnitCount"),
-              transform: (applicationRound: ApplicationRoundNode) =>
-                String(applicationRound.applicationsCount),
-              key: "applicationsCount",
-            },
-            {
-              isSortable: true,
-              headerName: t("ApplicationRound.headings.applicationCount"),
-              transform: (applicationRound: ApplicationRoundNode) =>
-                String(applicationRound.reservationUnitCount),
-              key: "reservationUnitCount",
-            },
-            {
-              isSortable: true,
-              headerName: t("ApplicationRound.headings.sent"),
-              transform: (applicationRound: ApplicationRoundNode) =>
-                formatDate(applicationRound.statusTimestamp || null) || "-",
-              key: "statusTimestampSort",
-            },
-          ]}
+          cols={cols}
           indexKey="pk"
-          rows={
-            orderBy(sentApplicationRounds, ["statusTimestamp"], ["desc"]).map(
-              (a) => ({
-                ...a,
-                statusTimestampSort: new Date(
-                  a.statusTimestamp || ""
-                ).getTime(),
-              })
-            ) || []
-          }
+          rows={rows}
           variant="light"
         />
       </StyledAccordion>