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: input refocused after blur #541

Merged
merged 1 commit into from
Jun 9, 2021

Conversation

ThanoozN
Copy link
Contributor

@ThanoozN ThanoozN commented May 25, 2021

Describe the issue/change

In a huge app (my application has some 40+ controls and form management is done using Formik) and type some data in the react-number-format control and hit 'tab'/ go to some other control, then the focus goes back to the previous control this can be seen by entering a huge huge number in the example or a very small number in the huge app.

Add CodeSandbox link to illustrate the issue (If applicable)

Describe specs for failing cases if this is an issue (If applicable)

Describe the changes proposed/implemented in this PR

Debugged the issue and it narrowed down this line where the setTimeout execution was late so clearing this on blur and on unmount of the component to avoid a memory leak.

Link Github issue if this PR solved an existing issue

There was no such issue raised

Example usage (If applicable)

Type a huge huge number in the example app and hit 'tab' to observe the issue

Screenshot (If applicable)

Please check which browsers were used for testing

  • Chrome
  • Firefox
  • Firefox (Android)
  • Chrome (Android)
  • Safari (OSX)
  • Safari (iOS)

@s-yadav
Copy link
Owner

s-yadav commented May 27, 2021

@ThanoozN The change looks good. Can you test for it.

@ThanoozN
Copy link
Contributor Author

@ThanoozN The change looks good. Can you test for it.

hi @s-yadav,
all the unit tests were passing,
I've bundled the app after the fix and observed that the changes are working fine.
also, I entered some 1000+ digit number(to simulate) in the example app to verify the changes.

@s-yadav
Copy link
Owner

s-yadav commented May 28, 2021

Sorry, Typo. I meant to add test cases for your change.

@dahaca
Copy link

dahaca commented Jun 8, 2021

What's the status of this PR? Is the merge planned?

@ThanoozN
Copy link
Contributor Author

ThanoozN commented Jun 8, 2021

will work on a test case and update the PR

@ThanoozN
Copy link
Contributor Author

ThanoozN commented Jun 9, 2021

Hi unable to test that scenario using unit tests.
can we push the commit like clearing the memory leak

@s-yadav
Copy link
Owner

s-yadav commented Jun 9, 2021

Humm, I see your point. We will merge this.

@s-yadav s-yadav merged commit 14208b6 into s-yadav:master Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants