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.82% 83.82% -0.01%
==========================================
Files 330 331 +1
Lines 34682 34731 +49
Branches 9703 9629 -74
==========================================
+ Hits 29072 29113 +41
- Misses 5181 5190 +9
+ 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.
canova
left a comment
canova
left a comment
There was a problem hiding this comment.
Looks good to me overall, thanks. I added some comments below
Drop the parallel ResolverContext type and read this.props directly. Extract picowatt-hour conversions into pwhToWh and pwhPerMsToWatts helpers. Memoize the per-range sample sums so mouse-move re-renders don't rescan the counter samples.
|
Thanks for your review, @canova! It seems like I have addressed all the issues now. |
canova
left a comment
canova
left a comment
There was a problem hiding this comment.
Thanks! Looks good to me, but I noticed one thing that we should fix before landing.
I was testing the localization with togglePseudoLocalization('accented') executed in the devtools console, and I can't see that the tooltips are properly localized currently.
For example, this is the same profile with profiler.firefox.com:
This is from the deploy preview:
Note that the profile in the initial comment doesn't include the labelKey so it won't work. I captured a new profile
Looks like because we updated the ftl keys to append 2.
|
Great catches, thanks @canova! Fixed now. I've also updated the PR description with the link to the profile you recorded. |
|
And thanks for the trick with |
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.
Profile