From 8b4291537bc081ea0fce1629aab58864a577b6e0 Mon Sep 17 00:00:00 2001
From: Julius Volz <julius.volz@gmail.com>
Date: Sun, 8 Sep 2024 15:17:00 +0200
Subject: [PATCH] Clarify explain view, add tree view close button, fix
 callback bug

Signed-off-by: Julius Volz <julius.volz@gmail.com>
---
 .../pages/query/ExplainViews/ExplainView.tsx  | 33 +++++-----
 .../mantine-ui/src/pages/query/QueryPanel.tsx | 19 +++---
 .../mantine-ui/src/pages/query/TreeNode.tsx   | 60 +++++++++----------
 .../mantine-ui/src/pages/query/TreeView.tsx   | 15 ++++-
 4 files changed, 69 insertions(+), 58 deletions(-)

diff --git a/web/ui/mantine-ui/src/pages/query/ExplainViews/ExplainView.tsx b/web/ui/mantine-ui/src/pages/query/ExplainViews/ExplainView.tsx
index 907830757..e089c7851 100644
--- a/web/ui/mantine-ui/src/pages/query/ExplainViews/ExplainView.tsx
+++ b/web/ui/mantine-ui/src/pages/query/ExplainViews/ExplainView.tsx
@@ -11,32 +11,35 @@ import classes from "./ExplainView.module.css";
 import SelectorExplainView from "./Selector";
 import AggregationExplainView from "./Aggregation";
 import BinaryExprExplainView from "./BinaryExpr/BinaryExpr";
