-
Notifications
You must be signed in to change notification settings - Fork 16.4k
Rearrange Dag details view #46939
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
Rearrange Dag details view #46939
Conversation
jscheffl
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.
I tried the same dag and could replicate. But this is a bug on main, not due to changes to this branch. |
jscheffl
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.
Approve - under the assumption that you fix the three dots :-D
|
Thanks for bringing back the grid view. I tried the PR locally. Couple of suggestions from my end.
|
|
|
Thanks @bbovenzi . |
Did you play with resizing the panels? When this is merged. I think we'll need to fix our duration charts to stay a consistent size. |
Yes, the x axis names didn't fit the chart and the chart was also compressed on smaller spaces not being very responsive. IIRC initially there was design to have 3 charts as a grid with remaining 2 charts requiring API for grouping and other idea to have a calendar/heatmap view of the task durations. Maybe on resize they can become a chart per row or to have the duration chart as the only chart for now till the other charts are finalized. |
How about reducing the line height on the breadcrumb label: (I also noticed that the height of the Task one doesn't match the DAG and Dag Run crumb) |
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.
Grid data should be invalidated on actions (clear, mark as etc...) because for now the grid stay identical until we hard refresh after clearing for instance. Or even better auto refreshed.
I can't clear a task instance (error 500 but do not seem related to this PR, task sdk issue i think

Mark run as is missing the initial tasks table. (The initial dry-run call is not being send)

|
|
Looks good. I think lets not sweat the small details too much in this PR; lets get this in for the first Beta as it's such a massive UX improvement over what is there right now! |
Agreed. I think its important to get this major change in. We'll get more feedback and can keep improving it. I don't want to leave this open for a week because I'm fiddling with css details here and there. |
* Rearrange Dag details view * Address PR feedback
* Rearrange Dag details view * Address PR feedback
* Rearrange Dag details view * Address PR feedback












Problem:
Solution:
default_viewvalue and then persists the panel width and view selection in localStorage by dagIdOther features:
To Do in other PRs:
^ 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 newsfragments.