-
Notifications
You must be signed in to change notification settings - Fork 279
feat(ui5-step-input): add thousands separator & scrolling #12751
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
Conversation
|
🧹 Preview deployment cleaned up: https://pr-12751--ui5-webcomponents.netlify.app |
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.
Now that we have thousands separator this sometimes causes the _isValueWithCorrectPrecision check to evaluate wrong.
The reason behind that is because we split the number into decilmals and whole numbers. So when we have 500,000 for example, it assumes that the three zeros behind are 'precision' numbers.
|
We also have a introduced a bug, where when we type a number, and then Submit (e.g. press Enter), if we then try to continue typing it is not working—it was working before. EDIT: Checked and it worked with different locale, so it might be something related to the comma separators mentioned in the above comment. |
packages/main/src/StepInput.ts
Outdated
| const delimiter = this.input?.value?.includes(".") ? "." : ","; | ||
| const numberParts = this.input?.value?.split(delimiter); | ||
| // @ts-ignore oFormatOptions is a private API of NumberFormat but we need it here to get the decimal separator | ||
| const delimiter = this.formatter?.oFormatOptions?.decimalSeparator || "."; |
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.
Usually the formatter is created either with format options passed as parameter or with the locale ones. In that case it will be with the locale ones so I would suggest taking the formatOptions and the decimal separator out of the locale data and not trough this private APi
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.
Fixed in 3c83942
packages/main/src/StepInput.ts
Outdated
| return; | ||
| } | ||
|
|
||
| const cursorPosition = this._getCursorPosition(); |
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.
maybe caretPosition?
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.
Fixed in 3c83942
|
🎉 This PR is included in version v2.19.0-rc.2 🎉 The release is available on v2.19.0-rc.2 Your semantic-release bot 📦🚀 |
This PR introduces two new features to the ui5-step-input component: