Skip to content
This repository has been archived by the owner on Jan 5, 2023. It is now read-only.

Keep focus on input after entering a key #32

Merged
merged 3 commits into from Dec 2, 2019
Merged

Keep focus on input after entering a key #32

merged 3 commits into from Dec 2, 2019

Conversation

nixterrimus
Copy link
Contributor

Keeps the focus on the keypad after entering a key by listening for changes to the focus event.

In future revisions we need to listen for keyboard events when focused and handle focus/blur events for showing and hiding the keyboard.

Copy link
Contributor

@mpolyak mpolyak left a comment

Choose a reason for hiding this comment

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

I periodically skim the comments in #exercise-next, and I'm guessing this has to do with your investigation around keeping the focus in the input, something is taking it away?

event.preventDefault();
this.inputRef.focus();
} else {
this.inputRef.blur();
Copy link
Contributor

Choose a reason for hiding this comment

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

Related to the above question, why does this need an explicit blur, wouldn't doing nothing here automatically blur the input as the focus leaves?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is for when a learner tabs beyond the field.

Comment on lines +229 to +231
if (event.relatedTarget === null) {
event.preventDefault();
this.inputRef.focus();
Copy link
Contributor

Choose a reason for hiding this comment

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

So the intent is to grab focus when the focus leaves any "input" element in the document, not just inputRef?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This only happens if THIS input is focused. There's a check for that at the start of this block

Copy link
Contributor

Choose a reason for hiding this comment

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

What I'm getting from this code is that - if a user is inputting via touching the keypad, either with their finger/finger alternative or with a mouse, we keep the focus on the input field itself despite the fact that the user is interacting with the keypad because we're treating the keypad as if it were an external keyboard. Is that right?

Some documentation here explaining the focus flow would really help as it's not a common pattern and a little difficult to reason about.

The only thing I'm concerned about it what this flow is like if you have a screen reader on. Can a user still navigate through the keypad keys without the focus being pulled back to the input field?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe the flow still works out ok (at least in my testing!). I've added a bunch of comments to make more clear what's going on here. Thank you for the suggestion.

Copy link
Contributor

@mpolyak mpolyak Nov 22, 2019

Choose a reason for hiding this comment

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

I was also wondering what happens if there are multiple widgets using math-input in an exercise question? Is it still working as expected with the input the user is currently interacting.

EDIT:
n/m the focused state check handles this case.

src/components/input/math-input.js Outdated Show resolved Hide resolved
Copy link
Contributor

@mpolyak mpolyak left a comment

Choose a reason for hiding this comment

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

Thank you @nixterrimus, LGTM.

Copy link
Contributor

@dierat dierat left a comment

Choose a reason for hiding this comment

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

Thanks for adding all the documentation!!

@nixterrimus nixterrimus merged commit e745368 into Khan:master Dec 2, 2019
nixterrimus pushed a commit that referenced this pull request Jan 22, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants