refactor: columnpicker remove hard coded value #9342
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9342 +/- ##
==========================================
- Coverage 45.14% 38.56% -6.59%
==========================================
Files 1230 906 -324
Lines 154523 114686 -39837
Branches 2405 2404 -1
==========================================
- Hits 69767 44234 -25533
+ Misses 84561 70257 -14304
Partials 195 195
Flags with carried forward coverage won't be shown. Click here to find out more.
|
✅ Deploy Preview for determined-ui ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
| @@ -284,7 +280,7 @@ const ColumnPickerMenu: React.FC<ColumnMenuProps> = ({ | |||
| ), | |||
| forceRender: true, | |||
| key: canonicalTab, | |||
| label: locationLabelMap[canonicalTab], | |||
| label: locationLabelMap[canonicalTab as keyof typeof locationLabelMap], | |||
| }; | |||
| })} | |||
| /> | |||
There was a problem hiding this comment.
tbh i'm not really sure why we have a tabs.length === 1 down two lines. are we planning on using that condition later?
There was a problem hiding this comment.
the Searches view is a case where we only display columns with location EXPERIMENT, so we don't need the Pivot component and can just display one ColumnPickerTab instead.
There was a problem hiding this comment.
do we ever do that anywhere?
There was a problem hiding this comment.
looks like there's only one reference right now
Ticket
Description
I saw a hardcoded value used in two different places. Here's the location where it's already being defined and used.
https://github.com/determined-ai/determined/blob/main/webui/react/src/pages/F_ExpList/F_ExperimentList.tsx#L1073-L1077
Test Plan
Make sure the column picker looks the same as before
Checklist
docs/release-notes/.See Release Note for details.