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 - Focus jumps from hours field to seconds field, missing the minutes field. #283

Closed
wants to merge 2 commits into from

Conversation

IndraManickam
Copy link

@IndraManickam
Copy link
Author

@wojtekmaj can you pls review this defect and the attached PR for the fix.

@wojtekmaj
Copy link
Owner

Doesn't look like the way to go, why would we wait before focusing the next input? This will actually break date input if you're typing too fast.

@IndraManickam
Copy link
Author

@wojtekmaj the wait is actually introduced only to fix the date input if the user is typing in too fast.
If there are two key up happening rapidly from one field, it should stay that way. From the user's perspective, he is typing in the value on the hour field. but since we have the move focus function in-between, as per the browser the focus moves to the minute field, and the second key up results as the event target minute field. I have described the issue on #282.

  1. Set the minute value as 53.
  2. Rapidly type '12' on the hour field.
  3. The focus will move to the seconds' field.
  4. If the user type '12' slowly the focus moves to the minute field.
  5. Setting the delay for the focus will still keep the focus in the minute field only.

@IndraManickam
Copy link
Author

@wojtekmaj Please let me know if you need more information.

@wojtekmaj wojtekmaj force-pushed the main branch 2 times, most recently from fc2f3a2 to 3442fda Compare April 5, 2023 11:00
@wojtekmaj
Copy link
Owner

The correct way to fix this, it seems, is to prevent onKeyUp event from happening if two key presses overlap each other. setTimeout is not the way to do this, but thanks for the insight!

@wojtekmaj wojtekmaj closed this Apr 18, 2023
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.

None yet

2 participants