From ec8fde0a5f8384872cfe14329df3acadfc427378 Mon Sep 17 00:00:00 2001
From: Saumya Pandey <sp160899@gmail.com>
Date: Wed, 18 Aug 2021 03:27:23 +0530
Subject: [PATCH] fix: Improvements in table component (#2450)

---
 app/components/Table.js                    | 71 +++++++++++++++++-----
 app/scenes/Settings/People.js              |  1 +
 shared/i18n/locales/en_US/translation.json |  4 +-
 3 files changed, 58 insertions(+), 18 deletions(-)

diff --git a/app/components/Table.js b/app/components/Table.js
index c229ee45..49a4fb60 100644
--- a/app/components/Table.js
+++ b/app/components/Table.js
@@ -1,4 +1,5 @@
 // @flow
+import { isEqual } from "lodash";
 import { observer } from "mobx-react";
 import { CollapsedIcon } from "outline-icons";
 import * as React from "react";
@@ -24,6 +25,7 @@ export type Props = {|
   onChangePage: (index: number) => void,
   onChangeSort: (sort: ?string, direction: "ASC" | "DESC") => void,
   columns: any,
+  defaultSortDirection: "ASC" | "DESC",
 |};
 
 function Table({
@@ -39,6 +41,7 @@ function Table({
   topRef,
   onChangeSort,
   onChangePage,
+  defaultSortDirection,
 }: Props) {
   const { t } = useTranslation();
   const {
@@ -62,32 +65,52 @@ function Table({
       autoResetPage: false,
       pageCount: totalPages,
       initialState: {
-        sortBy: [{ id: defaultSort, desc: false }],
+        sortBy: [
+          {
+            id: defaultSort,
+            desc: defaultSortDirection === "DESC" ? true : false,
+          },
+        ],
         pageSize,
         pageIndex: page,
       },
+      stateReducer: (newState, action, prevState) => {
+        if (!isEqual(newState.sortBy, prevState.sortBy)) {
+          return { ...newState, pageIndex: 0 };
+        }
+        return newState;
+      },
     },
     useSortBy,
     usePagination
   );
 
-  React.useEffect(() => {
-    onChangePage(pageIndex);
-  }, [pageIndex]);
+  const prevSortBy = React.useRef(sortBy);
 
   React.useEffect(() => {
-    onChangePage(0);
-    onChangeSort(
-      sortBy.length ? sortBy[0].id : undefined,
-      sortBy.length && sortBy[0].desc ? "DESC" : "ASC"
-    );
-  }, [sortBy]);
+    if (!isEqual(sortBy, prevSortBy.current)) {
+      prevSortBy.current = sortBy;
+      onChangePage(0);
+      onChangeSort(
+        sortBy.length ? sortBy[0].id : undefined,
+        !sortBy.length ? defaultSortDirection : sortBy[0].desc ? "DESC" : "ASC"
+      );
+    }
+  }, [defaultSortDirection, onChangePage, onChangeSort, sortBy]);
+
+  const handleNextPage = () => {
+    nextPage();
+    onChangePage(pageIndex + 1);
+  };
+
+  const handlePreviousPage = () => {
+    previousPage();
+    onChangePage(pageIndex - 1);
+  };
 
   const isEmpty = !isLoading && data.length === 0;
   const showPlaceholder = isLoading && data.length === 0;
 
-  console.log({ canNextPage, pageIndex, totalPages, rows, data });
-
   return (
     <>
       <Anchor ref={topRef} />
@@ -142,12 +165,12 @@ function Table({
         >
           {/* Note: the page > 0 check shouldn't be needed here but is */}
           {canPreviousPage && page > 0 && (
-            <Button onClick={previousPage} neutral>
+            <Button onClick={handlePreviousPage} neutral>
               {t("Previous page")}
             </Button>
           )}
           {canNextPage && (
-            <Button onClick={nextPage} neutral>
+            <Button onClick={handleNextPage} neutral>
               {t("Next page")}
             </Button>
           )}
@@ -209,7 +232,7 @@ const SortWrapper = styled(Flex)`
 `;
 
 const Cell = styled.td`
-  padding: 8px 0;
+  padding: 6px;
   border-bottom: 1px solid ${(props) => props.theme.divider};
   font-size: 14px;
 
@@ -226,6 +249,14 @@ const Cell = styled.td`
 `;
 
 const Row = styled.tr`
+  ${Cell} {
+    &:first-child {
+      padding-left: 0;
+    }
+    &:last-child {
+      padding-right: 0;
+    }
+  }
   &:last-child {
     ${Cell} {
       border-bottom: 0;
@@ -237,7 +268,7 @@ const Head = styled.th`
   text-align: left;
   position: sticky;
   top: 54px;
-  padding: 6px 0;
+  padding: 6px;
   border-bottom: 1px solid ${(props) => props.theme.divider};
   background: ${(props) => props.theme.background};
   transition: ${(props) => props.theme.backgroundTransition};
@@ -245,6 +276,14 @@ const Head = styled.th`
   color: ${(props) => props.theme.textSecondary};
   font-weight: 500;
   z-index: 1;
+
+  :first-child {
+    padding-left: 0;
+  }
+
+  :last-child {
+    padding-right: 0;
+  }
 `;
 
 export default observer(Table);
diff --git a/app/scenes/Settings/People.js b/app/scenes/Settings/People.js
index 5b95ae8c..2b06722d 100644
--- a/app/scenes/Settings/People.js
+++ b/app/scenes/Settings/People.js
@@ -226,6 +226,7 @@ function People(props) {
         onChangePage={handleChangePage}
         page={page}
         totalPages={totalPages}
+        defaultSortDirection="ASC"
       />
       {can.inviteUser && (
         <Modal
diff --git a/shared/i18n/locales/en_US/translation.json b/shared/i18n/locales/en_US/translation.json
index c1d63440..33e6572a 100644
--- a/shared/i18n/locales/en_US/translation.json
+++ b/shared/i18n/locales/en_US/translation.json
@@ -202,14 +202,14 @@
   "Choose a collection": "Choose a collection",
   "Unpin": "Unpin",
   "Pin to collection": "Pin to collection",
-  "Enable embeds": "Enable embeds",
-  "Disable embeds": "Disable embeds",
   "New nested document": "New nested document",
   "Create template": "Create template",
   "Duplicate": "Duplicate",
   "Unpublish": "Unpublish",
   "Permanently delete": "Permanently delete",
   "Move": "Move",
+  "Enable embeds": "Enable embeds",
+  "Disable embeds": "Disable embeds",
   "Download": "Download",
   "Print": "Print",
   "Move {{ documentName }}": "Move {{ documentName }}",