Skip to content

Conversation

@KatalKavya96
Copy link
Contributor

PR Description

This PR improves the grid view task instance tooltips to display full task instance details, similar to the tooltip shown in the DAG run details page.

Changes Made

GridTI.tsx

  • Integrated TaskInstanceTooltip directly into grid view task instance ticks.

  • Ensures hover on grid ticks shows the same rich details as the task success/failure badges.

  • Cleaned up unused state handling and simplified hydration logic.

TaskInstanceTooltip.tsx

  • Updated to handle additional task instance fields consistently.

  • Fixed eslint/type issues and standardised object key ordering.

  • Improved null/undefined handling for optional fields.

TaskInstancesColumn.tsx

  • Updated to pass richer instance data down to GridTI.

DurationTick.tsx

  • Minor adjustments for clarity in tick labels.

Why

  • Previously, tooltips in grid view only showed minimal info (state + basic metadata). This update ensures users can inspect full task instance details directly from grid ticks, improving usability and parity across views.

Testing

  • Verified locally in Breeze UI:

  • Hovering grid ticks now shows all task instance fields.

  • Tooltips match those in DAG run success/failure badges.

  • UI tests (pnpm test) pass.

#Demo Video

Screen.Recording.2025-09-21.at.2.30.37.AM.mov

#Related

related: #55900

@boring-cyborg boring-cyborg bot added the area:UI Related to UI/UX. For Frontend Developers. label Sep 20, 2025
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.

I don't think the grid should fetch TI details.

If we need more information the details panel is for that purpose.

The grid summary endpoint sends back min_start_date and max_end_date. It's only populated for groups, but we could rename it a reuse it for start_date and end_date and always populate it. So we have access to that directly. 'duration' and 'trigger rule' I think we can remove for now.

@RoyLee1224 RoyLee1224 mentioned this pull request Sep 30, 2025
19 tasks
@r-richmond
Copy link
Contributor

Just wanted to say having duration be back in the grid tooltip is a very welcome/desired changed from an end user perspective.

@bbovenzi
Copy link
Contributor

bbovenzi commented Oct 1, 2025

I think the best approach is to see what we can easily add to the LightGridTaskInstanceSummary without affecting API performance.

@RoyLee1224
Copy link
Contributor

RoyLee1224 commented Oct 3, 2025

@KatalKavya96 Here's what the previous grid legend looks like #53438

@RoyLee1224
Copy link
Contributor

RoyLee1224 commented Oct 4, 2025

@KatalKavya96 Let's try to rebase and use the new renderDuration from #56310, that would be great!

@KatalKavya96
Copy link
Contributor Author

KatalKavya96 commented Oct 5, 2025

@RoyLee1224 I have applied changes to use renderDuration,Please review it once again. Thanks

@KatalKavya96 KatalKavya96 requested a review from bbovenzi October 10, 2025 00:11
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.

Code is not super elegant. It's hard to read and there is a lot of unnecessary stuff.

I assume it was done with help of AI tools and could use some refinement.

Also for the backend you probably need to update tests accordingly, it looks like we want to add 'duration' to the GridTISummaries datamodel but the model was not updated so it's most likely ignored in the response. Can you add new screenshots of what it looks like too please.

CI need fixing, there are typing errors.

Comment on lines +98 to +99
((selectedTaskId ?? undefined) !== undefined && selectedTaskId === taskId) ||
((selectedGroupId ?? undefined) !== undefined && selectedGroupId === taskId);
Copy link
Member

Choose a reason for hiding this comment

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

(selectedTaskId ?? undefined) that's not necessary I believe.

Co-authored-by: Pierre Jeambrun <pierrejbrun@gmail.com>
Co-authored-by: Pierre Jeambrun <pierrejbrun@gmail.com>
@KatalKavya96
Copy link
Contributor Author

KatalKavya96 commented Oct 16, 2025

@pierrejeambrun This is what it looks like on UI right now
image

But i made a few changes as the Grid Icon fetch was giving Internal Server Error

Copy link
Contributor

@bbovenzi bbovenzi left a comment

Choose a reason for hiding this comment

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

Leaving a Request Changes on here until all unnecessary code changes are reverted.

@RoyLee1224
Copy link
Contributor

@KatalKavya96 are you still working on this?
If not, I’d be happy to take it over. 💪

@KatalKavya96
Copy link
Contributor Author

KatalKavya96 commented Nov 13, 2025

@RoyLee1224 Sure!, You can take this PR further up as i am currently equipped with few other things so it would be better if you could resolve this

@pierrejeambrun
Copy link
Member

Closing as @RoyLee1224 is working on that on different PRs and we don't plan to continue the work on this PR.

Thanks for your effort @KatalKavya96, feel free to re-open if you plan to work on this again.

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants