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

Make CW Dropdown searchable #2226

Closed
wants to merge 6 commits into from
Closed

Conversation

gdjohnson
Copy link
Contributor

Added to the components showcase for easy testing.

Main things that should be paid attention to:

  1. That this doesn't disrupt any of the functionality of a non-searchable dropdown
  2. That this doesn't disrupt any of the functionality of existing text inputs, particularly oninput & input validation
  3. That the text input, dropdown menu options, and validation function all play well together.

Users should be able to enter any text that is either (1) included as a menu item label, (2) passes the rules specified in the inputValidationFn.

I've been unable to come up with an input defaultValue/value solution I feel totally satisfied with. The solution I've chosen has the downside that, if you select an option from the list, and then click to edit it, the input field clears/is emptied. This is the result of deleting this.value on input click. If I did not delete this.value, then the text input field would not be editable.

@zakhap
Copy link
Collaborator

zakhap commented Nov 1, 2022

I've been unable to come up with an input defaultValue/value solution I feel totally satisfied with. The solution I've chosen has the downside that, if you select an option from the list, and then click to edit it, the input field clears/is emptied. This is the result of deleting this.value on input click. If I did not delete this.value, then the text input field would not be editable

I think this is a fine solution. you selected something in full, or you input what you want. seems fine to me?

@gdjohnson
Copy link
Contributor Author

It's possible I was just feeling extremely discouraged by constantly keeping in mind some ideal, and my work is in fact acceptable.

@zakhap
Copy link
Collaborator

zakhap commented Nov 3, 2022

so i do get some weird behavior in this dropdown search component around validation x user generated input (not in list).

  1. in component kit, it says it validates A-Z (caps only) but then will validate the inputs in the array of options (which are of mixed capitalization).
  2. if i type all caps or all lower case or mix, and do not select an option from the dropdown, the box shows up red.
  3. the dropdown carat doesn't seem to work.

Is this desired? if so, we should change the copy on the component labels (that explain validation).

@gdjohnson
Copy link
Contributor Author

so i do get some weird behavior in this dropdown search component around validation x user generated input (not in list).

  1. in component kit, it says it validates A-Z (caps only) but then will validate the inputs in the array of options (which are of mixed capitalization).
  2. if i type all caps or all lower case or mix, and do not select an option from the dropdown, the box shows up red.
  3. the dropdown carat doesn't seem to work.

Is this desired? if so, we should change the copy on the component labels (that explain validation).

#1 absolutely is desired. It's a little funky in the component showcase, because I just extended the existing validation function from Alex's previous CWDropdown showcase example. But yes, the input has to validate (as legitimate) any inputs that are passed via menu items—those are treated as automatically valid given that they are default options provided by the site. I also don't think there's a real way to ensure, without a serious re-architecting of both our menu items and kit-wide input validation function, that the internal (often regex) validation rules a dev is allowed to pass are logically consistent with the default menu items. Finally, we can't just allow custom user input without any sort of validation function. So practically speaking, the conditions you describe here are (as far as I am capable of imagining) the only possible state of affairs for a component wired up this way.

#2 and #2 shouldn't occur and I'm a bit confused why they are, #2 basically implies that the validation function is completely broken, and #3 that the iconRightOnClick listener is totally broken. I will investigate what's up.

@gdjohnson
Copy link
Contributor Author

@zakhap I'm unable to replicate your validation error. Are you sure there weren't e.g. spaces in your string? Was there previous context that could've gotten input validation into a weird state?

As for the dropdown caret not working, this only happens when there are no matching options in the dropdown. I could add a default menu item that reads (eg) "No matching items found". But we might want to think through whether we want to make it clickable so users can add the option, or disable that if it's invalid input, etc. So would need some thought.

@zakhap
Copy link
Collaborator

zakhap commented Nov 4, 2022

@zakhap I'm unable to replicate your validation error. Are you sure there weren't e.g. spaces in your string? Was there previous context that could've gotten input validation into a weird state?

As for the dropdown caret not working, this only happens when there are no matching options in the dropdown. I could add a default menu item that reads (eg) "No matching items found". But we might want to think through whether we want to make it clickable so users can add the option, or disable that if it's invalid input, etc. So would need some thought.

There were spaces in my inputs

@gdjohnson
Copy link
Contributor Author

Closing because of lingering bugs and technical limitations of HTML5 Input element.

Future attempts at a searchable dropdown will need to either re-implement the InputElement from scratch, to avoid value/defaultvalue limitations, or else will require a redesign.

See e.g. construct-ui SelectList for precedent.

@gdjohnson gdjohnson closed this Nov 8, 2022
@gdjohnson gdjohnson deleted the graham.searchable-dropdown branch November 27, 2023 16:26
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.

2 participants