Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/hide show results #203

Merged
merged 2 commits into from
Jun 1, 2016
Merged

Conversation

export-mike
Copy link
Contributor

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
Copy link
Owner

fmoo commented Jun 1, 2016

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

@fmoo fmoo merged commit 4ef4274 into fmoo:master Jun 1, 2016
@export-mike
Copy link
Contributor Author

I'll add it 👍

@export-mike
Copy link
Contributor Author

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

@fmoo
Copy link
Owner

fmoo commented Jun 1, 2016

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
Copy link
Contributor

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
Copy link
Contributor Author

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

@zdhickman
Copy link
Contributor

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 #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
Copy link

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

@fmoo
Copy link
Owner

fmoo commented Jul 1, 2016

@export-mike - this change (6cc4422) 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,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants