You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by je...@apache.org on 2024/02/09 18:00:10 UTC
(airflow) 25/26: Adjust node width based on task name length (#37254)
This is an automated email from the ASF dual-hosted git repository.
jedcunningham pushed a commit to branch v2-8-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
commit 6a18db922c00618f7db13ac94a45151f2ce01677
Author: Brent Bovenzi <br...@astronomer.io>
AuthorDate: Fri Feb 9 01:12:35 2024 -0500
Adjust node width based on task name length (#37254)
(cherry picked from commit 9b997638f31fabe9d075a63647e74924f4b5c0c3)
---
.../www/static/js/dag/{grid => }/TaskName.test.tsx | 13 +--
airflow/www/static/js/dag/TaskName.tsx | 89 +++++++++++++++
.../www/static/js/dag/details/graph/Node.test.tsx | 1 -
airflow/www/static/js/dag/details/graph/Node.tsx | 119 +++++++--------------
airflow/www/static/js/dag/grid/TaskName.tsx | 70 ------------
airflow/www/static/js/dag/grid/index.test.tsx | 14 +--
airflow/www/static/js/dag/grid/renderTaskRows.tsx | 12 ++-
airflow/www/static/js/utils/graph.ts | 7 +-
8 files changed, 149 insertions(+), 176 deletions(-)
diff --git a/airflow/www/static/js/dag/grid/TaskName.test.tsx b/airflow/www/static/js/dag/TaskName.test.tsx
similarity index 83%
rename from airflow/www/static/js/dag/grid/TaskName.test.tsx
rename to airflow/www/static/js/dag/TaskName.test.tsx
index 8002ab78ca..27c7e2419e 100644
--- a/airflow/www/static/js/dag/grid/TaskName.test.tsx
+++ b/airflow/www/static/js/dag/TaskName.test.tsx
@@ -38,13 +38,7 @@ describe("Test TaskName", () => {
test("Displays a mapped task name", () => {
const { getByText } = render(
- <TaskName
- level={0}
- label="test"
- id="test"
- isMapped
- onToggle={() => {}}
- />,
+ <TaskName label="test" id="test" isMapped onToggle={() => {}} />,
{ wrapper: ChakraWrapper }
);
@@ -52,12 +46,11 @@ describe("Test TaskName", () => {
});
test("Displays a group task name", () => {
- const { getByText, getByTestId } = render(
- <TaskName level={0} label="test" id="test" isGroup onToggle={() => {}} />,
+ const { getByText } = render(
+ <TaskName label="test" id="test" isGroup onToggle={() => {}} />,
{ wrapper: ChakraWrapper }
);
expect(getByText("test")).toBeDefined();
- expect(getByTestId("open-group")).toBeDefined();
});
});
diff --git a/airflow/www/static/js/dag/TaskName.tsx b/airflow/www/static/js/dag/TaskName.tsx
new file mode 100644
index 0000000000..cf151d7fd7
--- /dev/null
+++ b/airflow/www/static/js/dag/TaskName.tsx
@@ -0,0 +1,89 @@
+/*!
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import React, { MouseEventHandler, 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;
+ setupTeardownType?: string;
+ isZoomedOut?: boolean;
+}
+
+const TaskName = ({
+ isGroup = false,
+ isMapped = false,
+ onToggle,
+ isOpen = false,
+ label,
+ id,
+ setupTeardownType,
+ isZoomedOut,
+ ...rest
+}: Props) => {
+ const { colors } = useTheme();
+ const iconStyle: CSSProperties = {
+ display: "inline",
+ position: "relative",
+ verticalAlign: "middle",
+ };
+ return (
+ <Text
+ cursor={isGroup ? "pointer" : undefined}
+ onClick={onToggle}
+ data-testid={id}
+ width="100%"
+ color={colors.gray[800]}
+ fontSize={isZoomedOut ? 24 : undefined}
+ textAlign="justify"
+ {...rest}
+ >
+ {label}
+ {isMapped && " [ ]"}
+ {isGroup && (
+ <FiChevronUp
+ size={isZoomedOut ? 24 : 15}
+ strokeWidth={3}
+ style={{
+ transition: "transform 0.5s",
+ transform: `rotate(${isOpen ? 0 : 180}deg)`,
+ ...iconStyle,
+ }}
+ />
+ )}
+ {setupTeardownType === "setup" && (
+ <FiArrowUpRight size={isZoomedOut ? 24 : 15} style={iconStyle} />
+ )}
+ {setupTeardownType === "teardown" && (
+ <FiArrowDownRight size={isZoomedOut ? 24 : 15} style={iconStyle} />
+ )}
+ </Text>
+ );
+};
+
+// Only rerender the component if props change
+const MemoizedTaskName = React.memo(TaskName);
+
+export default MemoizedTaskName;
diff --git a/airflow/www/static/js/dag/details/graph/Node.test.tsx b/airflow/www/static/js/dag/details/graph/Node.test.tsx
index 251f3c6aeb..a01114ec04 100644
--- a/airflow/www/static/js/dag/details/graph/Node.test.tsx
+++ b/airflow/www/static/js/dag/details/graph/Node.test.tsx
@@ -110,7 +110,6 @@ describe("Test Graph Node", () => {
expect(getByText("success")).toBeInTheDocument();
expect(getByText("task_id")).toBeInTheDocument();
- expect(getByText("+ 5 tasks")).toBeInTheDocument();
});
test("Renders normal task correctly", async () => {
diff --git a/airflow/www/static/js/dag/details/graph/Node.tsx b/airflow/www/static/js/dag/details/graph/Node.tsx
index 7be5ae7b88..cbe269f997 100644
--- a/airflow/www/static/js/dag/details/graph/Node.tsx
+++ b/airflow/www/static/js/dag/details/graph/Node.tsx
@@ -18,7 +18,7 @@
*/
import React from "react";
-import { Box, Text, Flex, useTheme } from "@chakra-ui/react";
+import { Box, Text, Flex } from "@chakra-ui/react";
import { Handle, NodeProps, Position } from "reactflow";
import { SimpleStatus } from "src/dag/StatusBox";
@@ -28,7 +28,7 @@ import { getGroupAndMapSummary, hoverDelay } from "src/utils";
import Tooltip from "src/components/Tooltip";
import InstanceTooltip from "src/dag/InstanceTooltip";
import { useContainerRef } from "src/context/containerRef";
-import { ImArrowUpRight2, ImArrowDownRight2 } from "react-icons/im";
+import TaskName from "src/dag/TaskName";
export interface CustomNodeProps {
label: string;
@@ -69,7 +69,6 @@ export const BaseNode = ({
isZoomedOut,
},
}: NodeProps<CustomNodeProps>) => {
- const { colors } = useTheme();
const { onSelect } = useSelection();
const containerRef = useContainerRef();
@@ -138,84 +137,48 @@ export const BaseNode = ({
});
}
}}
+ px={isZoomedOut ? 1 : 2}
+ mt={isZoomedOut ? -2 : 0}
>
- <Flex
- justifyContent="space-between"
- width={width}
- p={2}
- flexWrap={isZoomedOut ? "nowrap" : "wrap"}
- flexDirection={isZoomedOut ? "row" : "column"}
- >
- <Flex flexDirection="column" width="100%">
- <Flex
- justifyContent="space-between"
- alignItems="center"
- width="100%"
- fontSize={isZoomedOut ? 25 : undefined}
- fontWeight="bold"
- >
- <Text noOfLines={1} maxWidth={`calc(${width}px - 8px)`}>
- {taskName}
+ <TaskName
+ label={taskName}
+ isOpen={isOpen}
+ isGroup={!!childCount}
+ onToggle={(e) => {
+ e.stopPropagation();
+ onToggleCollapse();
+ }}
+ setupTeardownType={setupTeardownType}
+ fontWeight="bold"
+ isZoomedOut={isZoomedOut}
+ mt={isZoomedOut ? -2 : 0}
+ noOfLines={2}
+ />
+ {!isZoomedOut && (
+ <>
+ {!!instance && instance.state && (
+ <Flex alignItems="center">
+ <SimpleStatus state={instance.state} />
+ <Text ml={2} color="gray.500" fontWeight={400} fontSize="md">
+ {instance.state}
+ </Text>
+ </Flex>
+ )}
+ {task?.operator && (
+ <Text
+ noOfLines={1}
+ maxWidth={`calc(${width}px - 12px)`}
+ fontWeight={400}
+ fontSize="md"
+ width="fit-content"
+ color={operatorTextColor}
+ px={1}
+ >
+ {task.operator}
</Text>
- {setupTeardownType === "setup" && (
- <ImArrowUpRight2 size={15} color={colors.gray[800]} />
- )}
- {setupTeardownType === "teardown" && (
- <ImArrowDownRight2 size={15} color={colors.gray[800]} />
- )}
- </Flex>
- {!isZoomedOut && (
- <>
- {!!instance && instance.state && (
- <Flex alignItems="center">
- <SimpleStatus state={instance.state} />
- <Text
- ml={2}
- color="gray.500"
- fontWeight={400}
- fontSize="md"
- >
- {instance.state}
- </Text>
- </Flex>
- )}
- {task?.operator && (
- <Text
- noOfLines={1}
- maxWidth={`calc(${width}px - 12px)`}
- fontWeight={400}
- fontSize="md"
- width="fit-content"
- color={operatorTextColor}
- px={1}
- >
- {task.operator}
- </Text>
- )}
- </>
)}
- </Flex>
- {!!childCount && (
- <Text
- noOfLines={1}
- fontSize={isZoomedOut ? 25 : undefined}
- color="blue.600"
- cursor="pointer"
- width="150px"
- fontWeight="bold"
- // Increase the target area to expand/collapse a group
- p={2}
- m={-2}
- onClick={(e) => {
- e.stopPropagation();
- onToggleCollapse();
- }}
- >
- {isOpen ? "- " : "+ "}
- {childCount} tasks
- </Text>
- )}
- </Flex>
+ </>
+ )}
</Box>
</Tooltip>
);
diff --git a/airflow/www/static/js/dag/grid/TaskName.tsx b/airflow/www/static/js/dag/grid/TaskName.tsx
deleted file mode 100644
index ce2de10bf6..0000000000
--- a/airflow/www/static/js/dag/grid/TaskName.tsx
+++ /dev/null
@@ -1,70 +0,0 @@
-/*!
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements. See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership. The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License. You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied. See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-import React from "react";
-import { Text, Flex } from "@chakra-ui/react";
-import { FiChevronUp, FiChevronDown } from "react-icons/fi";
-
-interface Props {
- isGroup?: boolean;
- isMapped?: boolean;
- onToggle: () => void;
- isOpen?: boolean;
- level?: number;
- label: string;
- id: string;
-}
-
-const TaskName = ({
- isGroup = false,
- isMapped = false,
- onToggle,
- isOpen = false,
- level = 0,
- label,
- id,
-}: Props) => (
- <Flex
- as={isGroup ? "button" : "div"}
- onClick={onToggle}
- aria-label={label}
- data-testid={id}
- title={label}
- mr={4}
- width="100%"
- alignItems="center"
- fontWeight={isGroup || isMapped ? "bold" : "normal"}
- >
- <Text display="inline" ml={level * 4 + 4} noOfLines={1}>
- {label}
- {isMapped && " [ ]"}
- </Text>
- {isGroup &&
- (isOpen ? (
- <FiChevronUp data-testid="close-group" />
- ) : (
- <FiChevronDown data-testid="open-group" />
- ))}
- </Flex>
-);
-
-// Only rerender the component if props change
-const MemoizedTaskName = React.memo(TaskName);
-
-export default MemoizedTaskName;
diff --git a/airflow/www/static/js/dag/grid/index.test.tsx b/airflow/www/static/js/dag/grid/index.test.tsx
index 5cc6e71788..3322e3b1b0 100644
--- a/airflow/www/static/js/dag/grid/index.test.tsx
+++ b/airflow/www/static/js/dag/grid/index.test.tsx
@@ -193,7 +193,7 @@ describe("Test ToggleGroups", () => {
});
test("Group defaults to closed", () => {
- const { queryAllByTestId, getByText, getAllByTestId } = render(
+ const { getByText, getAllByTestId } = render(
<Grid openGroupIds={[]} onToggleGroups={() => {}} />,
{ wrapper: Wrapper }
);
@@ -204,7 +204,6 @@ describe("Test ToggleGroups", () => {
expect(getAllByTestId("task-instance")).toHaveLength(2);
expect(groupName1).toBeInTheDocument();
expect(groupName2).toBeInTheDocument();
- expect(queryAllByTestId("open-group")).toHaveLength(2);
});
test("Buttons are disabled if all groups are expanded or collapsed", () => {
@@ -246,25 +245,18 @@ describe("Test ToggleGroups", () => {
expect(groupName1).toBeInTheDocument();
expect(groupName2).toBeInTheDocument();
- expect(queryAllByTestId("close-group")).toHaveLength(4);
- expect(queryAllByTestId("open-group")).toHaveLength(0);
-
fireEvent.click(collapseButton);
await waitFor(() =>
expect(queryAllByTestId("task-instance")).toHaveLength(2)
);
expect(queryAllByTestId("close-group")).toHaveLength(0);
- // Since the groups are nested, only the parent rows is rendered
- expect(queryAllByTestId("open-group")).toHaveLength(2);
fireEvent.click(expandButton);
await waitFor(() =>
expect(queryAllByTestId("task-instance")).toHaveLength(6)
);
- expect(queryAllByTestId("close-group")).toHaveLength(4);
- expect(queryAllByTestId("open-group")).toHaveLength(0);
});
test("Toggling a group does not affect groups with the same label", async () => {
@@ -280,8 +272,6 @@ describe("Test ToggleGroups", () => {
await waitFor(() =>
expect(queryAllByTestId("task-instance")).toHaveLength(6)
);
- expect(queryAllByTestId("close-group")).toHaveLength(4);
- expect(queryAllByTestId("open-group")).toHaveLength(0);
const group2Task1 = getByTestId("group_2.task_1");
@@ -291,8 +281,6 @@ describe("Test ToggleGroups", () => {
await waitFor(() =>
expect(queryAllByTestId("task-instance")).toHaveLength(5)
);
- expect(queryAllByTestId("close-group")).toHaveLength(3);
- expect(queryAllByTestId("open-group")).toHaveLength(1);
});
test("Hovered effect on task state", async () => {
diff --git a/airflow/www/static/js/dag/grid/renderTaskRows.tsx b/airflow/www/static/js/dag/grid/renderTaskRows.tsx
index ab33c0c145..b32c1b4397 100644
--- a/airflow/www/static/js/dag/grid/renderTaskRows.tsx
+++ b/airflow/www/static/js/dag/grid/renderTaskRows.tsx
@@ -24,7 +24,7 @@ import useSelection, { SelectionProps } from "src/dag/useSelection";
import type { Task, DagRun } from "src/types";
import StatusBox, { boxSize, boxSizePx } from "../StatusBox";
-import TaskName from "./TaskName";
+import TaskName from "../TaskName";
const boxPadding = 3;
const boxPaddingPx = `${boxPadding}px`;
@@ -172,7 +172,15 @@ const Row = (props: RowProps) => {
label={task.label || task.id || ""}
id={task.id || ""}
isOpen={isOpen}
- level={level}
+ ml={level * 4 + 4}
+ setupTeardownType={task.setupTeardownType}
+ mr={4}
+ fontWeight={
+ isGroup || (task.isMapped && !isParentMapped)
+ ? "bold"
+ : "normal"
+ }
+ noOfLines={1}
/>
</Td>
)}
diff --git a/airflow/www/static/js/utils/graph.ts b/airflow/www/static/js/utils/graph.ts
index 1f77990930..71003b0f19 100644
--- a/airflow/www/static/js/utils/graph.ts
+++ b/airflow/www/static/js/utils/graph.ts
@@ -132,7 +132,7 @@ const generateGraph = ({
},
label: value.label,
layoutOptions: {
- "elk.padding": "[top=60,left=10,bottom=10,right=10]",
+ "elk.padding": "[top=80,left=15,bottom=15,right=15]",
},
children: children.map(formatChildNode),
edges: filteredEdges
@@ -172,6 +172,8 @@ const generateGraph = ({
}));
closedGroupIds.push(id);
}
+ const extraLabelLength =
+ value.label.length > 20 ? value.label.length - 19 : 0;
return {
id,
label: value.label,
@@ -180,7 +182,8 @@ const generateGraph = ({
isJoinNode,
childCount,
},
- width: isJoinNode ? 10 : 200,
+ // Make tasks with long names wider
+ width: isJoinNode ? 10 : 200 + extraLabelLength * 5,
height: isJoinNode ? 10 : 70,
};
};