vaadin / flow

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

Dialogs are not closed on view change #19648

Open MichaelPluessErni opened 4 days ago

MichaelPluessErni commented 4 days ago

Description of the bug

In version 24.4.4 of Vaadin, dialogs are not automatically closed anymore when navigating into another view.

Expected behavior

Dialogs should automatically close, as they did in previous versions.

Minimal reproducible example

  1. Navigate into a view.
  2. Navigate into a different view.
  3. Open a dialog.
  4. Go back into previous view with browser back button.
  5. Dialog is still opened.

Versions

Hilla: 24.4.2 Flow: 24.4.2 Vaadin: 24.4.4 Copilot: 24.4.3 Copilot IDE Plugin: false Java: Eclipse Adoptium 17.0.8.1 Java Hotswap: true Frontend Hotswap: Enabled, using Vite OS: amd64 Windows 10 10.0 Browser: Mozilla/5.0 (Windows NT 10.0

knoobie commented 4 days ago

Dialogs did NOT close automatically in previous versions. Which version are we talking exactly?

See for example: https://github.com/vaadin/flow-components/issues/1541

MichaelPluessErni commented 4 days ago

Previously we were on Vaadin version 24.3.13 where dialogs were automatically closed. Now with switching to 24.4.4 (without changing anything else) dialogs no not close anymore when navigating to another view.

To elaborate on that, this is how we configure our dialogs in the default case:

Dialog dialog = new Dialog();
dialog.setCloseOnEsc(true);
dialog.setCloseOnOutsideClick(false);
dialog.setModal(true);
dialog.addDialogCloseActionListener(e -> dialog.close());
knoobie commented 4 days ago

If you follow the link mentioned above you can see that it never worked (and obviously should not work). There has to be something special what you've done that they are removed on navigation previously which stopped working now. For example you are missing a call to open() to fully initialize the dialog in your example. There is also no mention how you are navigating; with UI::navigate, RouterLinks ot other means.

In the provided link is also a workaround / solution which worked for many years.

MichaelPluessErni commented 4 days ago

The call to open() was left out on purpose for brevity.

A workaround might be a necessary fix on our side, but it was surprising to see this behaviour change on a minor version update.

As it seems to our team, UI.navigate does not update the location anymore, which our application expects to happen. Specifically, we are mostly using UI.navigate(String locationString, QueryParameters queryParameters) to navigate between views.

knoobie commented 4 days ago

Specifically, we are mostly using UI.navigate(String locationString, QueryParameters queryParameters) to navigate between views.

Sounds like you found the real bug here 🙂 let's see what the team says about it on Monday.

Edit: as workaround it might be possible to disable the new react router and go back to the old one. (I think it was mentioned in the upgrading guide)

To opt-out, use the reactEnable configuration parameter. Enabling it excludes React completely from your project and fallbacks to vaadin-router and Lit:

<plugin>
   <groupId>com.vaadin</groupId>
   <artifactId>vaadin-maven-plugin</artifactId>
   <configuration>
       <reactEnable>false</reactEnable>
   </configuration>
</plugin>
MichaelPluessErni commented 4 days ago

Thanks for the recommendation.

The documentation to the reactEnable seems to be formulated confusingly: reactEnable: "Enabling it excludes React completely from your project" From the name of the property I would suspect the opposite.

Both values (true / false) did not work. To find out whether that is an issue of my build pipeline I will try more variations after the weekend.

knoobie commented 4 days ago

If you find react-router in your package.json then you know for sure it did not work.

There is also this blog post by @mstahv about a react-less flow experience: https://vaadin.com/blog/streamlined-vaadin-flow-setup-for-v24.4