Closed
Bug 1239730
Opened 8 years ago
Closed 8 years ago
Add spaces after every three digits to make numbers more readable
Categories
(DevTools :: Memory, defect, P2)
DevTools
Memory
Tracking
(firefox46 fixed)
RESOLVED
FIXED
Firefox 46
Tracking | Status | |
---|---|---|
firefox46 | --- | fixed |
People
(Reporter: fitzgen, Assigned: jdescottes)
References
(Blocks 1 open bug)
Details
(Whiteboard: [polish-backlog][difficulty=easy])
Attachments
(1 file, 1 obsolete file)
10.83 KB,
patch
|
jdescottes
:
review+
|
Details | Diff | Splinter Review |
Right now we have > 1234567890 but it would be more readable if it was like > 1 234 567 890
Assignee | ||
Updated•8 years ago
|
Assignee: nobody → jdescottes
Status: NEW → ASSIGNED
Comment 1•8 years ago
|
||
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
Reporter | ||
Comment 2•8 years ago
|
||
Yeah, that is probably higher priority work than this, although neither blocks the other.
Assignee | ||
Comment 3•8 years ago
|
||
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 4•8 years ago
|
||
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•8 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•8 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•8 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•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/90e90ec9da96
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox46:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
Comment 10•8 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•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•