You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/06/27 19:22:18 UTC
[GitHub] [airflow] bbovenzi opened a new pull request, #24684: Fix Grid vertical scrolling
bbovenzi opened a new pull request, #24684:
URL: https://github.com/apache/airflow/pull/24684
We had 2 issues with grid scrolling for large DAGs:
1. Grid actions (expand groups, close details panel, auto refresh) would get cut off or overlap with the grid
2. Sticky positioning wasn't working right and the dag run bar chart could be lost.
Before:
After:
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragement file, named `{pr_number}.significant.rst`, in [newsfragments](https://github.com/apache/airflow/tree/main/newsfragments).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bbovenzi commented on a diff in pull request #24684: Fix Grid vertical scrolling
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on code in PR #24684:
URL: https://github.com/apache/airflow/pull/24684#discussion_r907798759
##########
airflow/www/static/js/grid/dagRuns/index.tsx:
##########
@@ -54,58 +63,45 @@ const DagRuns = () => {
// calculate dag run bar heights relative to max
const max = Math.max.apply(null, durations);
- const tickWidth = `${runs.length * 16}px`;
return (
- <Tr
- borderBottomWidth={3}
- position="relative"
- >
- <Td
- height="155px"
- p={0}
- position="sticky"
- left={0}
- backgroundColor="white"
- zIndex={2}
- borderBottom={0}
- width="100%"
- >
- {!!runs.length && (
- <>
- <DurationTick bottom="120px">Duration</DurationTick>
- <DurationTick bottom="96px">
- {formatDuration(max)}
- </DurationTick>
- <DurationTick bottom="46px">
- {formatDuration(max / 2)}
- </DurationTick>
- <DurationTick bottom={0}>
- 00:00:00
- </DurationTick>
- </>
- )}
- </Td>
- <Td p={0} borderBottom={0}>
- <Box position="absolute" bottom="100px" borderBottomWidth={1} zIndex={0} opacity={0.7} width={tickWidth} />
- <Box position="absolute" bottom="50px" borderBottomWidth={1} zIndex={0} opacity={0.7} width={tickWidth} />
- <Box position="absolute" bottom="4px" borderBottomWidth={1} zIndex={0} opacity={0.7} width={tickWidth} />
- </Td>
- <Td p={0} align="right" verticalAlign="bottom" borderBottom={0} width={`${runs.length * 16}px`}>
- <Flex justifyContent="flex-end">
- {runs.map((run: DagRun, i: number) => (
+ <Tr>
+ <Th left={0} zIndex={2}>
+ <Box borderBottomWidth={3} position="relative" height="100%" width="100%">
+ {!!runs.length && (
+ <>
+ <DurationTick bottom="120px">Duration</DurationTick>
+ <DurationTick bottom="96px">
+ {formatDuration(max)}
+ </DurationTick>
+ <DurationTick bottom="46px">
+ {formatDuration(max / 2)}
+ </DurationTick>
+ <DurationTick bottom={0}>
+ 00:00:00
+ </DurationTick>
+ </>
+ )}
+ </Box>
+ </Th>
+ <Th align="right" verticalAlign="bottom">
+ <Flex justifyContent="flex-end" borderBottomWidth={3} position="relative">
+ {runs.map((run: RunWithDuration, index) => (
<DagRunBar
key={run.runId}
run={run}
- max={max}
- index={i}
+ index={index}
totalRuns={runs.length}
+ max={max}
isSelected={run.runId === selected.runId}
onSelect={onSelect}
/>
))}
+ <Box position="absolute" bottom="100px" borderBottomWidth={1} zIndex={0} opacity={0.7} width="100%" />
Review Comment:
Good call. Updated.
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] bbovenzi merged pull request #24684: Fix Grid vertical scrolling
Posted by GitBox <gi...@apache.org>.
bbovenzi merged PR #24684:
URL: https://github.com/apache/airflow/pull/24684
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] pierrejeambrun commented on a diff in pull request #24684: Fix Grid vertical scrolling
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on code in PR #24684:
URL: https://github.com/apache/airflow/pull/24684#discussion_r907766849
##########
airflow/www/static/js/grid/dagRuns/index.tsx:
##########
@@ -54,58 +63,45 @@ const DagRuns = () => {
// calculate dag run bar heights relative to max
const max = Math.max.apply(null, durations);
- const tickWidth = `${runs.length * 16}px`;
return (
- <Tr
- borderBottomWidth={3}
- position="relative"
- >
- <Td
- height="155px"
- p={0}
- position="sticky"
- left={0}
- backgroundColor="white"
- zIndex={2}
- borderBottom={0}
- width="100%"
- >
- {!!runs.length && (
- <>
- <DurationTick bottom="120px">Duration</DurationTick>
- <DurationTick bottom="96px">
- {formatDuration(max)}
- </DurationTick>
- <DurationTick bottom="46px">
- {formatDuration(max / 2)}
- </DurationTick>
- <DurationTick bottom={0}>
- 00:00:00
- </DurationTick>
- </>
- )}
- </Td>
- <Td p={0} borderBottom={0}>
- <Box position="absolute" bottom="100px" borderBottomWidth={1} zIndex={0} opacity={0.7} width={tickWidth} />
- <Box position="absolute" bottom="50px" borderBottomWidth={1} zIndex={0} opacity={0.7} width={tickWidth} />
- <Box position="absolute" bottom="4px" borderBottomWidth={1} zIndex={0} opacity={0.7} width={tickWidth} />
- </Td>
- <Td p={0} align="right" verticalAlign="bottom" borderBottom={0} width={`${runs.length * 16}px`}>
- <Flex justifyContent="flex-end">
- {runs.map((run: DagRun, i: number) => (
+ <Tr>
+ <Th left={0} zIndex={2}>
+ <Box borderBottomWidth={3} position="relative" height="100%" width="100%">
+ {!!runs.length && (
+ <>
+ <DurationTick bottom="120px">Duration</DurationTick>
+ <DurationTick bottom="96px">
+ {formatDuration(max)}
+ </DurationTick>
+ <DurationTick bottom="46px">
+ {formatDuration(max / 2)}
+ </DurationTick>
+ <DurationTick bottom={0}>
+ 00:00:00
+ </DurationTick>
+ </>
+ )}
+ </Box>
+ </Th>
+ <Th align="right" verticalAlign="bottom">
+ <Flex justifyContent="flex-end" borderBottomWidth={3} position="relative">
+ {runs.map((run: RunWithDuration, index) => (
<DagRunBar
key={run.runId}
run={run}
- max={max}
- index={i}
+ index={index}
totalRuns={runs.length}
+ max={max}
isSelected={run.runId === selected.runId}
onSelect={onSelect}
/>
))}
+ <Box position="absolute" bottom="100px" borderBottomWidth={1} zIndex={0} opacity={0.7} width="100%" />
Review Comment:
Should we factorize the style with a local component to avoid repetition ? (Same way we did with `Th`).
--
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: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org