Combobox loses value when doing setItems with identical items set

[enver-haase]

Description

See this example:

package com.vaadin.qa.views.helloworld;

import com.vaadin.flow.component.combobox.ComboBox;
import com.vaadin.flow.component.notification.Notification;
import com.vaadin.flow.component.orderedlayout.VerticalLayout;

import com.vaadin.flow.router.PageTitle;
import com.vaadin.flow.router.Route;
import com.vaadin.flow.router.RouteAlias;

@PageTitle("Hello World")
@Route(value = "hello")
@RouteAlias(value = "")
public class HelloWorldView extends VerticalLayout {

    public HelloWorldView() {
        setSizeFull();

        String[] items = {"eins", "zwei", "drei"};

        ComboBox<String> comboBox = new ComboBox<>();
        comboBox.setItems(items);
        comboBox.setValue("zwei");
        comboBox.addValueChangeListener( e -> Notification.show("Value: "+e.getValue()));
        comboBox.setItems(items);

        add(comboBox);
    }

}

When running the example, the notification with the set value 'null' is shown.

Expected outcome

I expect the set value 'two' to be retained.

This would be consistent with Grid, and also with the Vaadin 8 Combobox.

All the components with items should operate the same way.

The culprit seems to be here: ComboBoxDataController:523 reads comboBox.setValue(null);

Minimal reproducible example

See above.

Steps to reproduce

Run the example.

Environment

Vaadin version(s): 23.2.2 OS: independent

Browsers

Issue is not browser related

[yuriy-fix]

Related to: https://github.com/vaadin/flow-components/issues/2674#issuecomment-1047758661.

[enver-haase]

@yuriy-fix can we agree that this is a bug, and so is #2674 ?

[yuriy-fix]

It's a documented behaviour, so it's not a bug. This phrase occurs in multiple places over the methods docs: https://github.com/vaadin/flow-components/blob/cadfe69f0594feeda0389a8b3ea7385de195602b/vaadin-combo-box-flow-parent/vaadin-combo-box-flow/src/main/java/com/vaadin/flow/component/combobox/ComboBoxBase.java#L608

However, we are considering changing that as there are multiple similar requests. Once we will select the best approach for https://github.com/vaadin/flow-components/issues/2674#issuecomment-1047758661 we will be able to change it.

[TatuLund]

What behavior you want really depends on the application. The current behavior is quite natural default as it more likely that new data set does not have the value than it has.

It is not hard to store the value and restore it when dataset is changed in those cases where you want so.

If something is changed it requires a new API so that one can choose whether value is preserved or not.

[enver-haase]

If that is the envisioned API, being able to switch between old (not preserving) and new behaviour, I do not see why adding a method to switch between the two and defaulting to the old behaviour would "requires new major".

[mvysny]

I agree with Enver. The combobox value should be decoupled from the items in the dropdown, for the following reasons:

1. When DataProvider fetches items lazily, it can not really be known whether the value is in the DataProvider or not. Therefore, this proposition should not be used as support for an argument.
2. Migrating from Vaadin 8, this kind of behavioral change is surprising.
3. It's not hard to work around this, but shouldn't we strive for components that do what users expect them to do (AKA no surprises principle)?
4. This behavior is not documented in functions coming from HasLazyDataView.
5. A ComboBox in a form may show a value that's no longer selectable yet it may be desired to preserve the value if the user doesn't "touch" the ComboBox. E.g. editing a web shop order that already has been dispatched yet the ordered item is no longer being sold.

I also vote for ComboBox to keep the value.