-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
Migrate inline JavaScript scripting to external files #14115
Comments
I can take this one, if that's cool. |
@bbovenzi that would be great—assigning to you! |
Thanks @bbovenzi for your interest! If you would like to proceed, it would be great to share how you would plan to make this change, and potential issues you can foresee. I believe @ryanahamilton , our UI guru, already has some brief ideas about these, so it would be great to discuss together. |
Ha, I just see you guys are from the same team. This issue is in good hands👍 Tip-top! |
…5257) This is a collection of related updates to improve the overall user experience of the Graph view. I tested these updates across varying DAG shapes/sizes including Task Groups. This testing also covered the hovering/clicking of the task status legend and the search filter functionality. ### General - Simplifies JavaScript by removing the redundant inline styling for highlighting tasks and paths between them. This is accomplished by pairing a `data-highlight` attribute with CSS keyed off of it. - I tried not to go overboard (but couldn't help myself in a few places) on the syntax cleanup as that will be handled in the migration to an external JavaScript file for #14115. - Changes the "edge" (lines between tasks) curve to be smooth instead of angled straight lines. Single, smooth lines are easier for eyes to follow the path of than multiple jointed lines. This was accomplished by utilizing the `d3-shape` library. - Slightly increases the vertical node separation (`nodeSep`) to make the graphs feel a little less crowded. This also reduces the amount that the task nodes overlap with the edges. - Fixed an unreported bug when you hover or click on "no_status" in the status legend at the top, the correlating tasks would not highlight. - Updated the Graph UI screenshot in the docs to reflect the changes in this PR. ### Task and Path highlighting - Removes the varying stroke (border) weights of tasks when highlighting/hovering. This interaction isn't necessary given the non-highlighted tasks fade out. It (subjectively) makes for a smoother transition for only the stroke color to change instead of the weight as well. - Improves the styling of the "path highlighting". Instead of styling the downstream, hovered, and upstream tasks with 3 different border colors, I've given them all the same "highlighted" color and have also given the edges that connect them a similar styling. This simplifies the pattern and visually highlights the actual path between the tasks. The upstream and downstream directions can still easily be deciphered by the edge arrows. ### Fixes tooltip jank - Prevents the jumping of the tooltip position that occurs when moving your cursor within the task node - Positions the tooltip slightly higher so it no longer overlaps with the task node's border - Adds a very slight opacity to the tooltip background since it can cover up relevant paths between the hovered task node
Description
Currently we have inline JavaScript scripting in templates like
airflow/www/templates/airflow/[dags/dag/gantt/tree/graph].html
etc. In these scripting, some practices are not optimal, like unused arguments, usage of undeclared variables, style issues, etc.Use case / motivation
As briefly discussed in #14019 (comment), it's better to migrate these inline JavaScript scripting to external files.
The text was updated successfully, but these errors were encountered: