From a61fee2a8cfc0e6cea30990cd5efbe401958d325 Mon Sep 17 00:00:00 2001
From: Randy Schott <1815175+schottra@users.noreply.github.com>
Date: Mon, 6 Jul 2020 12:32:17 -0700
Subject: [PATCH] fix: prevents collapsing of execution errors when scrolled
 out of view

---
 .../Tables/ExpandableExecutionError.tsx       |   6 +-
 .../Tables/WorkflowExecutionsTable.tsx        | 116 ++++++++++++------
 .../common/ExpandableMonospaceText.tsx        |   8 +-
 3 files changed, 87 insertions(+), 43 deletions(-)

diff --git a/src/components/Executions/Tables/ExpandableExecutionError.tsx b/src/components/Executions/Tables/ExpandableExecutionError.tsx
index 815cc4518..7716dd2a8 100644
--- a/src/components/Executions/Tables/ExpandableExecutionError.tsx
+++ b/src/components/Executions/Tables/ExpandableExecutionError.tsx
@@ -8,13 +8,15 @@ import { useExecutionTableStyles } from './styles';
  */
 export const ExpandableExecutionError: React.FC<{
     error: ExecutionError;
-    onExpandCollapse?(): void;
-}> = ({ error, onExpandCollapse }) => {
+    initialExpansionState?: boolean;
+    onExpandCollapse?(expanded: boolean): void;
+}> = ({ error, initialExpansionState = false, onExpandCollapse }) => {
     const styles = useExecutionTableStyles();
     return (
         <div className={styles.errorContainer}>
             <ExpandableMonospaceText
                 onExpandCollapse={onExpandCollapse}
+                initialExpansionState={initialExpansionState}
                 text={error.message}
             />
         </div>
diff --git a/src/components/Executions/Tables/WorkflowExecutionsTable.tsx b/src/components/Executions/Tables/WorkflowExecutionsTable.tsx
index cc4b2b203..786486bb4 100644
--- a/src/components/Executions/Tables/WorkflowExecutionsTable.tsx
+++ b/src/components/Executions/Tables/WorkflowExecutionsTable.tsx
@@ -11,12 +11,13 @@ import {
 } from 'common/formatters';
 import { timestampToDate } from 'common/utils';
 import { DataList, DataListRef } from 'components';
+import { getCacheKey } from 'components/Cache';
 import { ListProps } from 'components/common';
 import { useCommonStyles } from 'components/common/styles';
 import { Execution } from 'models';
 import { WorkflowExecutionPhase } from 'models/Execution/enums';
 import * as React from 'react';
-import { ListRowRenderer } from 'react-virtualized';
+import { ListRowProps, ListRowRenderer } from 'react-virtualized';
 import { ExecutionInputsOutputsModal } from '../ExecutionInputsOutputsModal';
 import { ExecutionStatusBadge } from '../ExecutionStatusBadge';
 import { getWorkflowExecutionTimingMS } from '../utils';
@@ -168,6 +169,59 @@ function generateColumns(
     ];
 }
 
+interface WorkflowExecutionRowProps extends ListRowProps {
+    columns: WorkflowExecutionColumnDefinition[];
+    errorExpanded: boolean;
+    execution: Execution;
+    onExpandCollapseError(expanded: boolean): void;
+    state: ReturnType<typeof useWorkflowExecutionsTableState>;
+}
+
+const WorkflowExecutionRow: React.FC<WorkflowExecutionRowProps> = ({
+    columns,
+    errorExpanded,
+    execution,
+    onExpandCollapseError,
+    state,
+    style
+}) => {
+    const { error } = execution.closure;
+    const tableStyles = useExecutionTableStyles();
+    const styles = useStyles();
+
+    return (
+        <div
+            className={classnames(
+                tableStyles.row,
+                styles.row,
+                tableStyles.borderBottom
+            )}
+            style={style}
+        >
+            <div className={tableStyles.rowColumns}>
+                {columns.map(({ className, key: columnKey, cellRenderer }) => (
+                    <div
+                        key={columnKey}
+                        className={classnames(tableStyles.rowColumn, className)}
+                    >
+                        {cellRenderer({
+                            execution,
+                            state
+                        })}
+                    </div>
+                ))}
+            </div>
+            {error ? (
+                <ExpandableExecutionError
+                    onExpandCollapse={onExpandCollapseError}
+                    initialExpansionState={errorExpanded}
+                    error={error}
+                />
+            ) : null}
+        </div>
+    );
+};
+
 export interface WorkflowExecutionsTableProps extends ListProps<Execution> {}
 
 /** Renders a table of WorkflowExecution records. Executions with errors will
@@ -175,11 +229,20 @@ export interface WorkflowExecutionsTableProps extends ListProps<Execution> {}
  */
 export const WorkflowExecutionsTable: React.FC<WorkflowExecutionsTableProps> = props => {
     const executions = props.value;
+    const [expandedErrors, setExpandedErrors] = React.useState<
+        Dictionary<boolean>
+    >({});
     const state = useWorkflowExecutionsTableState();
     const commonStyles = useCommonStyles();
     const styles = useStyles();
     const tableStyles = useExecutionTableStyles();
     const listRef = React.useRef<DataListRef>(null);
+
+    // Reset error expansion states whenever list changes
+    React.useEffect(() => {
+        setExpandedErrors({});
+    }, [executions]);
+
     // Memoizing columns so they won't be re-generated unless the styles change
     const columns = React.useMemo(() => generateColumns(styles, tableStyles), [
         styles,
@@ -197,44 +260,21 @@ export const WorkflowExecutionsTable: React.FC<WorkflowExecutionsTableProps> = p
     // Custom renderer to allow us to append error content to executions which
     // are in a failed state
     const rowRenderer: ListRowRenderer = rowProps => {
-        const { index: rowIndex, style } = rowProps;
-        const execution = executions[rowIndex];
-        const { error } = execution.closure;
-
+        const execution = executions[rowProps.index];
+        const cacheKey = getCacheKey(execution.id);
+        const onExpandCollapseError = (expanded: boolean) => {
+            setExpandedErrors({ ...expandedErrors, [cacheKey]: expanded });
+            recomputeRow(rowProps.index);
+        };
         return (
-            <div
-                className={classnames(
-                    tableStyles.row,
-                    styles.row,
-                    tableStyles.borderBottom
-                )}
-                style={style}
-            >
-                <div className={tableStyles.rowColumns}>
-                    {columns.map(
-                        ({ className, key: columnKey, cellRenderer }) => (
-                            <div
-                                key={columnKey}
-                                className={classnames(
-                                    tableStyles.rowColumn,
-                                    className
-                                )}
-                            >
-                                {cellRenderer({
-                                    execution,
-                                    state
-                                })}
-                            </div>
-                        )
-                    )}
-                </div>
-                {error ? (
-                    <ExpandableExecutionError
-                        onExpandCollapse={recomputeRow.bind(null, rowIndex)}
-                        error={error}
-                    />
-                ) : null}
-            </div>
+            <WorkflowExecutionRow
+                {...rowProps}
+                columns={columns}
+                execution={execution}
+                errorExpanded={!!expandedErrors[cacheKey]}
+                onExpandCollapseError={onExpandCollapseError}
+                state={state}
+            />
         );
     };
 
diff --git a/src/components/common/ExpandableMonospaceText.tsx b/src/components/common/ExpandableMonospaceText.tsx
index d7b15c02b..34b5ce558 100644
--- a/src/components/common/ExpandableMonospaceText.tsx
+++ b/src/components/common/ExpandableMonospaceText.tsx
@@ -105,8 +105,9 @@ export const useExpandableMonospaceTextStyles = makeStyles((theme: Theme) => ({
 }));
 
 export interface ExpandableMonospaceTextProps {
+    initialExpansionState?: boolean;
     text: string;
-    onExpandCollapse?(): void;
+    onExpandCollapse?(expanded: boolean): void;
 }
 
 /** An expandable/collapsible container which renders the provided text in a
@@ -114,13 +115,14 @@ export interface ExpandableMonospaceTextProps {
  */
 export const ExpandableMonospaceText: React.FC<ExpandableMonospaceTextProps> = ({
     onExpandCollapse,
+    initialExpansionState = false,
     text
 }) => {
-    const [expanded, setExpanded] = React.useState(false);
+    const [expanded, setExpanded] = React.useState(initialExpansionState);
     const styles = useExpandableMonospaceTextStyles();
     const onClickExpand = () => {
         setExpanded(!expanded);
-        typeof onExpandCollapse === 'function' && onExpandCollapse();
+        typeof onExpandCollapse === 'function' && onExpandCollapse(!expanded);
     };
     const onClickCopy = () => copyToClipboard(text);