-
Notifications
You must be signed in to change notification settings - Fork 357
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
feat: add heatmap to runs table [ET-230] #9429
Conversation
✅ Deploy Preview for determined-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9429 +/- ##
==========================================
- Coverage 48.61% 43.38% -5.23%
==========================================
Files 1233 909 -324
Lines 158972 119200 -39772
Branches 2778 2778
==========================================
- Hits 77278 51718 -25560
+ Misses 81520 67308 -14212
Partials 174 174
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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.
Looks like this is missing the 'Apply'/'Cancel' heatmap option in header menus. Other than that and the tooltip issue, everything else looks good.
This is an pre-existing issue, it should be fixed by upgrading to antd 5.3.0. We're currently using 5.1.7. I support updating the library but it's out of scope for this PR. |
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 might wrap the Button
in a Tooltip
like we do in TableActionBar
but I see the argument for using the tooltip
prop and fixing the issue with it separately. everything else LGTM
Ticket
ET-230
Description
Adds heatmap to Flat Runs table
Test Plan
Checklist
docs/release-notes/
.See Release Note for details.