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

ListBox: Current selection is not visible after reopening a dialog #1038

Closed
yoshie82 opened this issue Jan 29, 2021 · 3 comments
Closed

ListBox: Current selection is not visible after reopening a dialog #1038

yoshie82 opened this issue Jan 29, 2021 · 3 comments

Comments

@yoshie82
Copy link

        ListBox<String> listBox = new ListBox<>();
        Dialog dialog = new Dialog(listBox);
        dialog.setModal(false);
        listBox.setItems("x1", "x2", "x3", "x4");
        listBox.setValue("x2");

       layout.add(new Button("click me", e -> {
            if (dialog.isOpened()) {
                dialog.close();
            } else {
                dialog.open();
            }
        }));

After closing and reopen the dialog again the selection-icon is not visible anymore.
Tested with: vaadin 18.0.5

@weiliang2
Copy link

any progress on this?

@vaadin-bot vaadin-bot transferred this issue from vaadin/vaadin-list-box May 19, 2021
@vaadin-bot vaadin-bot transferred this issue from vaadin/web-components May 21, 2021
@maltindal
Copy link
Contributor

maltindal commented Aug 15, 2021

I could reproduce this issue in version 18.0.5. In version 20.0.6 I couldn't reproduce it, so it worked fine there. However I don't know yet from which version on it is fixed.

Debugging

The first time the setItems() is called on the ListBox, a new DataProvider is set inside the ListBox instance. With this also a listener on the DataProvider is set up and stored in the field dataProviderListenerRegistration. Everything is working fine until here.

However, when the dialog is closed the first time, and thus the ListBox gets detached, so onDetach() the ListBox removes its dataProviderListenerRegistration, because I guess the idea here is since the UI component is anyway not shown it wants to avoid any related DataProvider computation. So the listener is removed and dataProviderListenerRegistration is set to null.

The first time onAttach() is called dataProviderListenerRegistration was non-null but from first detach it is null. Now, when the dialog is opened a second time and the ListBox is attached (onAttach()), and because dataProviderListenerRegistration is null (from last detach), a different code path is taken this time. The listener is going to be setup freshly but after that it also rebuild() is called on the ListBox instance which basically clears the whole component and again adds the items from the current DataProvider but the selection before clear is not restored afterwards. And this causes the selected-icon to be not visible the second time.

In version 20.0.6 I've noticed, that clear() and thus setValue(null) has been removed from rebuild() and done only in other needed places which fixes the issue.

Possible quick fix

One possibility is to extend ListBox like this and use your own ListBox class instead:

public class ListBox<T> extends com.vaadin.flow.component.listbox.ListBox<T> {

    @Override
    protected void onAttach(AttachEvent attachEvent) {
        T currentValue = getValue();
        super.onAttach(attachEvent);
        setValue(currentValue);
    }
}

So you basically make sure the current selection is restored. Be careful though in value change listeners on the ListBox. Calling setValue() will also trigger registered value change listeners. Calling setValue() as above is a server side value change but fortunately the API allows to distinguish whether the value change is coming from server side or client side:

listBox.addValueChangeListener(e -> {
    if (e.isFromClient()) {
        System.out.println("value change from client"); // solely triggered for client side selection changes
    } else {
        System.out.println("value change from server"); // e.g. triggered via setValue() inside onAttach()
    }
});

@tomivirkki tomivirkki self-assigned this Jan 24, 2022
@tomivirkki
Copy link
Member

Confirmed this to work with all the supported Vaadin versions. Closing. Please let us know if you'll face this issue again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants