[GitHub] [airflow] pierrejeambrun commented on a diff in pull request #27273: Make grid view widths adjustable

[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