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

Typeahead widget does not trigger onValue event when user clears the field (no selection) #1655

Open
arndbeissner opened this issue Jan 27, 2021 · 4 comments · May be fixed by #1718
Open

Typeahead widget does not trigger onValue event when user clears the field (no selection) #1655

arndbeissner opened this issue Jan 27, 2021 · 4 comments · May be fixed by #1718
Assignees
Labels
high priority medium Medium size issue

Comments

@arndbeissner
Copy link

Bug

If set to controlled but not required the user can select items from the list which triggers the onValue event. This is expected and needed. However, if the user then emptied the Typeahead field so that there is no selection, the onValue event is NOT triggered. This is a critical bug as it does not allow for status changes on dependent widgets.

Package Version: @dojo/widgets 7.0.3

Code

The issue can be seen with Validation.tsx example from the Dojo widgets documentation. Just remove the required attribute from the widget, add alert("value=" + value ? value.label :" -"); to the existing onValue event.

Expected behavior:

The onValue event is triggered will value undefined (to set to empty the Typeahead wants undefined instead of null, so the event should use undefined as well).

Actual behavior:

The onValue event is not trigger when the user changes the Typeahead to be empty

@tomdye
Copy link
Member

tomdye commented Jan 27, 2021

@arndbeissner thanks for raising this issue. It has started a discussion within our team regarding the behaviour of the typeahead and that of the select widget it is based upon and how a standard html select behaves with regard to deselection.

Looking at the default behaviour of a native html select, it will select the first value (if no selected value is specified) and does not allow you to deselect. The standard way to get around this is to add an empty value option to the select which therefor gets selected by default and can be used to effectively de-select.
We believe that following this approach and standardising it across both the Typeahead and the Select is the best and most consistent way forward.

The changes we put forward to achieve this are:

  • new placeholder? property to be added. When supplied, this will be used to create an empty first option on the select / typeahead which is considered valid and can be used to de-select the widget value. When it is selected, onValue will be called with this value.
  • When setting required: false, we will add a visible empty initial row to the select / typeahead values using the placeholder if supplied, if no placeholder is supplied, an empty row with a default value of '' will be used.
  • In the case of a strict typeahead, any value entered which is not in the value set provided will make the widget invalid and will fire onValidate.

@arndbeissner
Copy link
Author

@tomdye thanks for looking quite deep into this. I think it's worth it, a good Typeahead widget is a core component of so many applications.

What you outline seems to be a very flexible solution, and it also gives a way to make that "empty" selection from within the list, not just from within the textbox. I just have two amendments (which may already be on your list):

  • If that empty row gets a default of '', the initial value should default to it as well, not undefined, as it is now (the type definition for value also is string | undefined currently).
  • In strict typeahead mode with required: false, when the user clears the text field via mouse/keyboard, that should lead to the same outcome as selecting the "empty" item in the list (whether it has a placeholder defined or not)

That being said, for 7.x, I will have to make a private fork with a simple behavior patch that makes onValue fire on empty, because honestly, I just can't ship as it is. Even if it's just a small admin application.

@tomdye
Copy link
Member

tomdye commented Jan 27, 2021

I agree with both your amendments @arndbeissner , I thought they might have been implied by the changes I suggested but I'm thankful that you clarified.

@tomdye tomdye self-assigned this Mar 22, 2021
@tomdye
Copy link
Member

tomdye commented Mar 22, 2021

This issue is related to #583 also

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high priority medium Medium size issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants