-
Notifications
You must be signed in to change notification settings - Fork 326
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
Implemented option for free-form text entry into combo box - see header ... #96
base: master
Are you sure you want to change the base?
Implemented option for free-form text entry into combo box - see header ... #96
Conversation
…er comment notes for details.
This is the traditional use case for a "ComboBox". Thanks for your edit. |
Thank you, Guy - I hope it has helped you and others, which was my goal in submitting this change. Hopefully it will get pulled into the main branch someday... |
+1 I need this functionality. |
To anyone reading this (including DF) - I'm in the process of fixing some bugs I noticed with my implementation; specifically there isn't a way to pass a user-defined value -into- the combobox (for instance, if you are doing a post-refresh cycle, and you did some validation that failed, but you want to show what the user typed after the refresh of your form). So - while this extra feature is something wanted by others - it may be a bit premature for DF to implement it. But thank you all for your support of it! |
Thanks for the update. I would very much like this functionality in the library. Can you please add a test for it as well? My work project that has been taking up all my time is almost done, and then this project will get some much needed attention. |
Thank you for your response. By "test" - do you mean something in the unit test area of the repo (/js/tests/unit/bootstrap-combobox.js)? Or something else? I'm not sure how that works, exactly – but I can figure it out. If that is what is needed and wanted, then I'll go ahead and put something in place. I can't say when it would be ready; I was hoping for something soon, but due to our development process here, I am unable to dedicate the time I would like. That said, I am pretty sure the code and method I have implemented for this will work – though it still isn't completely tested (and a unit test may help with that, so it would likely be in my interest). I need to resolve this testing phase, and then I will be ready to issue another pull request for the changes. Thank you again for your interest! Andrew From: Daniel Farrell <[email protected]mailto:[email protected]> Thanks for the update. I would very much like this functionality in the library. Can you please add a test for it as well? My work project that has been taking up all my time is almost done, and then this project will get some much needed attention. — CONFIDENTIALITY NOTICE: This e-mail and any accompanying documents contain |
Added method, via a custom "data-value" attribute on the select element of the combo-box, to enable the passing of freeform data back to the combo-box control for later display on refresh of a form. Also added ability to use the down-arrow while on a combo-box to open the drop-down option list.
Updated README file to include details on how to use the new "freeform" mode of the combo box control.
Implemented unit tests for new features; fixed bugs found during creation of unit tests; updated documentation to reflect changes.
Ok - well - many more changes now; I hope the unit tests are what was wanted. I found some more bugs with them (hey - I love that unit test tool from jQuery - very handy!) - fixed them, updated docs, etc - had a good time, overall. I can't say what I have submitted will be the final revision, but I think it is really close! Let me know what you think - comments, criticism, etc welcome... |
Andrew, thanks. I tried using this and the free-form part worked. One thing I noticed is some weird behavior with the X button even with free-form off option off. It doesn't seem to clear/reset the input as it should. Haven't dug any deeper but will update if/when I do, could just as well be something on my end. |
Just to follow up, my issue wasn't related to your code. To elaborate, when an item is selected, the change events are fired before the 'combobox-selected' class (which controls the state of the toggle button) is added to the container. Handling the change event on the source/target/element and then trying to clear the combobox doesn't work because the combobox doesn't get the 'combobox-selected' class until after the change event is fired. |
@jzamanski - So I take it that everything is working otherwise then (nothing needed on my end)? |
@aayers-fender - Yes, turned out my issue was in the main source. Not really an error, more of a feature request I guess. I forked the repo and sent a pull request. |
How would this effect the usability for people using screen readers? One of the reasons for not doing freeform text is because semantically correct HTML will allow people with screen readers to still use the select menu for the options. Also typeahead.js does a fantastic job of handling freeform text input already. Thoughts? |
To allow a homogeneous control and allow to use of the "option" html clause pure and clear design instead of creating routines with javascript to load the list (as required with typehead.js) could be a reason to implement this issue. if (this.options.validate == 1) { ... } else { ... } Finally put it in the balance. |
...comment notes for details.
This is my first time using github, so forgive me if this is wrong or incorrect in some manner...
A little background: I am a web developer for Fender (http://www.fender.com/) and we had a need for a combobox widget; I found your widget and it fit the need almost perfectly, but a means to enter in a freeform text entry (that wasn't in the dropdown) was also needed. A few minor changes, and what you see here is what I came up with.
I won't claim it to be totally correct, but it seems to work; if the "freeform" option is set when the widget is instantiated, my changes are used, otherwise the default is used.
I hope this makes sense; the comments in the header are just there to document this option - it would be preferable, should this change be merged into the main branch that they are stripped and become a part of the documentation of course.
Thank you.