Add spaces after every three digits to make numbers more readable

RESOLVED FIXED in Firefox 46

Status

defect
P2
normal
RESOLVED FIXED
3 years ago
10 months ago

People

(Reporter: fitzgen, Assigned: jdescottes)

Tracking

(Blocks 1 bug)

unspecified
Firefox 46

Firefox Tracking Flags

(firefox46 fixed)

Details

(Whiteboard: [polish-backlog][difficulty=easy])

Attachments

(1 attachment, 1 obsolete attachment)

Right now we have

> 1234567890

but it would be more readable if it was like

> 1 234 567 890
(Assignee)

Updated

3 years ago
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
I imagine we'll have to tackle the un-tableness of the tree view soon as these numbers grow larger and larger in render area
Yeah, that is probably higher priority work than this, although neither blocks the other.
(Assignee)

Comment 3

3 years ago
Posted patch bug1239730.diff (obsolete) — Splinter Review
Moved the format functions used in the different tree item classes to devtools/client/memory/utils.js and added a unit test.

try push : https://treeherder.mozilla.org/#/jobs?repo=try&revision=5899c19faad7
Attachment #8708120 - Flags: review?(jsantell)
Comment on attachment 8708120 [details] [diff] [review]
bug1239730.diff

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

::: devtools/client/memory/components/census-tree-item.js
@@ +32,5 @@
>        showSign,
>        onViewSourceInDebugger,
>      } = this.props;
>  
> +    const bytes = formatNumber(item.bytes, showSign);

Are there any locales which do not have a delimiter every 3 digits? I know the delimiter can change, but wonder if this function should be localized somehow. Not too worried.

::: devtools/client/memory/test/unit/test_utils-format.js
@@ +1,1 @@
> +/* Any copyright is dedicated to the Public Domain.

These tests can probably be put into the other test utils file since those are all pretty much one line assertions

@@ +30,5 @@
> +  equal(utils.formatNumber(-12, true), "-12",
> +    "formatNumber can display number sign (negative)");
> +
> +  equal(utils.formatPercent(12), "12%", "formatPercent returns 12% for 12");
> +  equal(utils.formatPercent(12345), "12 345%",

I was going to say this looks weird for a %, but we shouldn't have > 100% anyway :)
Attachment #8708120 - Flags: review?(jsantell) → review+
(Assignee)

Comment 5

3 years ago
Carry over r+ from previous review, merged back format tests to main utils test file.
Attachment #8708120 - Attachment is obsolete: true
Attachment #8709058 - Flags: review+
(Assignee)

Comment 6

3 years ago
Previous try push had no related failure. 
New try push for reference : https://treeherder.mozilla.org/#/jobs?repo=try&revision=c955fa558b91
Keywords: checkin-needed
(Assignee)

Comment 7

3 years ago
(In reply to Jordan Santell [:jsantell] [@jsantell] (Please needinfo) from comment #4)
> Comment on attachment 8708120 [details] [diff] [review]
> bug1239730.diff
> 
> Review of attachment 8708120 [details] [diff] [review]:

Thanks for the review!

> 
> I was going to say this looks weird for a %, but we shouldn't have > 100%
> anyway :)

I was wondering the same, but if we display an increase as %, we might have > 100% I guess?

> 
> These tests can probably be put into the other test utils file since those
> are all pretty much one line assertions

Ok done!

> Are there any locales which do not have a delimiter every 3 digits? I know
> the delimiter can change, but wonder if this function should be localized
> somehow.

I don't know either. I guess it's fine to always display it this way for now.

Comment 9

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/90e90ec9da96
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46

Comment 10

3 years ago
[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

Comments:
STR: Not clear.
Developer specific testing

Component: 
Name			Firefox
Version			46.0b9
Build ID		20160322075646
Update Channel          beta
User Agent		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS			Windows 7 SP1 x86_64

Expected Results: 
Developer specific testing

Actual Results: 
As expected

Updated

10 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.