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/03/09 22:42:37 UTC

[GitHub] [airflow] bbovenzi opened a new pull request #22134: Add vertical scrolling to grid view

bbovenzi opened a new pull request #22134:
URL: https://github.com/apache/airflow/pull/22134


   After a few dozen tasks, the grid becomes too long to be able to see a task and the dag runs at the same time. This PR allows the table body the scroll vertically while keeping the table head fixed.
   
   Note: Right now, it is a little hacky to determine the max height to set for a table before scrolling kicks in. I am open to suggestions.
   
   ![Mar-09-2022 17-37-52](https://user-images.githubusercontent.com/4600967/157550633-1ace554f-72aa-47c1-8fc0-f77d1b03dd06.gif)
   
   
   ---
   **^ 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 [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
   


-- 
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] github-actions[bot] commented on pull request #22134: Add vertical scrolling to grid view

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #22134:
URL: https://github.com/apache/airflow/pull/22134#issuecomment-1066975645


   The PR is likely OK to be merged with just subset of tests for default Python and Database versions without running the full matrix of tests, because it does not modify the core of Airflow. If the committers decide that the full tests matrix is needed, they will add the label 'full tests needed'. Then you should rebase to the latest main or amend the last commit of the PR, and push it with --force-with-lease.


-- 
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 #22134: Add vertical scrolling to grid view

Posted by GitBox <gi...@apache.org>.
bbovenzi commented on pull request #22134:
URL: https://github.com/apache/airflow/pull/22134#issuecomment-1064146319


   > Would it be possible to get some gifs on non-Mac platforms? From experience scroll bar manipulation can have different behaviours depending on whether they are persistent; macOS hides them by default resulting in the content and wrapper have different sizes than say Windows. This sometimes results in wonky UI like double scrollbars and scrolling focus issues.
   
   I added a gif for Windows+Edge.


-- 
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] uranusjr commented on pull request #22134: Add vertical scrolling to grid view

Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #22134:
URL: https://github.com/apache/airflow/pull/22134#issuecomment-1063761625


   Would it be possible to get some gifs on non-Mac platforms? From experience scroll bar manipulation can have different behaviours depending on whether they are persistent (macOS hides them by default resulting in the content and wrapper have different sizes than say Windows).


-- 
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] ashb commented on a change in pull request #22134: Add vertical scrolling to grid view

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #22134:
URL: https://github.com/apache/airflow/pull/22134#discussion_r824112769



##########
File path: airflow/www/static/js/tree/renderTaskRows.jsx
##########
@@ -160,7 +172,7 @@ const Row = (props) => {
           </Collapse>
         </Td>
         <Td width={0} p={0} borderBottom={0} />
-        <Td p={0} align="right" _groupHover={{ backgroundColor: 'rgba(113, 128, 150, 0.1)' }} transition="background-color 0.2s" borderBottom={0}>
+        <Td p={0} align="right" _groupHover={{ backgroundColor: 'rgba(113, 128, 150, 0.1)' }} transition="background-color 0.2s" borderBottom={0} width={`${dagRunIds.length * 16}px`}>

Review comment:
       Where does the 16px per dagRun come from? Is there anyway to not have this hardcoded here but somehow worked out based on what the Column tells us?
   
   Secondly we have this in `16px` two places.
   
   Thirdly: on L111 we have 18px wide box for empty TI -- shouldn't that be the same width? (Not 100% sure)




-- 
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] uranusjr edited a comment on pull request #22134: Add vertical scrolling to grid view

Posted by GitBox <gi...@apache.org>.
uranusjr edited a comment on pull request #22134:
URL: https://github.com/apache/airflow/pull/22134#issuecomment-1063761625


   Would it be possible to get some gifs on non-Mac platforms? From experience scroll bar manipulation can have different behaviours depending on whether they are persistent; macOS hides them by default resulting in the content and wrapper have different sizes than say Windows. This sometimes results in wonky UI like double scrollbars and scrolling focus issues.


-- 
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 change in pull request #22134: Add vertical scrolling to grid view

Posted by GitBox <gi...@apache.org>.
bbovenzi commented on a change in pull request #22134:
URL: https://github.com/apache/airflow/pull/22134#discussion_r824995781



##########
File path: airflow/www/static/js/tree/renderTaskRows.jsx
##########
@@ -160,7 +172,7 @@ const Row = (props) => {
           </Collapse>
         </Td>
         <Td width={0} p={0} borderBottom={0} />
-        <Td p={0} align="right" _groupHover={{ backgroundColor: 'rgba(113, 128, 150, 0.1)' }} transition="background-color 0.2s" borderBottom={0}>
+        <Td p={0} align="right" _groupHover={{ backgroundColor: 'rgba(113, 128, 150, 0.1)' }} transition="background-color 0.2s" borderBottom={0} width={`${dagRunIds.length * 16}px`}>

Review comment:
       Yes, I just updated statusbox and taskrows to share a single value + padding for the box width and overall column width




-- 
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 #22134: Add vertical scrolling to grid view

Posted by GitBox <gi...@apache.org>.
bbovenzi merged pull request #22134:
URL: https://github.com/apache/airflow/pull/22134


   


-- 
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