From 42e133b6de6451c7819096c34c72319c7826382d Mon Sep 17 00:00:00 2001
From: Oliver Sand <oliver.sand@sda-se.com>
Date: Thu, 6 May 2021 10:55:48 +0200
Subject: [PATCH] Fix table toolbar rerendering to often and losing focus

Signed-off-by: Oliver Sand <oliver.sand@sda-se.com>
---
 .changeset/short-bobcats-mate.md             |   3 +-
 packages/core/src/components/Table/Table.tsx | 136 +++++++++++--------
 2 files changed, 81 insertions(+), 58 deletions(-)

diff --git a/.changeset/short-bobcats-mate.md b/.changeset/short-bobcats-mate.md
index 22aaf11d77..d63e7058a9 100644
--- a/.changeset/short-bobcats-mate.md
+++ b/.changeset/short-bobcats-mate.md
@@ -3,4 +3,5 @@
 '@backstage/plugin-api-docs': patch
 ---
 
-Fix setting the state with `useQueryParamState` makes inputs lose their focus.
+Fix state persisted in the URL make search input in the table toolbar lose their
+focus.
diff --git a/packages/core/src/components/Table/Table.tsx b/packages/core/src/components/Table/Table.tsx
index 2251b8304e..1dd09e7ab8 100644
--- a/packages/core/src/components/Table/Table.tsx
+++ b/packages/core/src/components/Table/Table.tsx
@@ -20,6 +20,7 @@ import {
   makeStyles,
   Typography,
   useTheme,
+  withStyles,
 } from '@material-ui/core';
 // Material-table is not using the standard icons available in in material-ui. https://github.com/mbrn/material-table/issues/51
 import AddBox from '@material-ui/icons/AddBox';
