This is an automated email from the ASF dual-hosted git repository. potiuk pushed a commit to branch v2-8-test in repository https://gitbox.apache.org/repos/asf/airflow.git
commit f9e208d6d84cc46788f25129c4aeabe1ef4ef2e1 Author: Brent Bovenzi <[email protected]> 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, }; };
