pierrejeambrun commented on code in PR #47994:
URL: https://github.com/apache/airflow/pull/47994#discussion_r2020558241
##########
airflow-core/src/airflow/ui/src/layouts/Details/Grid/TaskNames.tsx:
##########
@@ -86,12 +93,20 @@ export const TaskNames = ({ nodes }: Props) => {
}}
>
<TaskName
+ display="-webkit-box"
fontSize="sm"
fontWeight="normal"
isMapped={Boolean(node.is_mapped)}
label={node.label}
+ overflow="hidden"
paddingLeft={node.depth * 3 + 2}
setupTeardownType={node.setup_teardown_type}
+ style={{
+ WebkitBoxOrient: "vertical",
+ WebkitLineClamp: 1,
+ }}
+ title={node.label}
Review Comment:
I don't think title has any effect here ?
##########
airflow-core/src/airflow/ui/src/layouts/Details/Grid/Grid.tsx:
##########
@@ -73,13 +73,21 @@ export const Grid = ({ limit }: Props) => {
[structure?.nodes, openGroupIds],
);
+ const maxTaskNameLength = Math.max.apply(
+ undefined,
+ flatNodes.map((node) => node.label.length),
+ );
+
+ const extraPadding = maxTaskNameLength < 20 ? 30 : 0;
+ const taskNameWidth = maxTaskNameLength > 30 ? "150px" :
`${maxTaskNameLength * 8 + extraPadding}px`;
+
Review Comment:
I'm not a big fan of all that calculation to fix the columns width.
Can we make it relative maybe `20%` of the available pane space. (The left
pane allocated to the grid display).
Just an idea.
##########
airflow-core/src/airflow/ui/src/layouts/Details/Grid/Grid.tsx:
##########
@@ -73,13 +73,21 @@ export const Grid = ({ limit }: Props) => {
[structure?.nodes, openGroupIds],
);
+ const maxTaskNameLength = Math.max.apply(
+ undefined,
+ flatNodes.map((node) => node.label.length),
+ );
+
+ const extraPadding = maxTaskNameLength < 20 ? 30 : 0;
+ const taskNameWidth = maxTaskNameLength > 30 ? "150px" :
`${maxTaskNameLength * 8 + extraPadding}px`;
+
return (
- <Flex justifyContent="flex-end" mr={3} position="relative" pt={50}
width="100%">
- <Box position="absolute" top="150px" width="100%">
+ <Flex justifyContent="flex-start" mr={3} position="relative" pt={50}
width="100%">
+ <Box position="absolute" top="150px" width={taskNameWidth}>
<TaskNames nodes={flatNodes} />
</Box>
<Box>
- <Flex position="relative">
+ <Flex ml={taskNameWidth} position="absolute">
Review Comment:
I don't understand that, this looks hacky, why do we add a gigantic marging
left so the column is not overlapped by the rest of the component and place it
`absolute` ?
##########
airflow-core/src/airflow/ui/src/layouts/Details/Grid/TaskNames.tsx:
##########
@@ -86,12 +93,20 @@ export const TaskNames = ({ nodes }: Props) => {
}}
>
<TaskName
+ display="-webkit-box"
fontSize="sm"
fontWeight="normal"
isMapped={Boolean(node.is_mapped)}
label={node.label}
+ overflow="hidden"
paddingLeft={node.depth * 3 + 2}
setupTeardownType={node.setup_teardown_type}
+ style={{
+ WebkitBoxOrient: "vertical",
+ WebkitLineClamp: 1,
+ }}
Review Comment:
To avoid styling duplication, this could be moved inside the `TaskName`
component.
This way `TaskLink` in the Graph can also ellipsis if too long. (We need
some adjustment there).
##########
airflow-core/src/airflow/ui/src/layouts/Details/Grid/TaskNames.tsx:
##########
@@ -86,12 +93,20 @@ export const TaskNames = ({ nodes }: Props) => {
}}
>
<TaskName
+ display="-webkit-box"
fontSize="sm"
fontWeight="normal"
isMapped={Boolean(node.is_mapped)}
label={node.label}
+ overflow="hidden"
paddingLeft={node.depth * 3 + 2}
setupTeardownType={node.setup_teardown_type}
+ style={{
+ WebkitBoxOrient: "vertical",
+ WebkitLineClamp: 1,
+ }}
+ title={node.label}
Review Comment:
I don't think title has any effect here ? (title)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