@@ -49,6 +50,7 @@ import MTable, {
 } from 'material-table';
 import React, {
   forwardRef,
+  MutableRefObject,
   ReactNode,
   useCallback,
   useEffect,
@@ -100,19 +102,20 @@ function extractValueByField(data: any, field: string): any | undefined {
   return value;
 }
 
-const useHeaderStyles = makeStyles<BackstageTheme>(theme => ({
+const StyledMTableHeader = withStyles(theme => ({
   header: {
     padding: theme.spacing(1, 2, 1, 2.5),
     borderTop: `1px solid ${theme.palette.grey.A100}`,
     borderBottom: `1px solid ${theme.palette.grey.A100}`,
-    color: theme.palette.textSubtle,
+    // withStyles hasn't a generic overload for theme
+    color: (theme as BackstageTheme).palette.textSubtle,
     fontWeight: theme.typography.fontWeightBold,
     position: 'static',
     wordBreak: 'normal',
   },
-}));
+}))(MTableHeader);
 
-const useToolbarStyles = makeStyles<BackstageTheme>(theme => ({
+const StyledMTableToolbar = withStyles(theme => ({
   root: {
     padding: theme.spacing(3, 0, 2.5, 2.5),
   },
@@ -124,7 +127,7 @@ const useToolbarStyles = makeStyles<BackstageTheme>(theme => ({
   searchField: {
     paddingRight: theme.spacing(2),
   },
-}));
+}))(MTableToolbar);
 
 const useFilterStyles = makeStyles<BackstageTheme>(() => ({
   root: {
@@ -208,6 +211,59 @@ export interface TableProps<T extends object = {}>
   onStateChange?: (state: TableState) => any;
 }
 
+export function TableToolbar(toolbarProps: {
+  toolbarRef: MutableRefObject<any>;
+  setSearch: (value: string) => void;
+  onSearchChanged: (value: string) => void;
+  toggleFilters: () => void;
+  hasFilters: boolean;
+  selectedFiltersLength: number;
+}) {
+  const {
+    toolbarRef,
+    setSearch,
+    hasFilters,
+    selectedFiltersLength,
+    toggleFilters,
+  } = toolbarProps;
+  const filtersClasses = useFilterStyles();
+  const onSearchChanged = useCallback(
+    (searchText: string) => {
+      toolbarProps.onSearchChanged(searchText);
+      setSearch(searchText);
+    },
+    [toolbarProps, setSearch],
+  );
+
+  if (hasFilters) {
+    return (
+      <div className={filtersClasses.root}>
+        <div className={filtersClasses.root}>
+          <IconButton onClick={toggleFilters} aria-label="filter list">
+            <FilterList />
+          </IconButton>
+          <Typography className={filtersClasses.title}>
+            Filters ({selectedFiltersLength})
+          </Typography>
+        </div>
+        <StyledMTableToolbar
+          {...toolbarProps}
+          ref={toolbarRef}
+          onSearchChanged={onSearchChanged}
+        />
+      </div>
+    );
+  }
+
+  return (
+    <StyledMTableToolbar
+      {...toolbarProps}
+      ref={toolbarRef}
+      onSearchChanged={onSearchChanged}
+    />
+  );
+}
+
 export function Table<T extends object = {}>({
   columns,
   options,
@@ -219,10 +275,7 @@ export function Table<T extends object = {}>({
   onStateChange,
   ...props
 }: TableProps<T>) {
-  const headerClasses = useHeaderStyles();
-  const toolbarClasses = useToolbarStyles();
   const tableClasses = useTableStyles();
-  const filtersClasses = useFilterStyles();
 
   const { data, ...propsWithoutData } = props;
 
@@ -230,9 +283,12 @@ export function Table<T extends object = {}>({
 
   const calculatedInitialState = { ...defaultInitialState, ...initialState };
 
-  const [filtersOpen, toggleFilters] = useState(
+  const [filtersOpen, setFiltersOpen] = useState(
     calculatedInitialState.filtersOpen,
   );
+  const toggleFilters = useCallback(() => setFiltersOpen(v => !v), [
+    setFiltersOpen,
+  ]);
   const [selectedFiltersLength, setSelectedFiltersLength] = useState(0);
   const [tableData, setTableData] = useState(data as any[]);
   const [selectedFilters, setSelectedFilters] = useState(
@@ -379,64 +435,32 @@ export function Table<T extends object = {}>({
     }));
   };
 
+  const hasFilters = !!filters?.length;
   const Toolbar = useCallback(
     toolbarProps => {
-      const onSearchChanged = (searchText: string) => {
-        toolbarProps.onSearchChanged(searchText);
-        setSearch(searchText);
-      };
-
-      if (filters?.length) {
-        return (
-          <div className={filtersClasses.root}>
-            <div className={filtersClasses.root}>
-              <IconButton
-                onClick={() => toggleFilters(el => !el)}
-                aria-label="filter list"
-              >
-                <FilterList />
-              </IconButton>
-              <Typography className={filtersClasses.title}>
-                Filters ({selectedFiltersLength})
-              </Typography>
-            </div>
-            <MTableToolbar
-              classes={toolbarClasses}
-              {...toolbarProps}
-              ref={toolbarRef}
-              onSearchChanged={onSearchChanged}
-            />
-          </div>
-        );
-      }
-
       return (
-        <MTableToolbar
-          classes={toolbarClasses}
+        <TableToolbar
+          toolbarRef={toolbarRef}
+          setSearch={setSearch}
+          hasFilters={hasFilters}
+          selectedFiltersLength={selectedFiltersLength}
+          toggleFilters={toggleFilters}
           {...toolbarProps}
-          ref={toolbarRef}
-          onSearchChanged={onSearchChanged}
         />
       );
     },
-    [
-      filters?.length,
-      selectedFiltersLength,
-      toggleFilters,
-      toolbarClasses,
-      filtersClasses,
-      setSearch,
-      toolbarRef,
-    ],
+    [toggleFilters, hasFilters, selectedFiltersLength, setSearch, toolbarRef],
   );
 
+  const hasNoRows = typeof data !== 'function' && data.length === 0;
+  const columnCount = columns.length;
   const Body = useCallback(
     bodyProps => {
-      if (emptyContent && typeof data !== 'function' && data.length === 0) {
+      if (emptyContent && hasNoRows) {
         return (
           <tbody>
             <tr>
-              <td colSpan={columns.length}>{emptyContent}</td>
+              <td colSpan={columnCount}>{emptyContent}</td>
             </tr>
           </tbody>
         );
@@ -444,7 +468,7 @@ export function Table<T extends object = {}>({
 
       return <MTableBody {...bodyProps} />;
     },
-    [data, emptyContent, columns],
+    [hasNoRows, emptyContent, columnCount],
   );
 
   return (
@@ -458,9 +482,7 @@ export function Table<T extends object = {}>({
       )}
       <MTable<T>
         components={{
-          Header: headerProps => (
-            <MTableHeader classes={headerClasses} {...headerProps} />
-          ),
+          Header: StyledMTableHeader,
           Toolbar,
           Body,
         }}