From 9858ad74ab6f81c55c06c8f05b20ccd0ec67dd7a Mon Sep 17 00:00:00 2001
From: Lucas Di Pentima <lucas.dipentima@curii.com>
Date: Thu, 14 Apr 2022 10:24:33 -0300
Subject: [PATCH] 18972: Improves implementation on DataExplorer's "loading..."
 indicator.

Instead of using globals, I think this solution is more elegant, as it's
based on local component state.

Arvados-DCO-1.1-Signed-off-by: Lucas Di Pentima <lucas.dipentima@curii.com>
---
 .../data-explorer/data-explorer.tsx           | 48 +++++++++++++++++--
 .../data-explorer/data-explorer.tsx           | 30 ++++--------
 2 files changed, 52 insertions(+), 26 deletions(-)

diff --git a/src/components/data-explorer/data-explorer.tsx b/src/components/data-explorer/data-explorer.tsx
index 051f5d34..0363d333 100644
--- a/src/components/data-explorer/data-explorer.tsx
+++ b/src/components/data-explorer/data-explorer.tsx
@@ -66,6 +66,8 @@ interface DataExplorerDataProps<T> {
     contextMenuColumn: boolean;
     dataTableDefaultView?: React.ReactNode;
     working?: boolean;
+    currentRefresh?: string;
+    currentRoute?: string;
     hideColumnSelector?: boolean;
     paperProps?: PaperProps;
     actions?: React.ReactNode;
@@ -96,16 +98,55 @@ type DataExplorerProps<T> = DataExplorerDataProps<T> &
 
 export const DataExplorer = withStyles(styles)(
     class DataExplorerGeneric<T> extends React.Component<DataExplorerProps<T>> {
+        state = {
+            showLoading: false,
+            prevRefresh: '',
+            prevRoute: '',
+        };
+
+        componentDidUpdate(prevProps: DataExplorerProps<T>) {
+            const currentRefresh = this.props.currentRefresh || '';
+            const currentRoute = this.props.currentRoute || '';
+
+            if (currentRoute !== this.state.prevRoute) {
+                // Component already mounted, but the user comes from a route change,
+                // like browsing through a project hierarchy.
+                this.setState({
+                    showLoading: this.props.working,
+                    prevRoute: currentRoute,
+                });
+            }
+
+            if (currentRefresh !== this.state.prevRefresh) {
+                // Component already mounted, but the user just clicked the
+                // refresh button.
+                this.setState({
+                    showLoading: this.props.working,
+                    prevRefresh: currentRefresh,
+                });
+            }
+            if (this.state.showLoading && !this.props.working) {
+                this.setState({
+                    showLoading: false,
+                });
+            }
+        }
 
         componentDidMount() {
             if (this.props.onSetColumns) {
                 this.props.onSetColumns(this.props.columns);
             }
+            // Component just mounted, so we need to show the loading indicator.
+            this.setState({
+                showLoading: this.props.working,
+                prevRefresh: this.props.currentRefresh || '',
+                prevRoute: this.props.currentRoute || '',
+            });
         }
 
         render() {
             const {
-                columns, onContextMenu, onFiltersChange, onSortToggle, working, extractKey,
+                columns, onContextMenu, onFiltersChange, onSortToggle, extractKey,
                 rowsPerPage, rowsPerPageOptions, onColumnToggle, searchLabel, searchValue, onSearch,
                 items, itemsAvailable, onRowClick, onRowDoubleClick, classes,
                 dataTableDefaultView, hideColumnSelector, actions, paperProps, hideSearchInput,
@@ -113,8 +154,7 @@ export const DataExplorer = withStyles(styles)(
                 doHidePanel, doMaximizePanel, panelName, panelMaximized, elementPath
             } = this.props;
 
-            const dataCy = this.props["data-cy"];
-            return <Paper className={classes.root} {...paperProps} key={paperKey} data-cy={dataCy}>
+            return <Paper className={classes.root} {...paperProps} key={paperKey} data-cy={this.props["data-cy"]}>
                 <Grid container direction="column" wrap="nowrap" className={classes.container}>
                     <div>
                         {title && <Grid item xs className={classes.title}>{title}</Grid>}
@@ -156,7 +196,7 @@ export const DataExplorer = withStyles(styles)(
                     onFiltersChange={onFiltersChange}
                     onSortToggle={onSortToggle}
                     extractKey={extractKey}
-                    working={working}
+                    working={this.state.showLoading}
                     defaultView={dataTableDefaultView}
                     currentItemUuid={currentItemUuid}
                     currentRoute={paperKey} /></Grid>
diff --git a/src/views-components/data-explorer/data-explorer.tsx b/src/views-components/data-explorer/data-explorer.tsx
index 97e20b0d..06d97038 100644
--- a/src/views-components/data-explorer/data-explorer.tsx
+++ b/src/views-components/data-explorer/data-explorer.tsx
@@ -21,11 +21,6 @@ interface Props {
     extractKey?: (item: any) => React.Key;
 }
 
-let prevRoute = '';
-let prevRefresh = '';
-let routeChanged = false;
-let isWorking = false;
-
 const mapStateToProps = (state: RootState, { id }: Props) => {
     const progress = state.progressIndicator.find(p => p.id === id);
     const dataExplorerState = getDataExplorer(state.dataExplorer, id);
@@ -33,23 +28,14 @@ const mapStateToProps = (state: RootState, { id }: Props) => {
     const currentRefresh = localStorage.getItem(LAST_REFRESH_TIMESTAMP) || '';
     const currentItemUuid = currentRoute === '/workflows' ? state.properties.workflowPanelDetailsUuid : state.detailsPanel.resourceUuid;
 
-    if (currentRoute !== prevRoute || currentRefresh !== prevRefresh) {
-        routeChanged = true;
-        prevRoute = currentRoute;
-        prevRefresh = currentRefresh;
-    }
-    if (progress?.working) {
-        isWorking = true;
-    }
-
-    const working = routeChanged && isWorking;
-
-    if (working && !progress?.working) {
-        routeChanged = false;
-        isWorking = false;
-    }
-
-    return { ...dataExplorerState, working, paperKey: currentRoute, currentItemUuid };
+    return {
+        ...dataExplorerState,
+        working: !!progress?.working,
+        currentRefresh: currentRefresh,
+        currentRoute: currentRoute,
+        paperKey: currentRoute,
+        currentItemUuid
+    };
 };
 
 const mapDispatchToProps = () => {
-- 
2.30.2