Fix tabbing over typeahead elements

tanzoniteblack commented 5 years ago

Whenever a user tries to tab out of a typeahead box, if the 2 conditions in the and of re-com.typeahead/input-text-will-blur aren't met, then the local state-atom for typeahead is replaced with nil instead of having its value updated. This causes the entire component to just break.

tanzoniteblack commented 5 years ago

Also fixes most (all?) of the issues mentioned in : https://github.com/Day8/re-com/issues/165

itaysabato commented 5 years ago

Can this be merged? This bug is pretty bad... Willing to help in any way I can

Gregg8 commented 5 years ago

@tanzoniteblack, thanks for working on this, much appreciated. Today I went to merge it, and after loading it into a branch, started testing:

In terms of fixing #190 (@mpisanko), yes, I can confirm it fixes the two cases mentioned there.

However, my testing showed further sequences which lead to crashes (each one after a refreshing the browser):

Sequence 1 (happens with both the PR and live versions, except the live version DOESN'T crash if you click away):

Click on the typeahead text-box
Press 'a' - pops up suggestions
TAB - Highlights the paypal suggestion
ENTER - selects paypal
Now, pressing TAB or SHIFT+TAB or clicking away will cause a crash: Validation failed for argument ':model' in component 'input-text': Expected 'string/nil | atom'. Got '{:name "paypal", :more :stuff}'

Sequence 2 (same occurance as above):

Click on the false - Call on-change on every keystroke radio button
Press SHIFT+TAB twice - The typeahead text-box gets focus
Now, pressing TAB or SHIFT+TAB or clicking away will cause a crash: Validation failed for argument ':model' in component 'input-text': Expected 'string/nil | atom'. Got '{}'

The above two basically lead to the same error but might provide more info on fixing it.

So I am reticent to push a new version of the component when it is still crashing.

Unfortunately, I will not have time to work on it myself, so @tanzoniteblack, are you able to spare a bit more time to look into these further issues? If not then perhaps some of the other good folk, who have been involved in the typeahead issues? @blak3mill3r ?

tanzoniteblack commented 5 years ago

@Gregg8 nope, this is clearly a bug. Wonder why this isn't showing up in my usage of this component.... I'll work on an update to make sure that the demo is fully functional.

tanzoniteblack commented 5 years ago

Easily fixed issue: https://github.com/Day8/re-com/pull/194/commits/a5cf7fb7bade96a8c5f37f4f56a1ecb303687880 . Which is what I had deployed to our private maven repo, but not what I committed?

Made sure that this worked with the demo code before pushing this time.

Gregg8 commented 5 years ago

Awesome, thanks.

day8 / re-com

Fix tabbing over typeahead elements #194