Skip to content

Add support for the µWh unit in tooltips showing energy amounts.#4246

Merged
julienw merged 2 commits into
firefox-devtools:mainfrom
fqueze:microwatthour
Sep 27, 2022
Merged

Add support for the µWh unit in tooltips showing energy amounts.#4246
julienw merged 2 commits into
firefox-devtools:mainfrom
fqueze:microwatthour

Conversation

@fqueze

@fqueze fqueze commented Sep 26, 2022

Copy link
Copy Markdown
Contributor

The changes made to the tooltip in in #4172 are very useful, but very often when looking at a short range, the value displayed is 0.001 mWh, or 0.000 mWh, which are not helpful.

Adding support for the µWh unit in the tooltip would solve this.

Here is a profile that can be used for testing: https://share.firefox.dev/3SwNOFM

The lowest non-zero value I have seen so far is about 0.003 µWh, when selecting only one sample (and I think this represents the power used by the profiler overhead), so I think we won't need an even smaller unit.

I didn't add the string for µW, as I think we would never see it (the minimum values I have ever seen for instantaneous power are around 10 mW).

deploy preview

@fqueze fqueze requested a review from a team as a code owner September 26, 2022 08:00
@fqueze fqueze requested a review from julienw September 26, 2022 08:02
@codecov

codecov Bot commented Sep 26, 2022

Copy link
Copy Markdown

Codecov Report

Base: 88.50% // Head: 88.50% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (e267dc8) compared to base (cd14bd9).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head e267dc8 differs from pull request most recent head c8e05a8. Consider uploading reports for the commit c8e05a8 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4246   +/-   ##
=======================================
  Coverage   88.50%   88.50%           
=======================================
  Files         282      282           
  Lines       24762    24764    +2     
  Branches     6617     6617           
=======================================
+ Hits        21915    21917    +2     
  Misses       2645     2645           
  Partials      202      202           
Impacted Files Coverage Δ
src/components/tooltip/TrackPower.js 100.00% <100.00%> (ø)
src/components/shared/SourceView-codemirror.js 90.72% <0.00%> (-0.10%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

expect(
screen.getByText(/current selection:/).nextSibling
).toHaveTextContent('0.005\u2069 mWh');
).toHaveTextContent('5.0\u2069 µWh');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good thing we already had this test :-)

@julienw julienw left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks!

@julienw

julienw commented Sep 26, 2022

Copy link
Copy Markdown
Contributor

I'm waiting for a review from the l10n folks before merging.

@julienw julienw enabled auto-merge (squash) September 27, 2022 12:39
@julienw julienw merged commit ffe52a3 into firefox-devtools:main Sep 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants