JedWatson / react-select

The Select Component for React.js
https://react-select.com/
MIT License
27.65k stars 4.13k forks source link

onBlur behaviour is affected by "touch capabilities" #3737

Open theKashey opened 5 years ago

theKashey commented 5 years ago

Are you reporting a bug or runtime error?

That's not really a bug, and not an error - this is a not correct behavior.

There is an option - blurInputOnSelect

and the effect of it quite simple:

  selectOption = (newValue: OptionType) => {
    ....
    if (blurInputOnSelect) {
      this.blurInput();
    }

Problems

  1. Should it be delayed? In normal conditions there are two different events (click/change, blur) comes one after another with a little "gap" between them. But in this case - there is no gap. This results into some differences in the "state management" - blur evens is reported too soon - before the change(or select) event is finishing propagation (state change applied back to the component). Probably onBlur should be called in setTimeout, or at least a Promise

  2. Should it even exists?
    For all "touch devices", windows 10 on hybrid devices included, or only when select is caused by "touch" event? Probably here we should check touch event, and execute blur only there, but not as a part of selectOption

  3. Is it a real problem? Yes, it is. We run into it twice this year, causing SEV1s both times.

tstelzer commented 5 years ago

I believe we are having issues stemming from this as well, on touch devices the onBlur triggers before onChange, sometimes, which prevents the change event entirely.

theKashey commented 5 years ago

In our case - onBlur, fired with the still old event, was canceling onChange. Just wrapping onBlur with setImmediate fixed the problem, and that's a quick fix we might do. Adding blur only for touch-driven selects would make it even better, but that's something good to have, not crusial to have.

tstelzer commented 5 years ago

Yup, though we're using setTimeout(f, 0) instead of setImmediate(f).

theKashey commented 5 years ago

Who will try Promise.resolve().then(onBlur)? Not sure that would not be too fast. setImmediate is more "correct" way to emulate "browser tick", however, has to be polyfilled.

GeorgeMarcusGG commented 4 years ago

I wasted an entire day on this. I couldn't figure out why my test wasn't working. Apparently the test environment has touch capabilities...

theKashey commented 4 years ago

Lucky one, caught it in tests :) For select itself, this is not a real issue - as well not an issue for a random form management system - only for the ones which updates data on blur event as well (more common is to listen to onChange only).

However the behaviour of touch, and non-touch devices should not be so different and react-select should match native browser behaviour, letting js process whatever it needs between two events.

It's a bug.

karlvr commented 4 years ago

We appear to have this issue with Creatable on iOS (iPad only) where the <input> is receiving the blur event before the chosen option receives its click event, and so the menu closes before an option can be chosen! I'm currently trying to create a workaround. This used to work, so I'm not sure if it's an iOS behaviour change.

In particular, I think the implementation of onInputBlur in Select expects to find document.activeElement inside the menuListRef, but on my iPad the document.activeElement is the document.body.