-
Notifications
You must be signed in to change notification settings - Fork 350
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
Conditionally switch decimal separator based on locale #800
Conversation
GeraldRequired Reviewers
Don't want to be involved in this pull request? Comment |
Size Change: +234 B (0%) Total Size: 863 kB
ℹ️ View Unchanged
|
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #800 +/- ##
==========================================
+ Coverage 60.67% 61.93% +1.26%
==========================================
Files 484 486 +2
Lines 105880 105903 +23
Branches 6224 8919 +2695
==========================================
+ Hits 64243 65596 +1353
+ Misses 41637 40307 -1330
... and 31 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
npm Snapshot: Published🎉 Good news!! We've packaged up the latest commit from this PR (0f6e97c) and published it to npm. You Example: yarn add @khanacademy/perseus@PR800 |
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 guess the actual localization is handled in Webapp? I don't see here where we are conditionally switching based on locale.
It's based off of // utils.ts
import {getDecimalSeparator} from "@khanacademy/wonder-blocks-i18n"; |
Summary:
We were supposed to be switching between
,
and.
, but we didn't see that through when updating how we're rendering icons for the v2 keypad. This updates that and adds some unit tests.Issue: LC-1453
Test plan:
.
as the decimal separator (if you're in the US),
as the decimal separator