-
Notifications
You must be signed in to change notification settings - Fork 208
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
fix(number-field): updated number field to respect all locales #4508
Conversation
Lighthouse scores
What is this?Lighthouse scores comparing the documentation site built from the PR ("Branch") to that of the production documentation site ("Latest") and the build currently on Transfer Size
Request Count
|
Tachometer resultsChromenumber-field permalinkbasic-test
slider permalinktest-basic
Firefoxnumber-field permalinkbasic-test
slider permalinktest-basic
|
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.
This works!!! 🍡
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.
@blunteshwar Can you also create a story for this so that it surfaces up to the VRT's and we can make sure it always passes for other locales?
Description
Number-field was not respecting other locales than 'en'.
Related issue(s)
Motivation and context
The function
convertValueToNumber
was not receiving correct input when locale was anything other than 'en'.In function
stepBy
earlier we were doingthis.inputElement.value = value.toString();
which was not respecting locales.Thus to respect locales we are now formating the value like this
this.inputElement.value = this.numberFormatter.format(value);
How has this been tested?
I wrote some tests in
Number-field.test.ts
Test here
Screenshots (if appropriate)
Types of changes
Checklist
Best practices
This repository uses conventional commit syntax for each commit message; note that the GitHub UI does not use this by default so be cautious when accepting suggested changes. Avoid the "Update branch" button on the pull request and opt instead for rebasing your branch against
main
.