-
Notifications
You must be signed in to change notification settings - Fork 362
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 selection label to FlatRuns page (ET-309) #9670
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 #9670 +/- ##
==========================================
- Coverage 53.32% 48.72% -4.61%
==========================================
Files 1254 932 -322
Lines 152657 123488 -29169
Branches 3244 3246 +2
==========================================
- Hits 81403 60166 -21237
+ Misses 71102 63170 -7932
Partials 152 152
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.
few comments, overall LGTM
const allSelectedRunIds = useMemo(() => { | ||
return settings.selection.type === 'ONLY_IN' ? settings.selection.selections : []; | ||
}, [settings.selection]); | ||
|
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.
this can be replaced with selectedRunIdSet
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.
its totally optional, but do you think its a good idea to handle the edge case when selectedCount
> total
?
Ticket: ET-309
Description
Add the selection label to the flat runs table.
Also includes fix for bug with "Select all" action
Test Plan
Checklist
docs/release-notes/
.See Release Note for details.