Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unnecessary scrollbars in grid view #29100

Closed
1 of 2 tasks
tanelk opened this issue Jan 23, 2023 · 5 comments · Fixed by #29367
Closed
1 of 2 tasks

Unnecessary scrollbars in grid view #29100

tanelk opened this issue Jan 23, 2023 · 5 comments · Fixed by #29367
Assignees
Labels
area:UI Related to UI/UX. For Frontend Developers. kind:bug This is a clearly a bug
Milestone

Comments

@tanelk
Copy link
Contributor

tanelk commented Jan 23, 2023

Apache Airflow version

2.5.0

What happened

Compare the same DAG grid view in 2.4.3: (everything is scrolled using the "main" scrollbar of the window)
image
and in 2.5.0 (and 2.5.1) (left and right side of the grid have their own scrollbars):
image

It was much more ergonomic previously when only the main scrollbar was used.

I think the relevant change was in #27560, where maxHeight={offsetHeight} was added to some places.

Is this the intended way the grid view should look like or did happen as an accident?

I tried to look around in the developer tools and it seems like removing the max-height from this element restores the old look: div#react-container div div.c-1rr4qq7 div.c-k008qs div.c-19srwsc div.c-scptso div.c-l7cpmp. Well it does for the left side of the grid view. Similar change has to be done for some other divs also.

image

What you think should happen instead

No response

How to reproduce

No response

Operating System

Linux

Versions of Apache Airflow Providers

No response

Deployment

Virtualenv installation

Deployment details

No response

Anything else

No response

Are you willing to submit PR?

  • Yes I am willing to submit a PR!

Code of Conduct

@tanelk tanelk added area:core kind:bug This is a clearly a bug labels Jan 23, 2023
@tanelk
Copy link
Contributor Author

tanelk commented Jan 23, 2023

@bbovenzi I think you did this change - I'd like to argue, that it was better before. But perhaps I do not understand others usecases.

@bbovenzi bbovenzi added area:UI Related to UI/UX. For Frontend Developers. and removed area:core labels Jan 23, 2023
@bbovenzi
Copy link
Contributor

I've been back and forth on the best scrolling solution for the grid view because the right or left side could be significantly longer than the other. This was most notable with logs. Scrolling the entire window would mean half the page would just be blank and you'd lose some context.
I'd also like to make progress on cleaning up a lot of the menus above the grid view, so it all fits better in a typical window height.

@JoepvandenHoven-Bluemine
Copy link

JoepvandenHoven-Bluemine commented Jan 25, 2023

Regardless of the best solution it seems to have slipped in in a pull request that was supposed to make the grid width adjustable and also "fix" the logs height (i.e. made it depend on the screen height, which is arguably better than just using a fixed height). I think the change of the grid height, if any is indeed necessary, shouldn't have been part of the same pull request as well.

For what it's worth I find the current solution hard to use, it makes it harder to see the status of the dag on the main status page. It's especially bad on small screens, since the max-height for the logs and the grid-view are set separately the grid view can become a lot smaller than the log view next to it. It makes it really hard to see what's going on.

@bbovenzi
Copy link
Contributor

Very fair point. I'll play around with it again for 2.5.2

@bbovenzi bbovenzi self-assigned this Jan 25, 2023
@iuga
Copy link

iuga commented Jan 30, 2023

My 2 cents:
As strong users of Airflow, we often have complex DAGs with thousands of tasks, and the Grid View has been a crucial tool for us to manage and optimize how we consume information. In 2.4.x, we were able to take advantage of the scroll bar to maximize the information displayed on the screen. However, with the changes in 2.5.x we are only being able to view around 9 rows at a time, and it significantly restricts our ability to quickly access the information we need. Hope our use case can provide you more insights to improve the scrolling solution:

2.4.x
2 4 x

2.5.x
2 5 x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:UI Related to UI/UX. For Frontend Developers. kind:bug This is a clearly a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants