chore: fix expander bug (#677)

* chore: fix expander bug Signed-off-by: Carina Ursu <[email protected]> * chore: add await everywhere Signed-off-by: Carina Ursu <[email protected]> --------- Signed-off-by: Carina Ursu <[email protected]>
flyteorg · Feb 2, 2023 · 6a48658 · 6a48658
1 parent d11850c
commit 6a48658
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 38 deletions.
diff --git a/packages/console/src/components/Executions/ExecutionDetails/Timeline/ExecutionTimeline.tsx b/packages/console/src/components/Executions/ExecutionDetails/Timeline/ExecutionTimeline.tsx
@@ -172,7 +172,7 @@ export const ExecutionTimeline: React.FC<ExProps> = ({
   };
 
   const toggleNode = async (id: string, scopedId: string, level: number) => {
-    fetchChildrenExecutions(
+    await fetchChildrenExecutions(
       queryClient,
       scopedId,
       nodeExecutionsById,

diff --git a/packages/console/src/components/Executions/ExecutionDetails/Timeline/TaskNames.tsx b/packages/console/src/components/Executions/ExecutionDetails/Timeline/TaskNames.tsx
@@ -56,6 +56,8 @@ export const TaskNames = React.forwardRef<HTMLDivElement, TaskNamesProps>(
     const styles = useStyles();
     const { nodeExecutionsById } = useContext(NodeExecutionsByIdContext);
 
+    const expanderRef = React.useRef<HTMLButtonElement>();
+
     return (
       <div className={styles.taskNamesList} ref={ref} onScroll={onScroll}>
         {nodes.map(node => {
@@ -84,6 +86,7 @@ export const TaskNames = React.forwardRef<HTMLDivElement, TaskNamesProps>(
                 <div className={styles.namesContainerExpander}>
                   {nodeExecution && isParentNode(nodeExecution) ? (
                     <RowExpander
+                      ref={expanderRef as React.ForwardedRef<HTMLButtonElement>}
                       expanded={node.expanded || false}
                       onClick={() =>
                         onToggle(node.id, node.scopedId, nodeLevel)

diff --git a/packages/console/src/components/Executions/Tables/NodeExecutionRow.tsx b/packages/console/src/components/Executions/Tables/NodeExecutionRow.tsx
@@ -44,12 +44,13 @@ export const NodeExecutionRow: React.FC<NodeExecutionRowProps> = ({
 }) => {
   const styles = useStyles();
   const theme = useTheme();
+  const expanderRef = React.useRef<HTMLButtonElement>();
+
   const tableStyles = useExecutionTableStyles();
   const { selectedExecution, setSelectedExecution } =
     useContext(DetailsPanelContext);
 
   const nodeLevel = node?.level ?? 0;
-  const expanded = isExpanded(node);
 
   // For the first level, we want the borders to span the entire table,
   // so we'll use padding to space the content. For nested rows, we want the
@@ -66,18 +67,20 @@ export const NodeExecutionRow: React.FC<NodeExecutionRowProps> = ({
     ? isEqual(selectedExecution, nodeExecution)
     : false;
 
-  const expanderContent = (
-    <div className={styles.namesContainerExpander}>
-      {isParentNode(nodeExecution) ? (
-        <RowExpander
-          expanded={expanded}
-          onClick={() => onToggle(node.id, node.scopedId, nodeLevel)}
-        />
-      ) : (
-        <div className={styles.leaf} />
-      )}
-    </div>
-  );
+  const expanderContent = React.useMemo(() => {
+    return isParentNode(nodeExecution) ? (
+      <RowExpander
+        ref={expanderRef as React.ForwardedRef<HTMLButtonElement>}
+        key={node.scopedId}
+        expanded={isExpanded(node)}
+        onClick={() => {
+          onToggle(node.id, node.scopedId, nodeLevel);
+        }}
+      />
+    ) : (
+      <div className={styles.leaf} />
+    );
+  }, [node, nodeLevel]);
 
   // open the side panel for selected execution's detail
   // use null in case if there is no execution provided - when it is null, will close side panel
@@ -99,7 +102,9 @@ export const NodeExecutionRow: React.FC<NodeExecutionRowProps> = ({
           <div
             className={classnames(tableStyles.rowColumn, tableStyles.expander)}
           >
-            {expanderContent}
+            <div className={styles.namesContainerExpander}>
+              {expanderContent}
+            </div>
           </div>
           {columns.map(({ className, key: columnKey, cellRenderer }) => (
             <div

diff --git a/packages/console/src/components/Executions/Tables/NodeExecutionsTable.tsx b/packages/console/src/components/Executions/Tables/NodeExecutionsTable.tsx
@@ -13,6 +13,7 @@ import {
   isStartNode,
 } from 'components/WorkflowGraph/utils';
 import { useQueryClient } from 'react-query';
+import { merge, eq } from 'lodash';
 import { ExecutionsTableHeader } from './ExecutionsTableHeader';
 import { generateColumns } from './nodeExecutionColumns';
 import { NoExecutionsContent } from './NoExecutionsContent';
@@ -72,9 +73,19 @@ export const NodeExecutionsTable: React.FC<NodeExecutionsTableProps> = ({
   );
 
   useEffect(() => {
-    setOriginalNodes(
-      appliedFilters.length > 0 && filteredNodes ? filteredNodes : initialNodes,
-    );
+    setOriginalNodes(ogn => {
+      const newNodes =
+        appliedFilters.length > 0 && filteredNodes
+          ? filteredNodes
+          : merge(initialNodes, ogn);
+
+      if (!eq(newNodes, ogn)) {
+        return newNodes;
+      }
+
+      return ogn;
+    });
+
     const plainNodes = convertToPlainNodes(originalNodes);
     const updatedShownNodesMap = plainNodes.map(node => {
       const execution = nodeExecutionsById[node.scopedId];
@@ -88,7 +99,7 @@ export const NodeExecutionsTable: React.FC<NodeExecutionsTableProps> = ({
   }, [initialNodes, filteredNodes, originalNodes, nodeExecutionsById]);
 
   const toggleNode = async (id: string, scopedId: string, level: number) => {
-    fetchChildrenExecutions(
+    await fetchChildrenExecutions(
       queryClient,
       scopedId,
       nodeExecutionsById,

diff --git a/packages/console/src/components/Executions/Tables/RowExpander.tsx b/packages/console/src/components/Executions/Tables/RowExpander.tsx
@@ -1,25 +1,34 @@
+import * as React from 'react';
 import { IconButton } from '@material-ui/core';
 import ChevronRight from '@material-ui/icons/ChevronRight';
 import ExpandMore from '@material-ui/icons/ExpandMore';
-import * as React from 'react';
 import t from './strings';
 
-/** A simple expand/collapse arrow to be rendered next to row items. */
-export const RowExpander: React.FC<{
+interface RowExpanderProps {
   expanded: boolean;
+  key?: string;
   onClick: () => void;
-}> = ({ expanded, onClick }) => (
-  <IconButton
-    disableRipple={true}
-    disableTouchRipple={true}
-    size="small"
-    title={t('expanderTitle', expanded)}
-    onClick={(e: React.MouseEvent<HTMLElement>) => {
-      // prevent the parent row body onClick event trigger
-      e.stopPropagation();
-      onClick();
-    }}
-  >
-    {expanded ? <ExpandMore /> : <ChevronRight />}
-  </IconButton>
-);
+}
+/** A simple expand/collapse arrow to be rendered next to row items. */
+export const RowExpander = React.forwardRef<
+  HTMLButtonElement,
+  RowExpanderProps
+>(({ expanded, key, onClick }, ref) => {
+  return (
+    <IconButton
+      key={key}
+      ref={ref}
+      disableRipple={true}
+      disableTouchRipple={true}
+      size="small"
+      title={t('expanderTitle', expanded)}
+      onClick={(e: React.MouseEvent<HTMLElement>) => {
+        // prevent the parent row body onClick event trigger
+        e.stopPropagation();
+        onClick();
+      }}
+    >
+      {expanded ? <ExpandMore /> : <ChevronRight />}
+    </IconButton>
+  );
+});
diff --git a/packages/console/src/components/flytegraph/ReactFlow/ReactFlowWrapper.tsx b/packages/console/src/components/flytegraph/ReactFlow/ReactFlowWrapper.tsx
@@ -114,7 +114,7 @@ export const ReactFlowWrapper: React.FC<RFWrapperProps> = ({
   const onNodeClick = async (_event, node) => {
     const scopedId = node.data.scopedId;
     if (node.data.isParentNode) {
-      fetchChildrenExecutions(
+      await fetchChildrenExecutions(
         queryClient,
         scopedId,
         nodeExecutionsById,