Skip to content

Comma-delimit numbers; use tabular figures in tables#338

Merged
arjunsalyan merged 1 commit intomainfrom
feature/humanize
Aug 13, 2022
Merged

Comma-delimit numbers; use tabular figures in tables#338
arjunsalyan merged 1 commit intomainfrom
feature/humanize

Conversation

@amake
Copy link
Contributor

@amake amake commented Mar 23, 2022

I find large numbers difficult to read without delimiters. Also, in tables, using tabular-nums can help a lot with legibility.

I did try to test this with a local setup, but I'm not very confident that I didn't maybe break something.

@codecov
Copy link

codecov bot commented Mar 23, 2022

Codecov Report

Merging #338 (da3f478) into main (ed3d8ef) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #338   +/-   ##
=======================================
  Coverage   81.17%   81.17%           
=======================================
  Files         111      111           
  Lines        2762     2762           
  Branches      216      216           
=======================================
  Hits         2242     2242           
  Misses        474      474           
  Partials       46       46           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed3d8ef...da3f478. Read the comment docs.

@arjunsalyan
Copy link
Member

arjunsalyan commented Mar 31, 2022

Thank you. From a quick look it all looks good to me, but I do not have access to my computer at this moment, so can't verify the actual behaviour. Let's keep it open until someone or I can verify that this does not break anything.

@amake amake force-pushed the feature/humanize branch 2 times, most recently from bc79f98 to 1e5142f Compare May 5, 2022 12:53
@amake amake force-pushed the feature/humanize branch from 1e5142f to da3f478 Compare May 5, 2022 13:12
@amake
Copy link
Contributor Author

amake commented May 5, 2022

Using #339 I was able to locally confirm that all pages I could feasibly test are working without issue.

The only one I wasn't able to test was the profile screen which requires logging in.

@mojca
Copy link
Member

mojca commented May 5, 2022

Can you please share some screenshots here?

@amake
Copy link
Contributor Author

amake commented May 6, 2022

Before

image

After

image

@amake
Copy link
Contributor Author

amake commented May 6, 2022

The screenshots above illustrate 99% of the change. Basically anywhere an integer is shown it will be comma-delimited.

The tabular number thing is extremely subtle: without changing the font, only numbers become monospaced. This helps with tabular data where you want to be able to visually compare the sizes of numbers; otherwise for instance a run of 1s will make a number appear much narrower than a perhaps larger number that has multiple 8s.

Because I don't have mpstats working locally, I had to manually "fake" the following "after" picture:

Before

Screen Shot 2022-05-06 at 21 46 54

After

Screen Shot 2022-05-06 at 21 46 42

@arjunsalyan arjunsalyan merged commit eb8c7ed into main Aug 13, 2022
@arjunsalyan arjunsalyan deleted the feature/humanize branch August 13, 2022 10:34
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