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

Display menu #26

Closed
wants to merge 4 commits into from
Closed

Display menu #26

wants to merge 4 commits into from

Conversation

michelre
Copy link

Thanks for your great job!

I needed a way to hide the dropdown menu when the input field is empty. So I added a boolean props to do that!

@ericgio
Copy link
Owner

ericgio commented Jun 22, 2016

Can you tell me a little more about your use case? While I could potentially see a case for externally controlling the menu visibility, your implementation feels a bit specific. My feeling is that there could be a general showMenu prop, and you could use the onInputChange hook to fire it when the text string is empty.

@michelre
Copy link
Author

My case was simple but very specific, you're right. In fact, I didn't want the menu to be automatically displayed when the user click on the input field and when this field is empty (http://getbootstrap.com/2.3.2/javascript.html#typeahead)

My first idea was to control this option with a boolean since it seemed more natural to me. But your idea is great!!

Thanks! :)

@ericgio
Copy link
Owner

ericgio commented Jun 23, 2016

Ah actually, it sounds like you're saying you don't want to display the menu on focus by default, just like the twitter bootstrap example. So in that case, I guess the callback makes less sense. Someone just opened an issue with a similar use case, so it seems valid.

@ericgio ericgio mentioned this pull request Jun 23, 2016
@michelre
Copy link
Author

michelre commented Jun 30, 2016

Ok, I've removed the callback, you were right, it was completely unecessary!

@esthersweon
Copy link

esthersweon commented Jul 8, 2016

@ericgio just checking in – will this be merged in soon? Waiting on this fix before implementing this library in a project.

@ericgio
Copy link
Owner

ericgio commented Jul 13, 2016

Sorry for the delay, I'm out of the country on vacation for a couple weeks. I'll do my best to get it in sometime next week when I'm back.

@ericgio
Copy link
Owner

ericgio commented Jul 19, 2016

minLength prop added in v0.6.0

@ericgio ericgio closed this Jul 19, 2016
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.

None yet

3 participants