vaadin / flow-components

Java counterpart of Vaadin Web Components
101 stars 66 forks source link

lazy-loading combobox, typing in closed state -- better events #1039

Open Manfred-on-github opened 3 years ago

Manfred-on-github commented 3 years ago

Description

combobox via FLOW from java, with lazy loading, items are provided via FetchCallback, we have setAutoOpen(false).

Steps to reproduce

you can use the sample code in vaadin/vaadin-combo-box#992, and examine the system-out messages on java console.

Expected Results

Actual Results

due to bug in vaadin/vaadin-combo-box#992 this whole area is not usable at the moment, so cannot document current behavior.

in a previous version, this somehow worked, however the events 'custom-value-set' and/or 'value-changed', and 'filter', and the query request to data provider, came in inconsistently, so that from server side we couldn't find out if user is still just typing, if combobox is open, or if user has left combobox.

Version

TatuLund commented 3 years ago

While the user is typing some characters to filter values, the query to fill the data provider should only come when user actually wants to see the values (i.e. the list is already open, or user clicks open the list). Otherwise we would be retrieving lots of data from backend which the user might never see (especially in the case above, user types and then tabs out).

Just noting, that with this behavior progressive filtering wont be performed, since there is no items. Filtering this would be performed also only after popup opened.

There is also another consequence. ComboBox idea is to autoselect the value if only one filtered value is remaining.

The remaining problem is that how would ComboBox determine whether the value is a new value if ComboBox popup is not opened and thus no items fetched. Thus I assume that in case of tabing out, ComboBox needs to perform the query to check this.

rolfsmeds commented 3 years ago

Just to confirm that I've understood the proposal correctly, is the below interpretation correct?

With setAutoOpen(false) and setAllowCustomValue(true),

  1. CustomValueEvent is only triggered when field is blurred, not when dropdown is opened.
  2. Lazy data provider would be queried only when popup is explicitly opened, not when typing, not when field is blurred.

I don't really see a problem with vaadin/vaadin-combo-box#1 above, although it may need to be optional behavior rather than default (not sure at this point).

vaadin/vaadin-combo-box#2 would essentially mean that any typed value would be parsed as a custom value, without any attempt at matching it with one of the items, so if an item "Foo" exists, and the user types "Foo" into the field, it would not be parsed as the item, but as a new custom value with the same string as the item.

Manfred-on-github commented 3 years ago

The idea is to make loading of list truely lazy, behavior should be different when list was never opened, compared to 'normal' behavior when list was actually obtained. So if user never opens the list, while typing just nothing should happen, when user then blurs, this is notified to the server (via customvalueevent, or something else), and server can look this up if this already exists, or is truely new data, and react accordingly, so things like autoselect etc. are not expected from combobox code in this case. Of course you can provide this behavior as an optional feature.

Once user opens the list, the data provider is queried, passing as filter what user has currently typed, and combobox behaves normally, i.e. when user continues to type, the list is filtered accordingly (for us it does not matter if this is done in generic way by combobox code, or again queried from our data provider on server); when user deletes characters from the original filter value (when combobox had opened), of course the data provider definitely has to be queried.

As to my third point written in this issue: set content of text field without providing a row: this can be achieved from server by invoking getElement().executeJs("this.$.input.value = '"+valueForTextField+"';"); so we can live with this, although an official version would be helpful, in code above valueForTextField will have to be escaped to be a regular javascript source string. Also note: In previous paragraph regarding filter value in query of lazy data provider, I have explicitly wirtten 'what user has currently typed', as such initial text brings another scenario to consider: The screen opens (showing e.g. an address), combobox is filled with current value (e.g country 'FI'), then user opens combobox without typing: in this case the current content is not to be considered as filter, but the user wants to see whole list and select different country. Only when user changes text before opening the list (e.g. change to 'S'), this is probably intended as filter and list should just show spain,sweden,... . But if you consider this as too complicated, we can detect this also on server side as we can compare the filter value with the text that we have set when screen was opened.