ten1seven / what-input

A global utility for tracking the current input method (mouse/pointer, keyboard or touch).
https://ten1seven.github.io/what-input
MIT License
1.35k stars 89 forks source link

Using tab to change input elements does not trigger keyboard event #50

Closed felipesabino closed 7 years ago

felipesabino commented 7 years ago

Shouldn't changing focus from one element to another using TAB change the input type to keyboard?

Steps to reproduce at demo page:

  1. use mouse to click input Email address
  2. use keyboard TAB key to change focus to input Password
  3. input type stays as mouse 💣

If you keep using keyboard TAB key to change focus, it will change input type to mouse when button Submit is focused, which seems inconsistent

ten1seven commented 7 years ago

Hi @felipesabino,

This is intended behavior. Since everyone has to use a keyboard to interact with form elements, the script persists the last known input type while interacting with inputs, selects and textareas. I left buttons out since they are used outside of forms other interface interactions, like triggering a mobile menu.

In a previous iteration I had an optional flag that could be set to disable this behavior but I removed it in this version for simplicity. I could see a couple options to bring back that functionality:

What do you think?

jasonhibbs commented 7 years ago

I came here to report that keyboard-after-mouse didn’t update but understand your reasoning. Nice work.

Would it be accurate to say that focussing initially with mouse should leave data-whatintent="mouse" and tabbing would update data-whatinput="keyboard"?

As for buttons, I would style them differently in or outside of forms if I needed to, and would simply opt for accurate representation in the plugin.

DmitryGonchar commented 7 years ago

@ten1seven But what about updating data-whatinput to keyboard only if user pressed TAB key (in \<input>)?

Right now if user navigates between inputs with keyboard - data-whatinput can still stay as "mouse" (as described in the issue), which seems to behave like a bug

ten1seven commented 7 years ago

@DmitryGonchar it would switch to keyboard outside of the context of form elements but retain mouse (if that's what you had been using) since someone who switched to the keyboard to fill in a form might use the tab key to move to the next input.

I've been working on version 5.0 and it changes the logic of data-whatinput and data-whatintent. In this new version, data-whatinput no longer ignores form input keyboard use. It's the "hard-line", reporting exactly what's happening. data-whatintent continues to be more speculative, reporting mouse usage on mousemove and now has the "old" form input typing buffer. I'd love it if you, @jasonhibbs, and @felipesabino would take a look and let me know what you think since you all have been vocal about this feature!

https://github.com/ten1seven/what-input/tree/version-five

DmitryGonchar commented 7 years ago

@ten1seven Sorry, I did not understand, where is the problem with the suggestion about 'TAB' for inputs?

someone who switched to the keyboard to fill in a form might use the tab key to move to the next input.

Yes, and in this case (if I understand correctly) data-whatinput should become "keyboard", since right now user is using keyboard to navigate (keyboard became the current input method). But it does not.

Ok, will take a look

DmitryGonchar commented 7 years ago

About v5. Looks good!

But I am not sure it will be applicable for case that we have.

We want to show a special :focus style if user navigates around website with a keyboard (this is mostly for users with disabilities). In v5 if we set it in [data-whatinput="keyboard"] :focus { (as we do now) - it will be shown for non-disabled users when they type letters into the \<input> (which is not desired). If we do this for [data-whatintent="keyboard"] :focus { then this effect will disappear as soon as a non-disabled user drags a mouse around - also a strange (from the user's point of view) behavior.

And a code style opinion - for me it was difficult to distinguish currentInput and currentIntent variables - I often confused them because they look very similar. This can become a source of bugs later (for you or for other contributors). Some suggestions: renaming to currentIntention or currentInputIntent (maybe curInputIntent to make it shorter), or rename "Intent" part to something else.

ten1seven commented 7 years ago

@DmitryGonchar this is the big conundrum that I have with this script:

My attempt to address these conflicting usages was for data-whatinput to behave like bullet #1 and data-whatintent to behave like bullet #2. I'm curious what you think would be the ideal behavior? I'm still tweaking v5 and am happy to get more feedback!

Also, the current live demo page still relies on v4, so I'm hoping you downloaded a zip of v5 to test out the new behavior.

DmitryGonchar commented 7 years ago

ah, ok, so we are free to change the behavior of data-whatintent.

1) I would change currentIntent condition to

// list of keys codes for which we change intent even for form inputs
var changeIntentMap = [
 9 // Tab
];

...

if( (... && formInputs.indexOf(activeElem.nodeName.toLowerCase()) === -1) 
  || changeIntentMap.indexOf(eventKey) !== -1 ) { 
  currentIntent  = value;

this will enable switching intent to keyboard on Tab for inputs

2) And also there need to be a config option to disable switching intent to mouse on mouse move. I am not sure why it is there, since most often moving mouse is not an input action.

DmitryGonchar commented 7 years ago

Actually, based on your description of #1 ("Report switches in input immediately") you should change whatinput on mousemove , not whatintent. Then config option is not needed.

If you go with this option, remember to add a Breaking change comment to changelog.

I can create a PR for the Tab part, if you are ok with that.

DmitryGonchar commented 7 years ago

@ten1seven Should I?

ten1seven commented 7 years ago

Yes, I think this is a good idea 👍

DmitryGonchar commented 7 years ago

@ten1seven Sorry for the delay, done. Other files were generated by running gulp

Should I make similar fix and PR to master ?

Have you made up your mind about 2) - changing input instead of intent on mouse move (see this comment)?

ten1seven commented 7 years ago

Hi @DmitryGonchar, thanks! I'm out of the country on vacation all this week so I'll take a look when I get back on Monday.

ten1seven commented 7 years ago

@DmitryGonchar just rolled your changes into version 4: https://github.com/ten1seven/what-input/pull/64

ten1seven commented 7 years ago

This is rolled into v4 and v5. Going to close this issue.

nightpool commented 6 years ago

I'm having trouble understanding why this changed was rolled only into "intent", given the earlier comment and that that intent currently does update for mouse-move events. I would much rather be able to have this behavior without having the mouse move behavior