flow-components icon indicating copy to clipboard operation
flow-components copied to clipboard

CustomValueSetEvent is not fired when committing value before items are loaded

Open mstahv opened this issue 9 months ago • 9 comments

Description

If one sets ComboBox to autoOpen=false mode a customvaluesetevent is not fired, unless the popup is manually opened.

Expected outcome

Event should be fired like it is in the default mode.

Minimal reproducible example


    class PersonSelect extends ComboBox<Person> {
        {
            setItemLabelGenerator(Person::name);
            setAllowCustomValue(true);
            setAutoOpen(false);
            addCustomValueSetListener(e -> {
                Notification.show("Custom value set: " + e.getDetail());
            });

        }
    }

Steps to reproduce

Use the code above, type a new person and hit tab/enter.

Environment

Hilla: 24.6.5 Flow: 24.6.5 Vaadin: 24.6.5 Spring Boot: 3.4.2 Spring: 6.2.2 Copilot: 24.6.5 Frontend Hotswap: Disabled, using pre-built bundle OS: aarch64 Mac OS X 15.3.1 Java: Eclipse Adoptium 22.0.1 Browser: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/18.3 Safari/605.1.15 Java Hotswap: JRebel is in use IDE Plugin: 1.4.6 IntelliJ

Browsers

No response

mstahv avatar Feb 18 '25 16:02 mstahv

Maybe duplicate? https://github.com/vaadin/flow-components/issues/1039

mstahv avatar Feb 18 '25 16:02 mstahv

Seems to only happen when the combo box is using lazy-loading and when you tab/enter before results have been loaded after changing the filter. It works consistently when using client-side filtering (number of items is less than page size).

sissbruecker avatar Feb 18 '25 16:02 sissbruecker

Also wondering how it should work. I guess it would have to wait until the data is loaded and then try to match the filter against an item or send a custom value event. With that you would have some delay between pressing enter / tab and the actual value change or custom value event.

Sending a custom value event immediately just because the data hasn't loaded yet could be considered a bug as well, especially as you don't get feedback that the component is actually loading something when auto open is disabled.

sissbruecker avatar Feb 18 '25 17:02 sissbruecker

Well, bug or a feature, but that is the functionality that allows me to use ComboBox currently so that I'm happy with (I have never used autoOpen=false, filled this for a community member).

Related forum thread: https://vaadin.com/forum/t/combobox-still-poor-quality-and-not-suitable-for-data-entry-application/168345/1

mstahv avatar Feb 19 '25 07:02 mstahv

I'm the author of the forum thread were I found two new (?) bugs with ComboBox. I believe that makes it 5 in total that I've found, and I don't even use it. Almost everything seems to be a variant of "it works when all the data is on the client-side"

I assume that you have to continue fixing on ComboBox as it is, but I'd be happy if you just made a new component where almost everything is done server-side. Ideally I'd want a client-side widget that is just a text field with an anchored Panel or Popover. The only job of the client-side would be to open the panel, close it when focus leaves both, let arrow-down move into panel. Stuff like that. On the server-side this should be an abstract class that we can extend, and you could use it to create a ComboBox replacement with a lazy-loading and filtered Grid.

imho; "all widgets have to work standalone on client-side" is holding us back

guttormvik2 avatar Feb 19 '25 10:02 guttormvik2

Outcome from internal discussion: If filtered items are not loaded yet, the component should delay committing the value / removing the filter until items are loaded. Then either change the value to a matching item from the filtered items, or dispatch a custom value change. This may result in some delay before the value is changed, but is better than discarding user input.

In addition we might consider also showing a loading indicator when the overlay is closed, as there is currently no feedback that the component is still doing anything.

sissbruecker avatar Feb 28 '25 07:02 sissbruecker

Probably there is reasoning in that discussion, that might be helpful to understand the decision, but leaving couple of comments, questions:

  • You are planning to unify the behaviour also for the default "autoOpen" mode?!
  • This means that value change event in the future always needs additional server visit until it is fired (currently fixed timeout is 500ms + latencies).
  • A custom combobox component is needed for Java users who need UI suitable for fast data input 🤔

I'm bit sharing the concern ("imho; "all widgets have to work standalone on client-side" is holding us back") by @guttormvik2 here 😬

mstahv avatar Feb 28 '25 07:02 mstahv

imho; "all widgets have to work standalone on client-side" is holding us back

Ref that, here is my server-side ComboBox replacement. It consists of a TextField that opens an anchored Popover that contains a (lazy-loaded) filtered Grid (where I can have >1 column). No javascript code, other than registering a couple of event listeners. If the user just writes a value and tabs out, no data is sent to the client, except maybe to expand the value in the text field. Performance seems excellent. UX could use some work, but is acceptable. This took a lot of time, but only because I'm rubbish at client-side. The end result is embarrassingly little code. Server-side for the win!

Image

guttormvik2 avatar Feb 28 '25 09:02 guttormvik2

Some related input for the topic:

I was just using a web app using Vaadin Flow & ComboBox to input data, roughtly 50 rows, built by some very experienced Vaadin developers. That don't include a similar the hack I described in the forum thread, that I have used myself in some apps. That combined to the obsolete requests and hardcoded 500ms latency, made the UI quite nasty to use as sometimes my selections was lost. Elsewhere the UX is very nice.

Sending this in as your previous "resolution plan" to me sounds like that the only workaround for the currently bad default UX would be lost, so maybe you could re-visit your plans. And also prioritise handling of the other related PRs I recently made to improve ComboBox 🧸

mstahv avatar Mar 24 '25 19:03 mstahv