vaadin / flow-components

Java counterpart of Vaadin Web Components
102 stars 65 forks source link

Combobox selection disappears if its text is an empty string #1830

Closed KiteDX closed 3 years ago

KiteDX commented 3 years ago

Created after discussion with Vaadin Expert Chat. Might be related to https://github.com/vaadin/flow-components/issues/1511, but with impact on usage.

Description

Expected outcome

Actual outcome

Minimal reproducible example

@Route("")
public class MainView extends VerticalLayout {
    List<User> users = Arrays.asList(new User("A", ""), new User("B", "b@b.b"), new User("C", "c@c.c"), new User("D", "d@d.d"));

    private static class User {
        private String name;
        private String mail;

        public User(String name) {
            this.name = name;
        }

        public User(String name, String mail) {
            this.name = name;
            this.mail = mail;
        }

        public String getName() {
            return name;
        }

        public String getMail() {
            return mail;
        }
    }

    public MainView2() {
        ComboBox<User> cbWithoutFocus = new ComboBox<>("Combobox without focus");
        cbWithoutFocus.setItems(users);
        cbWithoutFocus.setValue(users.get(0));
        cbWithoutFocus.setItemLabelGenerator(User::getMail);

        ComboBox<User> cbWithFocus = new ComboBox<>("Combobox with focus");
        cbWithFocus.setItems(users);
        cbWithFocus.setValue(users.get(0));
        cbWithFocus.setItemLabelGenerator(User::getMail);

        Button button = new Button("Add/Remove");
        button.addClickListener(e -> {
            if(this.getChildren().anyMatch(child -> Objects.equals(child, cbWithoutFocus))) {
                remove(cbWithoutFocus);
            } else {
                add(cbWithoutFocus);
                Notification.show("Combobox without focus(): " + (cbWithoutFocus.getValue() == null ? "no value set" : cbWithoutFocus.getValue().getName()), 1000, Notification.Position.BOTTOM_START);
            }
        });
        button.addClickListener(e -> {
            if(this.getChildren().anyMatch(child -> Objects.equals(child, cbWithFocus))) {
                remove(cbWithFocus);
            } else {
                add(cbWithFocus);
                cbWithFocus.focus(); // remove this and everything works fine
                Notification.show("Combobox with focus(): " + (cbWithFocus.getValue() == null ? "no value set" : cbWithFocus.getValue().getName()), 1000, Notification.Position.BOTTOM_START);
            }
        });

        add(button);
        add(cbWithoutFocus);
        add(cbWithFocus);
    }
}

Steps to reproduce

There are different ways to reproduce this behavior (and I think you found some others internally, too?)

Combobox with .focus():

Combobox without .focus():

Environment

Vaadin 14.6.3 (I tested this back until Vaadin 14.1.0 which showed the same behavior. I didn't test versions before then) Windows 10 (latest Updates)

KiteDX commented 3 years ago

I found another case that manages to reproduce this behavior: The selection will persist as long as the "empty string" value is focussed (blue border) inside of the Combobox popup even if you repeatedly open and close the popup. However as soon as the focus is lost (e.b. by opening the box and pressing "ESC" so that the currently selected value loses its blue border) the selection will disappear by the next time you open the popup (it most likely disappears the moment the focus is lost/the popup is closed). combobox_case3

rolfsmeds commented 3 years ago

Hi, as you might have already learned from expert chat, this is caused by the vaadin-combo-box web component using empty string to represent null value. This is mentioned in the value property of the web component, but unfortunately the component documentation doesn't mention that empty strings are not supported.

The only fix we can think of is to change the null representation from empty string to undefined. The problem with this is that it will change the behavior of the component, at least when used in client-side (i.e. as a pure web component, rather than through Flow), i.e. a breaking change, which we cannot bring to Vaadin 14.x.

We could make this change in Vaadin 22, in which case it will be part of the next LTS version of the platform, but will not be backported to V14. Let us know if you'd like us to look into this option.

As for a workaround for V14, would it be possible for you to use something other than an empty string, such as a whitespace character like non breaking space (  or  ) or text like [none] instead?

KiteDX commented 3 years ago

As for a workaround for V14, would it be possible for you to use something other than an empty string, such as a whitespace character like non breaking space (  or  ) or text like [none] instead?

We thought about these as well, but in the end we deemed those workarounds as impracticable:

Is there maybe an option for a separate component that fixes this issue?

rolfsmeds commented 3 years ago

That does indeed start to sound like your best option. You can discuss custom development services with Expert Chat or your Vaadin account manager.

I'll create a ticket about mentioning this limitation in the documentation. I presume we can close this warranty bugfix ticket for now?

KiteDX commented 3 years ago

Sorry for the delay. If a modification for Vaadin14 is not possible, a fix for Vaadin22 would be greatly appreciated (and imho needed for consistency's sake). The current behavior is very inconsistent with the usual behavior of the ComboBox, no matter what other component is used underneath.

rolfsmeds commented 3 years ago

Alright, I opened a separate ticket for that: https://github.com/vaadin/flow-components/issues/1900.

Closing this one as this is now a feature/refactorization issue rather than a bug.