[removed] Remove typeahead (fixes #40, #111) #152
Conversation
I initially started by trying to fix a regression to #80 which was introduced in 2fbafa, but after having evaluated the code I discovered that there's no easy way to both fix the regression and make the typeahead logic work again. This is sadly a side-effect of no longer keeping `value` in state, since we can no longer diff previous state and next state in the same fashion. Since the current typeahead situation is very poor on mobile, not to mention how broken it is in IE, I decided the best way forward is to temporarily remove the typeahead logic.
| this.keyDownHandlers[event.key].call(this, event) | ||
| else { | ||
| const { selectionStart, value } = event.target | ||
| if (value === this.props.value) |
There was a problem hiding this comment.
This was previously value === this.state.value, which caused the next block of code to execute every time, and breaking the workaround which made user-defined selections not break. When I "fixed" it by changing it to it's current form I overlooked the fact that event.target.value is always equal to this.props.value, since React prevents the input's value from ever changing.
|
I'll update the CHANGELOG and README (to mention that we currently don't have typeahead) once this is merged. I noticed we're missing a few entries in the former. Oh, and during my bug hunt I discovered a substantial amount of code that can be removed/simplified now that |
|
Just to be crystal clear: I really do want to figure out how to make typeahead work across platforms and devices, because I think it's a really cool feature. I'm not sure in which form though, because after som a11y testing I discovered that the current implementation which uses text selection ranges is...not great if you're using a screen reader. |
|
Heya, sorry for the radio silence there - and for chasing the yaks on my build PR/comments. This looks good to me, and I think this gets us pretty close on the next release then if I'm not mistaken. Anything else new that should go in the next minor/patch (need to look in to whats there. I'll open a PR with the changelog/version proposal here shortly if the milestone looks pretty clear. |
No worries; here to help! :) |
2 participants
I initially started by trying to fix a regression to #80 which was introduced
in 2fbafa, but after having evaluated the code I discovered that there's no
easy way to both fix the regression and make the typeahead logic work again.
This is sadly a side-effect of no longer keeping
valuein state, since wecan no longer diff previous state and next state in the same fashion.
Since the current typeahead situation is very poor on mobile, not to mention
how broken it is in IE, I decided the best way forward is to temporarily remove
the typeahead logic.