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/10/25 18:45:31 UTC
[GitHub] [airflow] bbovenzi opened a new pull request, #27273: Make grid view widths adjustable
bbovenzi opened a new pull request, #27273:
URL: https://github.com/apache/airflow/pull/27273
Make the border between the grid and details panel draggable so users can adjust the width as they want.
---
**^ 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 changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+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 newsfragment file, named `{pr_number}.significant.rst` or `{issue_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 pull request #27273: Make grid view widths adjustable
Posted by GitBox <gi...@apache.org>.
bbovenzi commented on PR #27273:
URL: https://github.com/apache/airflow/pull/27273#issuecomment-1292578439
I cleaned up the magic numbers.
I don't think I understand the "jump" you're seeing. A gif would be helpful.
--
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] potiuk commented on pull request #27273: Make grid view widths adjustable
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #27273:
URL: https://github.com/apache/airflow/pull/27273#issuecomment-1292607645
Making Airflow UI better one piece at-a-time :)
--
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 pull request #27273: Make grid view widths adjustable
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on PR #27273:
URL: https://github.com/apache/airflow/pull/27273#issuecomment-1292477728
Tested locally and working great.
I just have one issue when displaying details for a task group with long 'tooltip'. There is a weird 'jump' when trying to resize at first.
Something like this:
And then on clicking on the separator for a rezise, it flickers, and the separator jumps to the left right away:
This seems ike something that could be just on my side and cause by my local setup:
- Version 103.0.5060.134 (Official Build) (64-bit)
- Ubuntu 20.04.4 LTS
--
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 #27273: Make grid view widths adjustable
Posted by GitBox <gi...@apache.org>.
bbovenzi merged PR #27273:
URL: https://github.com/apache/airflow/pull/27273
--
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 #27273: Make grid view widths adjustable
Posted by GitBox <gi...@apache.org>.
pierrejeambrun commented on code in PR #27273:
URL: https://github.com/apache/airflow/pull/27273#discussion_r1004923616
##########
airflow/www/static/js/dag/Main.tsx:
##########
@@ -65,24 +70,83 @@ const Main = () => {
onToggle();
};
+ useEffect(() => {
+ if (contentRef.current) {
+ const topOffset = contentRef.current.offsetTop;
+ const footerHeight = parseInt(getComputedStyle(document.getElementsByTagName('body')[0]).paddingBottom.replace('px', '').replace('em', ''), 10) || 0;
Review Comment:
This feels weird to replace indifferently `px` and `em`. I think footerHeight won't have the correct value if it is specified in 'em'. (We use it as a pixel value later in the code).
##########
airflow/www/static/js/dag/Main.tsx:
##########
@@ -65,24 +70,83 @@ const Main = () => {
onToggle();
};
+ useEffect(() => {
+ if (contentRef.current) {
+ const topOffset = contentRef.current.offsetTop;
+ const footerHeight = parseInt(getComputedStyle(document.getElementsByTagName('body')[0]).paddingBottom.replace('px', '').replace('em', ''), 10) || 0;
+ contentRef.current.style.height = `${window.innerHeight - topOffset - footerHeight}px`;
+ }
+ }, []);
+
+ const resize = useCallback((e: MouseEvent) => {
+ const gridEl = gridRef.current;
+ if (gridEl && e.x > 350 && e.x < window.innerWidth - 400) {
+ gridEl.style.width = `${e.x}px`;
+ }
+ }, [gridRef]);
+
+ useEffect(() => {
+ const resizeEl = resizeRef.current;
+ if (resizeEl) {
+ resizeEl.addEventListener('mousedown', (e) => {
+ e.preventDefault();
+ document.addEventListener('mousemove', resize);
+ });
+
+ document.addEventListener('mouseup', () => {
+ document.removeEventListener('mousemove', resize);
+ });
+
+ return () => {
+ resizeEl?.removeEventListener('mousedown', resize);
+ document.removeEventListener('mouseup', resize);
+ };
+ }
+ return () => {};
+ }, [resize, isLoading]);
+
return (
- <Box>
+ <Box flex={1}>
<FilterBar />
<LegendRow onStatusHover={onStatusHover} onStatusLeave={onStatusLeave} />
<Divider mb={5} borderBottomWidth={2} />
- <Flex justifyContent="space-between">
+ <Flex ref={contentRef}>
{isLoading || isEmpty(groups)
? (<Spinner />)
: (
<>
- <Grid
- isPanelOpen={isOpen}
- onPanelToggle={onPanelToggle}
- hoveredTaskState={hoveredTaskState}
- />
- <Box borderLeftWidth={isOpen ? 1 : 0} position="relative">
- {isOpen && (<Details />)}
+ <Box
+ minWidth="350px"
+ flex={isOpen ? undefined : 1}
+ ref={gridRef}
+ height="100%"
+ >
+ <Grid
+ isPanelOpen={isOpen}
+ onPanelToggle={onPanelToggle}
+ hoveredTaskState={hoveredTaskState}
+ />
</Box>
+ {isOpen && (
+ <>
+ <Box
+ width={2}
+ cursor="ew-resize"
+ bg="gray.200"
+ ref={resizeRef}
+ zIndex={1}
+ />
+ <Box
+ flexGrow={1}
+ minWidth="400px"
Review Comment:
This seems to be the same 400 and 350 px than above. (I'm not sure)
##########
airflow/www/static/js/dag/grid/index.tsx:
##########
@@ -122,9 +125,9 @@ const Grid = ({ isPanelOpen = false, onPanelToggle, hoveredTaskState }: Props) =
/>
</Flex>
<Box
- overflow="auto"
+ height={`calc(100% - ${dimensions?.borderBox.height || 36}px)`}
Review Comment:
What does this 36 px correspond to ?
##########
airflow/www/static/js/dag/Main.tsx:
##########
@@ -65,24 +70,83 @@ const Main = () => {
onToggle();
};
+ useEffect(() => {
+ if (contentRef.current) {
+ const topOffset = contentRef.current.offsetTop;
+ const footerHeight = parseInt(getComputedStyle(document.getElementsByTagName('body')[0]).paddingBottom.replace('px', '').replace('em', ''), 10) || 0;
+ contentRef.current.style.height = `${window.innerHeight - topOffset - footerHeight}px`;
+ }
+ }, []);
+
+ const resize = useCallback((e: MouseEvent) => {
+ const gridEl = gridRef.current;
+ if (gridEl && e.x > 350 && e.x < window.innerWidth - 400) {
Review Comment:
this 350 and 400, can they be extracted into variable, at least they have an explicit name. (And can be later reused)
--
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] potiuk commented on pull request #27273: Make grid view widths adjustable
Posted by GitBox <gi...@apache.org>.
potiuk commented on PR #27273:
URL: https://github.com/apache/airflow/pull/27273#issuecomment-1291353652
❤️
--
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