+import { IconInfoCircle } from "@tabler/icons-react";
 interface ExplainViewProps {
   node: ASTNode | null;
   treeShown: boolean;
-  setShowTree: () => void;
+  showTree: () => void;
 }
 
 const ExplainView: FC<ExplainViewProps> = ({
   node,
   treeShown,
-  setShowTree,
+  showTree: setShowTree,
 }) => {
   if (node === null) {
     return (
-      <Alert>
-        <>
-          To use the Explain view,{" "}
-          {!treeShown && (
-            <>
-              <Anchor fz="unset" onClick={setShowTree}>
-                enable the query tree view
-              </Anchor>{" "}
-              (also available via the expression input dropdown) and then
-            </>
-          )}{" "}
-          select a node in the tree above.
-        </>
+      <Alert title="How to use the Explain view" icon={<IconInfoCircle />}>
+        This tab can help you understand the behavior of individual components
+        of a query.
+        <br />
+        <br />
+        To use the Explain view,{" "}
+        {!treeShown && (
+          <>
+            <Anchor fz="unset" onClick={setShowTree}>
+              enable the query tree view
+            </Anchor>{" "}
+            (also available via the expression input menu) and then
+          </>
+        )}{" "}
+        select a node in the tree above.
       </Alert>
     );
   }
diff --git a/web/ui/mantine-ui/src/pages/query/QueryPanel.tsx b/web/ui/mantine-ui/src/pages/query/QueryPanel.tsx
index 8cddd601a..09deb95e3 100644
--- a/web/ui/mantine-ui/src/pages/query/QueryPanel.tsx
+++ b/web/ui/mantine-ui/src/pages/query/QueryPanel.tsx
@@ -6,16 +6,13 @@ import {
   Box,
   SegmentedControl,
   Stack,
-  Button,
   Skeleton,
 } from "@mantine/core";
 import {
   IconChartAreaFilled,
   IconChartLine,
-  IconCheckbox,
   IconGraph,
   IconInfoCircle,
-  IconSquare,
   IconTable,
 } from "@tabler/icons-react";
 import { FC, Suspense, useCallback, useMemo, useState } from "react";
@@ -129,6 +126,10 @@ const QueryPanel: FC<PanelProps> = ({ idx, metricNames }) => {
               retriggerIdx={retriggerIdx}
               selectedNode={selectedNode}
               setSelectedNode={setSelectedNode}
+              closeTreeView={() => {
+                dispatch(setShowTree({ idx, showTree: false }));
+                setSelectedNode(null);
+              }}
             />
           </Suspense>
         </ErrorBoundary>
@@ -165,11 +166,7 @@ const QueryPanel: FC<PanelProps> = ({ idx, metricNames }) => {
         <Tabs.Panel pt="sm" value="table">
           <TableTab expr={expr} panelIdx={idx} retriggerIdx={retriggerIdx} />
         </Tabs.Panel>
-        <Tabs.Panel
-          pt="sm"
-          value="graph"
-          // style={{ border: "1px solid lightgrey", borderTop: "none" }}
-        >
+        <Tabs.Panel pt="sm" value="graph">
           <Group mt="xs" justify="space-between">
             <Group>
               <RangeInput
@@ -214,7 +211,7 @@ const QueryPanel: FC<PanelProps> = ({ idx, metricNames }) => {
             </Group>
 
             <Group gap="lg">
-              <Button
+              {/* <Button
                 variant="subtle"
                 color="gray.9"
                 size="xs"
@@ -249,7 +246,7 @@ const QueryPanel: FC<PanelProps> = ({ idx, metricNames }) => {
                 }
               >
                 Show exemplars
-              </Button>
+              </Button> */}
 
               <SegmentedControl
                 onChange={(value) =>
@@ -325,7 +322,7 @@ const QueryPanel: FC<PanelProps> = ({ idx, metricNames }) => {
               <ExplainView
                 node={selectedNode?.node ?? null}
                 treeShown={panel.showTree}
-                setShowTree={() => {
+                showTree={() => {
                   dispatch(setShowTree({ idx, showTree: true }));
                 }}
               />
diff --git a/web/ui/mantine-ui/src/pages/query/TreeNode.tsx b/web/ui/mantine-ui/src/pages/query/TreeNode.tsx
index 102396f6e..8ac176e58 100644
--- a/web/ui/mantine-ui/src/pages/query/TreeNode.tsx
+++ b/web/ui/mantine-ui/src/pages/query/TreeNode.tsx
@@ -1,5 +1,6 @@
 import {
   FC,
+  useCallback,
   useEffect,
   useLayoutEffect,
   useMemo,
@@ -57,8 +58,10 @@ const TreeNode: FC<{
   selectedNode: { id: string; node: ASTNode } | null;
   setSelectedNode: (Node: { id: string; node: ASTNode } | null) => void;
   parentRef?: React.RefObject<HTMLDivElement>;
-  reportNodeState?: (state: NodeState) => void;
+  reportNodeState?: (childIdx: number, state: NodeState) => void;
   reverse: boolean;
+  // The index of this node in its parent's children.
+  childIdx: number;
 }> = ({
   node,
   selectedNode,
@@ -66,6 +69,7 @@ const TreeNode: FC<{
   parentRef,
   reportNodeState,
   reverse,
+  childIdx,
 }) => {
   const nodeID = useId();
   const nodeRef = useRef<HTMLDivElement>(null);
@@ -130,21 +134,32 @@ const TreeNode: FC<{
 
   useEffect(() => {
     if (mergedChildState === "error") {
-      reportNodeState && reportNodeState("error");
+      reportNodeState && reportNodeState(childIdx, "error");
     }
-  }, [mergedChildState, reportNodeState]);
+  }, [mergedChildState, reportNodeState, childIdx]);
 
   useEffect(() => {
     if (error) {
-      reportNodeState && reportNodeState("error");
+      reportNodeState && reportNodeState(childIdx, "error");
     }
-  }, [error, reportNodeState]);
+  }, [error, reportNodeState, childIdx]);
 
   useEffect(() => {
     if (isFetching) {
-      reportNodeState && reportNodeState("running");
+      reportNodeState && reportNodeState(childIdx, "running");
     }
-  }, [isFetching, reportNodeState]);
+  }, [isFetching, reportNodeState, childIdx]);
+
+  const childReportNodeState = useCallback(
+    (childIdx: number, state: NodeState) => {
+      setChildStates((prev) => {
+        const newStates = [...prev];
+        newStates[childIdx] = state;
+        return newStates;
+      });
+    },
+    [setChildStates]
+  );
 
   // Update the size and position of tree connector lines based on the node's and its parent's position.
   useLayoutEffect(() => {
@@ -185,7 +200,7 @@ const TreeNode: FC<{
       return;
     }
 
-    reportNodeState && reportNodeState("success");
+    reportNodeState && reportNodeState(childIdx, "success");
 
     let resultSeries = 0;
     const labelValuesByName: Record<string, Record<string, number>> = {};
@@ -228,7 +243,7 @@ const TreeNode: FC<{
       ),
       labelExamples,
     });
-  }, [data, reportNodeState]);
+  }, [data, reportNodeState, childIdx]);
 
   const innerNode = (
     <Group
@@ -367,13 +382,8 @@ const TreeNode: FC<{
             setSelectedNode={setSelectedNode}
             parentRef={nodeRef}
             reverse={true}
-            reportNodeState={(state: NodeState) => {
-              setChildStates((prev) => {
-                const newStates = [...prev];
-                newStates[0] = state;
-                return newStates;
-              });
-            }}
+            childIdx={0}
+            reportNodeState={childReportNodeState}
           />
         </Box>
         {innerNode}
@@ -384,13 +394,8 @@ const TreeNode: FC<{
             setSelectedNode={setSelectedNode}
             parentRef={nodeRef}
             reverse={false}
-            reportNodeState={(state: NodeState) => {
-              setChildStates((prev) => {
-                const newStates = [...prev];
-                newStates[1] = state;
-                return newStates;
-              });
-            }}
+            childIdx={1}
+            reportNodeState={childReportNodeState}
           />
         </Box>
       </div>
@@ -407,13 +412,8 @@ const TreeNode: FC<{
               setSelectedNode={setSelectedNode}
               parentRef={nodeRef}
               reverse={false}
-              reportNodeState={(state: NodeState) => {
-                setChildStates((prev) => {
-                  const newStates = [...prev];
-                  newStates[idx] = state;
-                  return newStates;
-                });
-              }}
+              childIdx={idx}
+              reportNodeState={childReportNodeState}
             />
           </Box>
         ))}
diff --git a/web/ui/mantine-ui/src/pages/query/TreeView.tsx b/web/ui/mantine-ui/src/pages/query/TreeView.tsx
index 01bfcd750..1455e4377 100644
--- a/web/ui/mantine-ui/src/pages/query/TreeView.tsx
+++ b/web/ui/mantine-ui/src/pages/query/TreeView.tsx
@@ -3,7 +3,7 @@ import { useSuspenseAPIQuery } from "../../api/api";
 import { useAppSelector } from "../../state/hooks";
 import ASTNode from "../../promql/ast";
 import TreeNode from "./TreeNode";
-import { Card } from "@mantine/core";
+import { Card, CloseButton } from "@mantine/core";
 
 const TreeView: FC<{
   panelIdx: number;
@@ -19,7 +19,8 @@ const TreeView: FC<{
       node: ASTNode;
     } | null
   ) => void;
-}> = ({ panelIdx, selectedNode, setSelectedNode }) => {
+  closeTreeView: () => void;
+}> = ({ panelIdx, selectedNode, setSelectedNode, closeTreeView }) => {
   const { expr } = useAppSelector((state) => state.queryPage.panels[panelIdx]);
 
   const { data } = useSuspenseAPIQuery<ASTNode>({
@@ -32,7 +33,17 @@ const TreeView: FC<{
 
   return (
     <Card withBorder fz="sm" style={{ overflowX: "auto" }} pl="sm">
+      <CloseButton
+        aria-label="Close tree view"
+        title="Close tree view"
+        pos="absolute"
+        top={7}
+        size="sm"
+        right={7}
+        onClick={closeTreeView}
+      />
       <TreeNode
+        childIdx={0}
         node={data.data}
         selectedNode={selectedNode}
         setSelectedNode={setSelectedNode}