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

Dialog is not updated when doing hotswap #20473

Closed
Artur- opened this issue Nov 14, 2024 · 3 comments · Fixed by #20540
Closed

Dialog is not updated when doing hotswap #20473

Artur- opened this issue Nov 14, 2024 · 3 comments · Fixed by #20540

Comments

@Artur-
Copy link
Member

Artur- commented Nov 14, 2024

Description of the bug

Given

@Route(value = "overlay", layout = MainLayout.class)
@AnonymousAllowed
public class OverlayView extends VerticalLayout {

    @Override
    protected void onAttach(AttachEvent attachEvent) {
        super.onAttach(attachEvent);

        Dialog dialog = new Dialog("The Dialog");
        dialog.setCloseOnOutsideClick(true);
        dialog.setCloseOnEsc(true);

        dialog.add(new Div("Text nr 1"));

        dialog.open();
    }

}

if you are running using HotswapAgent and edit the div text to "Text nr 2", then a dialog with "Text nr 1" remains on screen and a dialog with "Text nr 2" is not added at all. If you manually refresh the page, the dialog with "Text nr 2" is shown

Expected behavior

The old dialog is closed and the new one opened

Minimal reproducible example

as above

Versions

  • Vaadin / Flow version: 24.5.3
@mcollovati
Copy link
Collaborator

Refresh seems to work if the dialog is added to the route component.

add(dialog);
dialog.open();

If not added explicitly to another component, the dialog has UI as a parent, so it is not detached during refresh.
On refresh, we could probably remove modal components, but this might not be correct in all situations.
For example, if the dialog is added by a layout, and we request a refresh for the route component only, the dialog would not be re-opened, since the layout is not detached and attached.

@mcollovati
Copy link
Collaborator

In the case of route chain refresh, if dialogs attached to the UI are forcibly removed, they are not re-added.
Dialog class registers an afterNavigationListener that for programmatic navigation removes the beforeClientResponse callback that should do the attach.

https://github.com/vaadin/flow-components/blob/985c10c20d40bb706326d401bed7be50a45de4eb/vaadin-dialog-flow-parent/vaadin-dialog-flow/src/main/java/com/vaadin/flow/component/dialog/Dialog.java#L899-L925

@mcollovati mcollovati self-assigned this Nov 25, 2024
mcollovati added a commit that referenced this issue Nov 25, 2024
Modal components attached to the UI were not removed or replaced during
self-navigation triggered by a route refresh.
This change updates navigation handler to ensure modal components are
removed and adds a new navigation trigger for route refresh to differentiate
programmatic navigations (e.g., forward actions).
It also modifies Hotswapper to require a full chain refresh when modal
components are present.

Fixes #20473
@mcollovati mcollovati moved this from ⚒️ In progress to 🔎Iteration reviews in Vaadin Flow ongoing work (Vaadin 10+) Nov 25, 2024
@sveinnetnordic
Copy link

sveinnetnordic commented Nov 25, 2024

I am working with a big dialog an tough that form field was updated on the "new" hotswap way, but that did not happen.
What about having a way to set the way hotswap should work in Copilot in the browser (set at cookie etc). Mode: Full refresh/"new refresh". A -Dvaadin.hotswapmode=... would also be nice. I assume the "new hotswap" will be default.

EDIT: At the moment of saving this text a fix was created :-)

mcollovati added a commit that referenced this issue Nov 25, 2024
Modal components attached to the UI were not removed or replaced during
self-navigation triggered by a route refresh.
This change updates navigation handler to ensure modal components are
removed and adds a new navigation trigger for route refresh to differentiate
programmatic navigations (e.g., forward actions).
It also modifies Hotswapper to require a full chain refresh when modal
components are present.

Fixes #20473
mcollovati added a commit that referenced this issue Nov 29, 2024
Modal components attached to the UI were not removed or replaced during
self-navigation triggered by a route refresh.
This change updates navigation handler to ensure modal components are
removed and adds a new navigation trigger for route refresh to differentiate
programmatic navigations (e.g., forward actions).
It also modifies Hotswapper to require a full chain refresh when modal
components are present.

Fixes #20473
@github-project-automation github-project-automation bot moved this from 🔖 Normal Priority (P2) to ✅ Closed in Vaadin Flow bugs & maintenance (Vaadin 10+) Nov 29, 2024
@github-project-automation github-project-automation bot moved this from 🔎Iteration reviews to Done in Vaadin Flow ongoing work (Vaadin 10+) Nov 29, 2024
vaadin-bot pushed a commit that referenced this issue Nov 29, 2024
Modal components attached to the UI were not removed or replaced during
self-navigation triggered by a route refresh.
This change updates navigation handler to ensure modal components are
removed and adds a new navigation trigger for route refresh to differentiate
programmatic navigations (e.g., forward actions).
It also modifies Hotswapper to require a full chain refresh when modal
components are present.

Fixes #20473
vaadin-bot pushed a commit that referenced this issue Nov 29, 2024
Modal components attached to the UI were not removed or replaced during
self-navigation triggered by a route refresh.
This change updates navigation handler to ensure modal components are
removed and adds a new navigation trigger for route refresh to differentiate
programmatic navigations (e.g., forward actions).
It also modifies Hotswapper to require a full chain refresh when modal
components are present.

Fixes #20473
vaadin-bot added a commit that referenced this issue Nov 29, 2024
Modal components attached to the UI were not removed or replaced during
self-navigation triggered by a route refresh.
This change updates navigation handler to ensure modal components are
removed and adds a new navigation trigger for route refresh to differentiate
programmatic navigations (e.g., forward actions).
It also modifies Hotswapper to require a full chain refresh when modal
components are present.

Fixes #20473

Co-authored-by: Marco Collovati <marco@vaadin.com>
vaadin-bot added a commit that referenced this issue Nov 29, 2024
Modal components attached to the UI were not removed or replaced during
self-navigation triggered by a route refresh.
This change updates navigation handler to ensure modal components are
removed and adds a new navigation trigger for route refresh to differentiate
programmatic navigations (e.g., forward actions).
It also modifies Hotswapper to require a full chain refresh when modal
components are present.

Fixes #20473

Co-authored-by: Marco Collovati <marco@vaadin.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging a pull request may close this issue.

4 participants