-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Optimize Grid View for low-latency hover effect and keyboard navigation #58642
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
Conversation
bbovenzi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. We were already checking the element ids instead in order to speed it up. We should just get rid of all the individual functions.
airflow-core/src/airflow/ui/src/hooks/navigation/useNavigation.ts
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/ui/src/hooks/navigation/useKeyboardNavigation.ts
Outdated
Show resolved
Hide resolved
559b8f8 to
ad91092
Compare
pierrejeambrun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice improvement! 🎉
Looking good and working as expected.
As the grid view is a quite complex and important view and this represent a substantial code change, I would be in favor of marking this for 3.1.5 or maybe even 3.2.0. 3.1.4 is being cut out at the moment.
airflow-core/src/airflow/ui/src/layouts/Details/Grid/GridTI.tsx
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/ui/src/layouts/Details/Grid/GridOverlays.tsx
Outdated
Show resolved
Hide resolved
airflow-core/src/airflow/ui/src/layouts/Details/DetailsLayout.tsx
Outdated
Show resolved
Hide resolved
2f585c0 to
bd52016
Compare
|
Thanks for the review, @bbovenzi. I'll apply these fixes as soon as I finish my final exam.🚀 |
|
We just merged some big changes to the grid view. So this will need a significant rebase. |
|
That's a nice improvement I'd love for it to get merged! |
CleanShot.2026-01-27.at.23.32.56.mp4The hover performance is now quite acceptable thanks to the virtualization in #60241. While the overlay method is faster, it adds extra complexity (like the panel width issue @bbovenzi mentioned). I think the trade-off isn't worth it for now. I’ll close this PR and follow up with a more focused one for keyboard navigation performance (long press). Thanks! |



Key Changes:
GridOverlays.tsxto handle highlighting via direct DOM manipulation (CSS transforms), bypassing React re-renders.onMouseEnterlisteners inGridTIandTaskNameswith a singleonMouseMovehandler in the parentGridcontainer.useNavigationto provide instant visual feedback onkeydownvia refs, committing the URL change only onkeyup. This significantly optimizes long-press keyboard navigation, reducing traversal time for 13 cells from ~8s to ~3s.useHovercontext and expensivedocument.querySelectorAllcalls from child components.TO-DO
Screenshots
Hover Effect - Before
hover_before.mp4
Hover Effect - After
hover_after.mp4
Keyboard Navigation - Before (8s)
navigation_before.mp4
Keyboard Navigation - After (3s)
navigation_after.mp4
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in airflow-core/newsfragments.