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

UpNavigation with Hasparamater in v4 #317

Open
weiliang2 opened this issue Mar 15, 2020 · 6 comments
Open

UpNavigation with Hasparamater in v4 #317

weiliang2 opened this issue Mar 15, 2020 · 6 comments
Labels
Milestone

Comments

@weiliang2
Copy link

weiliang2 commented Mar 15, 2020

Describe the bug
A clear and concise description of what the bug is.

Caused by: java.lang.IllegalArgumentException: Navigation target 'QuoteQtyView' requires a parameter and can not be resolved. Use 'public <T, C extends Component & HasUrlParameter<T>> String getUrl(Class<? extends C> navigationTarget, T parameter)' instead
	at com.vaadin.flow.router.RouteConfiguration.getUrl(RouteConfiguration.java:407) ~[flow-server-2.1.5.jar:2.1.5]
	at com.github.appreciated.app.layout.navigation.UpNavigationHelper.performUpNavigation(UpNavigationHelper.java:62) ~[app-layout-addon-4.0.0.rc5.jar:4.0.0.rc5]
	at com.github.appreciated.app.layout.component.applayout.AppLayout.onUpNavigation(AppLayout.java:41) ~[app-layout-addon-4.0.0.rc5.jar:4.0.0.rc5]
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:1.8.0_241]
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[na:1.8.0_241]
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:1.8.0_241]
	at java.lang.reflect.Method.invoke(Method.java:498) ~[na:1.8.0_241]
	at com.vaadin.flow.server.communication.rpc.PublishedServerEventHandlerRpcHandler.invokeMethod(PublishedServerEventHandlerRpcHandler.java:167) ~[flow-server-2.1.5.jar:2.1.5]
	... 99 common frames omitted

it used to work in the older version.

@weiliang2 weiliang2 added the bug label Mar 15, 2020
@appreciated
Copy link
Owner

So what needs to be done to reproduce the issue?

@weiliang2
Copy link
Author

weiliang2 commented Mar 15, 2020

hello,

view 1:

@Route(value = "", layout = MainAppLayout.class)
public class View1 extends VerticalLayout {

    public View1() {
        Button navigate = new Button("Navigate to");
        navigate.addClickListener(e -> {
        	UI.getCurrent().navigate(View2.class, 123);	
        });
        add(navigate);
    }
}

View2:

@Route(value = "Route2", layout = MainAppLayout.class)
public class View2 extends VerticalLayout implements HasUrlParameter<Integer> {

    public View2() {
    }
 
 @Override
	public void setParameter(BeforeEvent event, Integer parameter) {
		add(new Label(parameter.toString()));
	}
}

MainAppLayout:

@Push
@Viewport("width=device-width, minimum-scale=1.0, initial-scale=1.0, user-scalable=yes")
public class MainAppLayout extends AppLayoutRouterLayout<LeftLayouts.Left> {

	public MainAppLayout() {
 
        init(AppLayoutBuilder.get(LeftLayouts.Left.class)
                .withTitle("App Layout")
                .withAppBar(AppBarBuilder.get()
                         .build())
                .withAppMenu(LeftAppMenuBuilder.get()
                        .addToSection(HEADER,
                                new LeftHeaderItem("Menu-Header", "Version 4.0.0", "images/logo.png"),
                                new LeftClickableItem("Clickable Entry", VaadinIcon.COG.create(), event -> Notification.show("onClick ..."))
                        )
                        .add(new LeftNavigationItem("Home", VaadinIcon.HOME.create(), View1.class))
                        .addToSection(FOOTER,
                                new LeftClickableItem("Clickable Entry", VaadinIcon.COG.create(), clickEvent -> Notification.show("onClick ..."))
                        )
                        .build())
                .withUpNavigation()
                .build());
        openDrawer();
    }
 
}

Step 1: click on navigate button
Step 2: click on back button.

image

@Winkefinger
Copy link

Hi!
I am having the same problem at the moment. I just ran into it while switching to this fantastic Addon.

As far as I can see, the following would help to fix it:

  • In the constructor of LeftNavigationItem the route gets registered: com.github.appreciated.app.layout.component.menu.left.items.LeftNavigationItem.LeftNavigationItem(String, Component, Class<? extends Component>)
  • There, UpNavigationHelper.registerNavigationRoute(className); is invoked. That fetches the route from RouteConfiguration via getUrl(className).

When a view uses HasUrlParameter, the registration via RouteConfiguration.forSessionScope().getUrl(className); fails.

With RouteConfiguration.forSessionScope().getUrl(className, T); (with T as view parameter), it should work.
There is also a method in RouteConfiguration.getUrl to pass a simple list of parameters, so maybe multiple constructors could be added to the LeftNavigationItem.

So,

  • with another constructor in LeftNavigationItem to pass the T parameter
  • and registering the route with the parameter in UpNavigationHelper.registerNavigationRoute

it should work. Would at least be enough for my use case. It might be needed to add some things to the builders, but I do not use the fluent API in this case anyway.

Currently, I haven't checked out this project. The fix seems quite fast and easy though - if you need any help regarding this, please let me know :-)

@Winkefinger
Copy link

Hi!
I looked into this again and have a possible workaround. With the introduction of a new class (ParameterizedLeftNavigationItem) I am able to use HasUrlParameter-Views...
I also extended the Spring-Examples to feature 2 parameterized views.

I would like to push my branch, so we could use that as basis for further discussion. But I think I have no permission to do so.

So, just notify me if you want that contribution... ;-)

@appreciated
Copy link
Owner

Another possiblity would be to add support for classes with the @HasUrlParameter annotation directly to the LeftNavigationItem. I am currently very busy, so I don't really got the time to develop and test a solution for this. I will try to come up with a workaround tonight.

@appreciated appreciated added this to the 4.0.0 beta milestone Mar 16, 2020
@weiliang2
Copy link
Author

weiliang2 commented Mar 16, 2020

I’m not sure if this helps. It works in version 2.0.8. But not on version 4.

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

No branches or pull requests

3 participants