Skip to content
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(numberInput):Cursor Positioning Fixed #180

Merged
merged 2 commits into from
Jan 26, 2022

Conversation

tatata96
Copy link
Contributor

@tatata96 tatata96 commented Jan 26, 2022

Description:
When user tries to enter a new input by selecting the default value and typing the new input without deleting the current one, the cursor appears on the left side of the new input when it should be on the right side, there is no issue when the default value is deleted and the new input is entered afterwards. You can test it in "Number Input with shouldFormatToLocaleString".

Solution:
A condition added to distinguish if one digit is edited, if so cursor positioning is applied, if not meaning that the input is a new one cursor position not modified.

Copy link
Collaborator

@jamcry jamcry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good code-wise. But I'm not able to test it, so if you think you tested enough and it's not breaking anything else, I think it's good to go 👍

if (
prevValueThousandthsSeparatorCount ===
thousandthsSeparatorCount + 1
// eslint-disable-next-line no-magic-numbers
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// eslint-disable-next-line no-magic-numbers

Do we need this? It shouldn't give magic-numbers error for 1

@tatata96 tatata96 merged commit b42163b into next-release Jan 26, 2022
@@ -1,6 +1,6 @@
{
"name": "@hipo/react-ui-toolkit",
"version": "1.0.0-alpha.5.1.0",
"version": "1.0.0-alpha.5.1.1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Version change shouldn't have been done here. It is generally better if the person who is preparing the new published version to update the version of the package after all the changes are merged. Also version changes should be reflected on the package-lock file as well.

@edizcelik edizcelik deleted the fix/number-input-cursor-position branch January 26, 2022 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants