-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Right justifies numeric types and changes font to a monspaced font #1621
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1621 +/- ##
==========================================
+ Coverage 85.02% 85.67% +0.64%
==========================================
Files 600 601 +1
Lines 30307 30429 +122
Branches 7740 7778 +38
==========================================
+ Hits 25768 26069 +301
+ Misses 4384 4205 -179
Partials 155 155
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Looks good after I added a commit:
- Number formatting applies to values, not columns.
- What do you think of
Chivo Mono
as an alternative?
Give it a whirl and see what you think.
cellClass: row => clsx("codap-data-cell", `rowId-${row.__id__}`, {"formula-column": hasFormula, | ||
"right-justify": userType === "numeric" || type === "numeric"}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Number formatting should be applied to values, not columns. I'll add a commit.
// font-family: 'Roboto Mono', monospace; | ||
font-family: 'Inconsolata', monospace; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I told myself I wasn't going to do this, but I couldn't resist looking into other fonts. Take a look at it after my commit and see what you think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. The 1
bugs me a little, but the 4
matches better 🤷♀️
codap-v3 Run #5229
Run Properties:
|
Project |
codap-v3
|
Branch Review |
main
|
Run status |
Passed #5229
|
Run duration | 06m 49s |
Commit |
a742196c25: Right justifies numeric types and changes font to a monspaced font (#1621)
|
Committer | Evangeline Ireland |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
35
|
Skipped |
0
|
Passing |
220
|
View all changes introduced in this branch ↗︎ |
…ium/codap into 182991523-number-styling
Precision is implemented with this PR #1620