feat: add a tooltip to clarify metric_name in the DatasetEditor #24348
Conversation
Codecov Report
@@ Coverage Diff @@
## master #24348 +/- ##
==========================================
- Coverage 69.08% 69.04% -0.04%
==========================================
Files 1905 1903 -2
Lines 74813 74538 -275
Branches 8107 8110 +3
==========================================
- Hits 51686 51468 -218
+ Misses 21016 20958 -58
- Partials 2111 2112 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 6 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| @@ -222,6 +226,11 @@ export default class CRUDCollection extends React.PureComponent< | |||
| return label; | |||
| } | |||
|
|
|||
| getTooltip(col: any) { | |||
There was a problem hiding this comment.
can we put a type for col here too? It looks like in line 232 it's defined as a string.
There was a problem hiding this comment.
| getTooltip(col: any) { | |
| getTooltip(col: string) { |
| renderTH(col: string, sortColumns: Array<string>) { | ||
| const tooltip = this.getTooltip(col); | ||
| return ( | ||
| <th key={col} className="no-wrap"> |
There was a problem hiding this comment.
no-wrap is the real mvp I snuck into this PR
There was a problem hiding this comment.
fixing this monstruosity:
mistercrunch
added
🏷️ bot
SUMMARY
Adding this tooltip that clarifies the usage of the Metric field in the dataset's metric list. It seems critical to let the user know how this is used.
I don't think this is how we want this to work (user-provided name as a unique key to associate with charts, but somehow is important to let the user know about it)
Longer term, we should use UUIDs for metrics internally and rename this "Metric Alias" or just get rid of it.