Drive counter tooltips from a tooltipRows schema#6023
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6023 +/- ##
=======================================
Coverage 83.77% 83.77%
=======================================
Files 329 330 +1
Lines 34423 34467 +44
Branches 9627 9651 +24
=======================================
+ Hits 28839 28876 +37
- Misses 5155 5163 +8
+ Partials 429 428 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Replace the four hardcoded category branches in TrackCounterGraph's tooltip rendering (Memory / Power / Bandwidth / processCPU) with a declarative tooltipRows array on CounterDisplayConfig. Each row picks a data source (count, rate, accumulated, selection-total, …), a value format (unit, optional CO₂e, optional auto-scale ladder), and a label. The new TrackCounterTooltip component iterates the rows, resolves each source, formats the value, and renders through TooltipDetails. Memory and CPU tooltips now use the same TooltipDetails layout as Power and Bandwidth. Profile labels are raw English so the format stays self-describing. The frontend translates labels it recognizes from a private allow-list and renders unknown ones verbatim. The dedicated TooltipTrackPower component is removed; the power/energy unit ladders move into the formatter. Bumps the processed profile format to v63. The v63 upgrader derives tooltipRows from each counter's category and name. Closes firefox-devtools#5961.
| // Maps recognized English labels to their Fluent message IDs. Labels not in | ||
| // this map render verbatim. For auto-scaled rows, the value is a Fluent ID | ||
| // prefix; the ladder tier's suffix is appended at render time. | ||
| export const KNOWN_LABEL_L10N: { [label: string]: string } = { |
There was a problem hiding this comment.
We've discussed it offline a bit, but adding here as well. I think it makes sense to have a mapping for l10n keys, but I don't think we should use the label as a string for the key. Because the English labels will likely change when we change the l10n keys as well. So it might be better to have another field to do this mapping. Maybe something like l10nHint or ideally something better.
There was a problem hiding this comment.
I thought about this after our discussion too. I think if we make it a hint then counters won't be self-descriptive anymore. So, there will be an assumption that adding a counter in backend requires communicating hint <-> label mapping to frontend. This kinda undermines our initial idea of adding counters without informing frontend. So, I would still bet on en labels inside counters. Wdyt, @canova ?
There was a problem hiding this comment.
Ah, I was still thinking about keeping labels and then having this hint additionally (and optionally). And falling back to the label if we don't have any hint or if it fails to match anything. So other users can still rely on the label without needing to look at the localizations. Or do you still think it's not self descriptive with it?
There was a problem hiding this comment.
Oh now it makes sense. Let's then keep both - required label and optional l10nHint (or similar).
3 participants
Replace the four hardcoded category branches in TrackCounterGraph's tooltip rendering (Memory / Power / Bandwidth / processCPU) with a declarative tooltipRows array on CounterDisplayConfig. Each row picks a data source (count, rate, accumulated, selection-total, …), a value format (unit, optional CO₂e, optional auto-scale ladder), and a label.
The new TrackCounterTooltip component iterates the rows, resolves each source, formats the value, and renders through TooltipDetails. Memory and CPU tooltips now use the same TooltipDetails layout as Power and Bandwidth.
Profile labels are raw English so the format stays self-describing. The frontend translates labels it recognizes from a private allow-list and renders unknown ones verbatim. The dedicated TooltipTrackPower component is removed; the power/energy unit ladders move into the formatter.
Bumps the processed profile format to v63. The v63 upgrader derives tooltipRows from each counter's category and name.
Closes #5961.
I've recorded a profile with the Power profile, please let me know if another one is needed.