vaadin / flow

Vaadin Flow is a Java framework binding Vaadin web components to Java. This is part of Vaadin 10+.
Apache License 2.0
616 stars 167 forks source link

Server-side modality can lead to unexpected behavior #18696

Open DiegoCardoso opened 8 months ago

DiegoCardoso commented 8 months ago

Description of the bug

There are some reported issues regarding the behavior of modal components when multiple components participating in the server-side modality are opened one after another.

The issue comes because when such a component is opened, Flow checks if there's already another modal component opened and, if there is, adds it as a child component of the already opened.

One issue it brings is the one reported in https://github.com/vaadin/flow-components/issues/5999. In summary, when we have a ConfirmDialog and a Notification opened in that order, the Notification is added to the ConfirmDialog as a child, which makes the ConfirmDialog not show the text that has been set to it (<vaadin-confirm-dialog> only shows the message property when there's no other slotted element defined).

Another issue can be seen reported in https://github.com/vaadin/flow-components/issues/3588 and https://github.com/vaadin/flow-components/issues/6052. Closing the parent modal component automatically closes the child modal component, which is not always desirable.

One last thing we noticed is that if the user opens, e.g, a Dialog and then a Notification, and at any moment a call to dialog.removeAll() is made, the Notification is removed, which might not be expected.

Expected behavior

Ideally, the child modal components should not be appended to the parent modal component on the client-side.

Minimal reproducible example

To reproduce the ConfirmDialog issue:

add(new Button("Open confirm dialog", e -> {
    ConfirmDialog dialog = new ConfirmDialog();
    dialog.setText("Are you sure?");
    dialog.open();

    Notification notification = new Notification("You clicked the button");
    notification.setOpened(true);
}));

To reproduce the dialog.removeAll() issue:

add(new Button("Add dialog", e -> {
    Dialog dialog = new Dialog();
    dialog.add(new Span("Dialog content"));
    dialog.add(new Button("Remove all children", event -> dialog.removeAll()));
    dialog.open();

    Notification notification = new Notification("You clicked the button");
    notification.setOpened(true);
}));

Versions

mshabarov commented 8 months ago

This is IMO between a bug and feature as for some business logic you may want to retain notification or close them. But would be good to provide a choice for developers. Needs a design.

ManuelAtJEE commented 3 months ago

This bug needs to be fixed for us as well. Its a show-stopper for the confirm-dialog.