1218670 - Add % to heap view

Status: RESOLVED FIXED

(DevTools :: Memory, defect)

Description

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

We could calculate this stuff on demand when rendering, or compute it up front in the heap worker.

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: Add % to heap view tree items

Comment 2

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8680553 [details] [diff] [review]
Add % to heap view tree items

Review of attachment 8680553 [details] [diff] [review]:
-----------------------------------------------------------------

some l10n nits, and haven't pulled down the patch, but the header spans in the screenshot[0] here you sent need to have some left margin (not sure why that regressed)

[0] https://usercontent.irccloud-cdn.com/file/sfIc6XQi/Screen%20Shot%202015-10-29%20at%202.40.38%20AM.png

::: devtools/client/memory/components/toolbar.js
@@ +41,5 @@
>          }),
>  
>          dom.label({},
>            L10N.getStr("toolbar.breakdownBy"),
> +          " ",

this should be handled via styling, or at the very least, l10n

::: devtools/client/memory/components/tree-item.js
@@ +18,5 @@
>  const TreeItem = module.exports = createClass({
>    displayName: "tree-item",
>  
> +  formatPercent(percent) {
> +    return Math.round(percent) + "%";

l10n

Comment 3

[Jordan Santell [:jsantell] [@jsantell]]

Comment on attachment 8680553 [details] [diff] [review]
Add % to heap view tree items

Review of attachment 8680553 [details] [diff] [review]:
-----------------------------------------------------------------

some l10n nits, and haven't pulled down the patch, but the header spans in the screenshot[0] here you sent need to have some left margin (not sure why that regressed)

[0] https://usercontent.irccloud-cdn.com/file/sfIc6XQi/Screen%20Shot%202015-10-29%20at%202.40.38%20AM.png

::: devtools/client/memory/components/toolbar.js
@@ +41,5 @@
>          }),
>  
>          dom.label({},
>            L10N.getStr("toolbar.breakdownBy"),
> +          " ",

this should be handled via styling, or at the very least, l10n

::: devtools/client/memory/components/tree-item.js
@@ +18,5 @@
>  const TreeItem = module.exports = createClass({
>    displayName: "tree-item",
>  
> +  formatPercent(percent) {
> +    return Math.round(percent) + "%";

L10N.getFormatStr("percentage", percent); and the string being just "%S%", the second possibly needing escaped, check out performance.properties

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Attachment: Add % to heap view tree items

Comment 5

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/8b2eb24d1b35

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/8b2eb24d1b35

Comment 7

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

https://treeherder.mozilla.org/#/jobs?repo=mozilla-aurora&revision=cb029edb917f

Comment 8

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/8b2eb24d1b35

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/99a46382ddf1