Refactor TaskName, hide null tooltips, and add fake tab link

apache · Feb 13, 2024 · 7752e9b · 7752e9b
1 parent 5b2e8a4
commit 7752e9b
Show file tree

Hide file tree

Showing 6 changed files with 60 additions and 29 deletions.
diff --git a/airflow/www/static/js/dag/TaskName.test.tsx b/airflow/www/static/js/dag/TaskName.test.tsx
@@ -28,28 +28,25 @@ import TaskName from "./TaskName";
 
 describe("Test TaskName", () => {
   test("Displays a normal task name", () => {
-    const { getByText } = render(
-      <TaskName label="test" id="test" onToggle={() => {}} />,
-      { wrapper: ChakraWrapper }
-    );
+    const { getByText } = render(<TaskName label="test" id="test" />, {
+      wrapper: ChakraWrapper,
+    });
 
     expect(getByText("test")).toBeDefined();
   });
 
   test("Displays a mapped task name", () => {
-    const { getByText } = render(
-      <TaskName label="test" id="test" isMapped onToggle={() => {}} />,
-      { wrapper: ChakraWrapper }
-    );
+    const { getByText } = render(<TaskName label="test" id="test" isMapped />, {
+      wrapper: ChakraWrapper,
+    });
 
     expect(getByText("test [ ]")).toBeDefined();
   });
 
   test("Displays a group task name", () => {
-    const { getByText } = render(
-      <TaskName label="test" id="test" isGroup onToggle={() => {}} />,
-      { wrapper: ChakraWrapper }
-    );
+    const { getByText } = render(<TaskName label="test" id="test" isGroup />, {
+      wrapper: ChakraWrapper,
+    });
 
     expect(getByText("test")).toBeDefined();
   });

diff --git a/airflow/www/static/js/dag/TaskName.tsx b/airflow/www/static/js/dag/TaskName.tsx
@@ -17,14 +17,13 @@
  * under the License.
  */
 
-import React, { MouseEventHandler, CSSProperties } from "react";
+import React, { CSSProperties } from "react";
 import { Text, TextProps, useTheme } from "@chakra-ui/react";
 import { FiChevronUp, FiArrowUpRight, FiArrowDownRight } from "react-icons/fi";
 
 interface Props extends TextProps {
   isGroup?: boolean;
   isMapped?: boolean;
-  onToggle: MouseEventHandler<HTMLDivElement>;
   isOpen?: boolean;
   label: string;
   id?: string;
@@ -35,7 +34,6 @@ interface Props extends TextProps {
 const TaskName = ({
   isGroup = false,
   isMapped = false,
-  onToggle,
   isOpen = false,
   label,
   id,
@@ -51,8 +49,7 @@ const TaskName = ({
   };
   return (
     <Text
-      cursor={isGroup ? "pointer" : undefined}
-      onClick={onToggle}
+      cursor="pointer"
       data-testid={id}
       width="100%"
       color={colors.gray[800]}

diff --git a/airflow/www/static/js/dag/details/graph/Node.tsx b/airflow/www/static/js/dag/details/graph/Node.tsx
@@ -144,7 +144,7 @@ export const BaseNode = ({
           label={taskName}
           isOpen={isOpen}
           isGroup={!!childCount}
-          onToggle={(e) => {
+          onClick={(e) => {
             e.stopPropagation();
             onToggleCollapse();
           }}

diff --git a/airflow/www/static/js/dag/details/index.tsx b/airflow/www/static/js/dag/details/index.tsx
@@ -27,6 +27,7 @@ import {
   TabPanels,
   Tab,
   Text,
+  Button,
 } from "@chakra-ui/react";
 import { useSearchParams } from "react-router-dom";
 
@@ -40,6 +41,7 @@ import {
   MdCode,
   MdOutlineViewTimeline,
   MdSyncAlt,
+  MdHourglassBottom,
 } from "react-icons/md";
 import { BiBracket } from "react-icons/bi";
 import URLSearchParamsWrapper from "src/utils/URLSearchParamWrapper";
@@ -174,7 +176,8 @@ const Details = ({
     // Default to graph tab when navigating from a task instance to a group/dag/dagrun
     const tabCount = runId && taskId && !isGroup ? 5 : 4;
     if (tabCount === 4 && tabIndex > 3) {
-      onChangeTab(1);
+      if (!runId && taskId) onChangeTab(0);
+      else onChangeTab(1);
     }
   }, [runId, taskId, tabIndex, isGroup, onChangeTab]);
 
@@ -292,6 +295,28 @@ const Details = ({
               </Text>
             </Tab>
           )}
+          {/* Match the styling of a tab but its actually a button */}
+          {!!taskId && !!runId && (
+            <Button
+              variant="unstyled"
+              display="flex"
+              alignItems="center"
+              fontSize="lg"
+              py={3}
+              // need to split pl and pr instead of px
+              pl={4}
+              pr={4}
+              mt="4px"
+              onClick={() => {
+                onSelect({ taskId });
+              }}
+            >
+              <MdHourglassBottom size={16} />
+              <Text as="strong" ml={1}>
+                Task Duration
+              </Text>
+            </Button>
+          )}
         </TabList>
         <TabPanels height="100%">
           <TabPanel height="100%">

diff --git a/airflow/www/static/js/dag/details/task/TaskDuration.tsx b/airflow/www/static/js/dag/details/task/TaskDuration.tsx
@@ -134,16 +134,24 @@ const TaskDuration = () => {
     return `
       Run Id: ${runId} <br>
       ${startCase(orderingLabel)}: ${formatDateTime(data[orderingLabel])} <br>
-      Try Number: ${tryNumber} <br>
+      ${tryNumber && tryNumber > -1 ? `Try Number: ${tryNumber} <br>` : ""}
       State: ${state} <br>
-      Queued: ${queuedDttm && formatDateTime(queuedDttm)} <br>
+      ${queuedDttm ? `Queued: ${formatDateTime(queuedDttm)} <br>` : ""}
       Started: ${startDate && formatDateTime(startDate)} <br>
-      Ended: ${endDate && formatDateTime(endDate)} <br>
-      Queued duration: ${queuedDurationUnit.toFixed(2)} ${unit}<br>
+      Ended: ${endDate && formatDateTime(endDate || undefined)} <br>
+      ${
+        queuedDttm
+          ? `Queued duration: ${queuedDurationUnit.toFixed(2)} ${unit}<br>`
+          : ""
+      }
       Run duration: ${runDurationUnit.toFixed(2)} ${unit}<br>
-      Total duration: ${(queuedDurationUnit + runDurationUnit).toFixed(
-        2
-      )} ${unit}<br>
+      ${
+        queuedDttm
+          ? `Total duration: ${(queuedDurationUnit + runDurationUnit).toFixed(
+              2
+            )} ${unit}<br>`
+          : ""
+      }
     `;
   }
 

diff --git a/airflow/www/static/js/dag/grid/renderTaskRows.tsx b/airflow/www/static/js/dag/grid/renderTaskRows.tsx
@@ -176,15 +176,19 @@ const Row = (props: RowProps) => {
             zIndex={1}
           >
             <TaskName
-              onToggle={memoizedToggle}
+              onClick={(e) => {
+                if (isGroup) {
+                  e.stopPropagation();
+                  memoizedToggle();
+                }
+              }}
               isGroup={isGroup}
               isMapped={task.isMapped && !isParentMapped}
               label={task.label || task.id || ""}
               id={task.id || ""}
               isOpen={isOpen}
-              ml={level * 4 + 4}
+              pl={level * 4 + 4}
               setupTeardownType={task.setupTeardownType}
-              mr={4}
               fontWeight={
                 isGroup || (task.isMapped && !isParentMapped)
                   ? "bold"