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

New gantt tab #31806

Merged
merged 8 commits into from
Jul 28, 2023
Merged

New gantt tab #31806

merged 8 commits into from
Jul 28, 2023

Conversation

bbovenzi
Copy link
Contributor

@bbovenzi bbovenzi commented Jun 8, 2023

New Gantt Chart in grid new tabs

Closes: #19940

Sync scrolling with grid:
Jun-08-2023 11-47-07

Support task groups and sync with grid:
Jun-08-2023 17-11-01

Sync task/run selection with grid:
Jun-08-2023 17-12-47

To Do:

  • Improve scroll/height calculation.
  • Fix and add tests

^ 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.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborg boring-cyborg bot added area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Jun 8, 2023
@bbovenzi bbovenzi added this to the Airflow 2.7.0 milestone Jun 8, 2023
@potiuk
Copy link
Member

potiuk commented Jun 8, 2023

Whoa. I don't have skills to review it but the screenshots looks like #protm

@eladkal
Copy link
Contributor

eladkal commented Jun 9, 2023

this looks really good!
What happens when task retry and it will show the timeline only for last attempt right?
I wonder if it's doable to allow users choose to see all retries (as requested in #17487 )

@marclamberti
Copy link

my eyes 🤩🤩🤩🤩🤩

@jscheffl
Copy link
Contributor

WOW and only 440 LoC! This smells like PR of the month is coming :-D

@bbovenzi bbovenzi marked this pull request as ready for review July 20, 2023 03:08
@bbovenzi
Copy link
Contributor Author

bbovenzi commented Jul 20, 2023

Ready for review although I'm still working to improve the synced scroll behavior.

@pierrejeambrun pierrejeambrun added the type:new-feature Changelog: New Features label Jul 23, 2023
Copy link
Member

@pierrejeambrun pierrejeambrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! Tested locally and seems nice. 🎉

I just have issues scrolling the gantt. (scroll bars are not working for me), also I have vertical scroll when there is plenty of space vertically to render the Gantt chart. (scrollbar are unresponsive, both with the scroll wheel and by click/drag on it, tested on chrome and firefox on ubuntu)

image

airflow/www/static/js/dag/details/gantt/Chart.tsx Outdated Show resolved Hide resolved
airflow/www/static/js/dag/details/index.tsx Outdated Show resolved Hide resolved
airflow/www/static/js/dag/details/index.tsx Outdated Show resolved Hide resolved
@bbovenzi
Copy link
Contributor Author

bbovenzi commented Jul 24, 2023

Looks great! Tested locally and seems nice. 🎉

I just have issues scrolling the gantt. (scroll bars are not working for me), also I have vertical scroll when there is plenty of space vertically to render the Gantt chart. (scrollbar are unresponsive, both with the scroll wheel and by click/drag on it, tested on chrome and firefox on ubuntu)

Figured out the scroll focus issue on the gantt chart itself. I also change the checkScrollPosition from onSelect to a timeout, so if the scroll of the grid/gantt are not synced, it should rectify itself.

I also updated the tooltip for dynamic/group tasks to better communicate the aggregate duration/start/end.
Screenshot 2023-07-24 at 4 54 12 PM

Copy link
Member

@ashb ashb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. In this case I think we could even remove the original Gantt chart entirely. Happy to make that a separate PR if you want. (Either way, when we delete that we should redirect the old URL to the new react-based one)

@eladkal
Copy link
Contributor

eladkal commented Jul 26, 2023

In this case I think we could even remove the original Gantt chart entirely

+1

@potiuk
Copy link
Member

potiuk commented Jul 26, 2023

In this case I think we could even remove the original Gantt chart entirely

+1

@bbovenzi bbovenzi merged commit 479cd43 into apache:main Jul 28, 2023
@bbovenzi bbovenzi deleted the new-gantt-tab branch July 28, 2023 00:15
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. area:webserver Webserver related Issues type:new-feature Changelog: New Features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a new gantt chart in React
8 participants