Closed Bug 1218670 Opened 4 years ago Closed 4 years ago

Add % to heap view

Categories

(DevTools :: Memory, defect)

defect
Not set

Tracking

(firefox45 fixed)

RESOLVED FIXED
Firefox 45
Tracking Status
firefox45 --- fixed

People

(Reporter: fitzgen, Assigned: fitzgen)

References

Details

Attachments

(1 file, 1 obsolete file)

We could calculate this stuff on demand when rendering, or compute it up front in the heap worker.
Assignee: nobody → nfitzgerald
Status: NEW → ASSIGNED
Attached patch Add % to heap view tree items (obsolete) — Splinter Review
Attachment #8680553 - Flags: review?(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
Attachment #8680553 - Flags: review?(jsantell) → review+
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
Attachment #8680553 - Attachment is obsolete: true
https://hg.mozilla.org/mozilla-central/rev/8b2eb24d1b35
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.