search

fmoo / react-typeahead

Pure react-based typeahead and typeahead-tokenizer
ISC License
677 stars 232 forks source link

Feature/hide show results #203

Closed export-mike closed 8 years ago

export-mike commented 8 years ago

I found that in my case options has data therefore causes the dropdown of options to be visible even when the field isn't focused.

I initially tried using the isFocused property on state, this didn't work when you interacted with the list of options below, so i've added another flag.

fmoo commented 8 years ago

Is there a test case for this? Would be great to avoid another regression :(

export-mike commented 8 years ago

I'll add it 👍

export-mike commented 8 years ago

lets publish to .5? whats the roadmap for 2.0.0?

fmoo commented 8 years ago

Okay, I've published 2.0.0-alpha.5

For 2.0.0, I really wanted to pick up something like #146 to fix issues like #128, but then that caused #166 and the fix (#167) would have forced a major version bump.

Considering all the 2.0 only features that have gone in so far, maybe we can wait to address that to 3.0. Thoughts?

zdhickman commented 8 years ago

It seems like this PR breaks some of the original behavior of the typeahead.

After entering some text in the input field and then selecting an option, focus would remain in the input field. Before this PR, if I modified the text in the input field further (backspace, typing more letters, etc.), the options would re-render based on the new input. After this PR, I need to blur the input field and then focus again if I want the dropdown to appear.

Is this intended @fmoo @export-mike ? If not, I'll open a PR to fix this

export-mike commented 8 years ago

hi @zdhickman thanks for noticing this issue, so the PR you are referencing you're finding this fixes the issue you are seeing?

zdhickman commented 8 years ago

Partially - the linked PR relies on some changes that only live in that fork.

I had to make an additional change after that PR because of https://github.com/fmoo/react-typeahead/pull/195. That PR introduces a call to _onTextEntryUpdated when the input loses focus, resulting in showResults being set to true and the dropdown is displayed. So, a PR on this main branch would need to be a bit different

andrewvmail commented 8 years ago

+1 on broken original behaviour, I'm seeing this too.

fmoo commented 8 years ago

@export-mike - this change (6cc4422ba44a2b2cf8892496b06d5d868917ccdd) actually broke a bunch of unittests (38 are now failing).

I'm inclined to back this out unless it's a problem with the tests. Thoughts?

I really need to set up travis or something,