vaadin / flow-components

Java counterpart of Vaadin Web Components
101 stars 66 forks source link

Router links in menu bar function inconsistently if placed in root vs submenu #1440

Open theshoeshiner opened 4 years ago

theshoeshiner commented 4 years ago

When using AppLayout, I have added MenuBar to the top nav area, and embedded RouterLinks into the menu bar. When clicking a router link that was added directly to the MenuBar (not a submenu) the entire page refreshes and creates a new UI instance. If I add the exact same RouterLink to a Sub menu, the router link functions correctly.

It seems like this may be related to other issues concerning refreshes and new UI instances, but those seem to more related to the PreserveOnRefresh functionality, not anything related to MenuBar, and they are seeing fairly consistent results, whereas with this the result is dependent on the MenuBar.

menuBar = new MenuBar();
//This home link causes a full refresh and new UI
 MenuItem home = addMenuElement(menuBar,HomeView.class, "Home");
 MenuItem admin = addMenuElement(menuBar,null, "Administration");
//This sub menu router link functions correctly and just loads the new View
addMenuElement( admin.getSubMenu(), RolesView.class, "Roles");
...
private MenuItem addMenuElement(HasMenuItems sub,Class<? extends Component> navigationTarget,String name) {
         MenuItem mi;
         if(navigationTarget != null) {
             RouterLink rl = new RouterLink(name, navigationTarget);
            rl.addClassName("router-link");
            mi = sub.addItem(rl, null);
         }
         else {
             mi = sub.addItem(name, click -> { });

         }
            return mi;
        }

I would expect both router links to function the same.

Versions:

- Vaadin / Flow version: 14.2.2
- Java version: 8
- OS version: Windows 10
- Browser version (if applicable): Chrome 83
- Application Server (if applicable): Tomcat
- IDE (if applicable): Eclipse + STS
stefanuebe commented 2 years ago

We had that issue now, too, so it is still a problem with V23 and the latest flow components.

norbertroamsys commented 8 months ago

We have exactly the same issue in V24. Is there already some idea/progress or workaround for this?

Just one tipp / additional information which may help locating or fixing the issue: We also noticed that the HighlightAction in RouterLinks at MenuBar root level are not triggered. In sub menus this works. It seems that these instances are somehow not correctly registered and getting no events in AfterNavigationObserver.afterNavigation(AfterNavigationEvent)

stefanuebe commented 8 months ago

You can try this as a workaround. It basically creates a special router link to be used as a menu bar root item by disabling the default click behavior in the browser. Instead it sends a custom event to the server to let the server handle the navigation using the UI navigate method. For the user it will look like a normal link in a menu, so they'll see the target url, can right click and open in it in a new tab, etc.

For router params, etc. simply extend the method.

    private void addHeaderContent() {
        MenuBar menuBar = new MenuBar();

        // create root links with a workaround for [1440](https://github.com/vaadin/flow-components/issues/1440)
        menuBar.addItem(createRootRouterLink("Hello", HelloWorldView.class));
        menuBar.addItem(createRootRouterLink("World", AboutView.class));

    }

    private RouterLink createRootRouterLink(String text, Class<? extends Component> target) {
        RouterLink link = new RouterLink(text, target);
        link.getElement().executeJs("""
                this.addEventListener("click", e => {
                    e.preventDefault();
                    this.dispatchEvent(new CustomEvent('client-side-click'));
                });
                """);

        link.getElement().addEventListener("client-side-click", event -> {
            UI.getCurrent().navigate(target);
        });
        return link;
    }
norbertroamsys commented 3 weeks ago

We noticed that this issue still exist in Vaadin 2.4.2 with even more worse effects: In the past a reload occurred only in top level menu item links. Now the reload also happens in sub menu links!

Because it looks like a fix will not be available soon, I would like to share our workaround solution for RouterLinks in MenuBar/MenuItem: We created a sub type using the code provided by @stefanuebe with some improvements for keyboard handling and closing the menu before/on navigation:

public class MenuRouterLink extends RouterLink {

    private final MenuBar menuBar;

    /**
     * Constructor.
     *
     * @param menuBar the menu bar the link should be added
     */
    public MenuRouterLink(final MenuBar menuBar) {
        this.menuBar = menuBar;
    }

    /**
     * Workaround because of reload bug in {@link RouterLink} when used top level see <a href="https://github.com/vaadin/flow-components/issues/1440">GitHub</a> for details.
     * This overload can be removed if issue gets fixed in later Vaadin version.
     */
    @Override
    public void setRoute(final Router router, final Class<? extends Component> navigationTarget, final RouteParameters parameters) {
        final Element element = getElement();
        element.executeJs("""
                this.addEventListener("click", e => {
                    if (!(e.ctrlKey || e.metaKey)) {
                      e.preventDefault();
                      this.dispatchEvent(new CustomEvent('client-side-menu-click'));
                    }
                });
                """);
        element.addEventListener("client-side-menu-click", event -> {
            menuBar.close();
            UI.getCurrent().navigate(navigationTarget);
        });
        super.setRoute(router, navigationTarget, parameters);
    }
}

Hope it helps some other developers having this issue 🤖...

knoobie commented 3 weeks ago

Vaadin 2.4.2

This version does not exists. If you mean 24.4.2: there a bug using Links in dialogs which resulted in a full page reload. The same might be also true for other popups. Using the latest version is highly recommend (24.4.11+) instead of applying band-aid

norbertroamsys commented 2 weeks ago

@knoobie : Sorry for the typo! Yes - 24.4.2 is the version we noticed the new behavior. I did a retest using 24.4.12 this morning and it seems that the bug with the full page reload has been fixed (again) for the use in Sub Menus. Unfortunately the bug still persist for the use in root menus. So it is correct that this issue is still open.

Sorry for the confusion 😒! Hope that the issue can be fixed in the near future...