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

added 'force_match' option #66

Closed
wants to merge 2 commits into from
Closed

added 'force_match' option #66

wants to merge 2 commits into from

Conversation

taleinat
Copy link

When set to false, disables clearing the text box when the text doesn't match an option.
Default is true, which retains the previous behavior.

When set to false, disables clearing the text box when the text doesn't match an option.
Default is true, which retains the previous behavior.
@danielfarrell
Copy link
Owner

This needs more logic than this. You need to set the hidden input with the value or it won't get submitted with the form.

@taleinat
Copy link
Author

Okay, I'll do that. Any other pointers?

@danielfarrell
Copy link
Owner

It would be nice if there was a test for it. I think that is all I was thinking was needed.

@taleinat
Copy link
Author

I have a question: What state should the V/X button (next to the input) assume when a non-matching input is entered?

@danielfarrell
Copy link
Owner

Very good question. That paradigm kind of breaks without forcing a match to the select, doesn't it? Do you have thoughts on what it should do? My initial instinct is that maybe the X is not used at all when you aren't matching to something from the select, but I'm very open to ideas on this.

I don't have a use case for not forcing a match here, so those of you who will be using this should drive how that behaves.

@taleinat
Copy link
Author

When forcing a match, the 'V' button is changed to an 'X' when a selection is made, and clicking the 'X' clears the input. Also, when a partial input is entered but no matching option select, the button is still 'V', and clicking it empties the input and opens the list of options.

In my opinion, when not forcing a match, the button should only be 'V' when the input is empty; the rest of the time it should be 'X'. But, I also think this should be the behavior when forcing matches! What do you say, Daniel?

@danielfarrell
Copy link
Owner

Current behavior on forcing matches is clearing the input on blur if it does not match. I think that makes sense to me. Feel free to attempt to convince me why another way is better(and probably show me an example).

For non matching I'm fine with what you suggest if that is what makes sense for your use case. I'd be interested in other people's thoughts on this too. @kersten @cmouse @dreadjr @dberansky @tachang

@kersten
Copy link
Contributor

kersten commented May 15, 2013

Hi,

I think this is OK. How about emitting an error, if it is an required field? Or does it do this already?

Greetings Kersten

@denishaskin
Copy link

fyi, there's a bug in @taleinat 's implementation in that if you do select an item from the list, it sends the selected option's text instead of the option's value.

I'm working on a fix which I think is mainly just to call .select() here, but it's not quite working yet. Work in progress in on this branch: https://github.com/constantorbit/bootstrap-combobox/tree/feature.support-force-match-flag

@denishaskin
Copy link

Okay, I have a fix, it's on my fork in the branch feature.support-force-match-flag and is actually published to rubygems as well: https://rubygems.org/gems/bootstrap-combobox

@danielfarrell , I assume you will want some test coverage before I create a pull request? existing tests pass but I haven't added new ones for this... yet...

@thephw thephw self-assigned this May 13, 2014
@thephw
Copy link
Collaborator

thephw commented May 13, 2014

This is going to be targeted for 2.0, but progress is going to be tracked on pull request #96

@thephw thephw closed this May 13, 2014
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.

5 participants