Skip to content

Commit

Permalink
fix: forward/back navigation in hybrid app (#19017)
Browse files Browse the repository at this point in the history
Going back to Hilla view from
Flow view should not override
history so that forward button
also works to get "back" to Flow view.

Fix context path navigation.

Fixes #19003
  • Loading branch information
caalador committed Mar 27, 2024
1 parent 3ab7cc4 commit 5c58117
Show file tree
Hide file tree
Showing 11 changed files with 161 additions and 20 deletions.
1 change: 1 addition & 0 deletions flow-client/src/main/frontend/Flow.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,7 @@ export class Flow {
} else if (cmd && cmd.redirect && redirectContext) {
resolve(cmd.redirect(redirectContext.pathname));
} else {
cmd?.continue?.();
this.container.style.display = '';
resolve(this.container);
}
Expand Down
34 changes: 28 additions & 6 deletions flow-server/src/main/java/com/vaadin/flow/component/UI.java
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
import com.vaadin.flow.server.VaadinService;
import com.vaadin.flow.server.VaadinServlet;
import com.vaadin.flow.server.VaadinSession;
import com.vaadin.flow.server.VaadinSessionState;
import com.vaadin.flow.server.auth.AnonymousAllowed;
import com.vaadin.flow.server.communication.PushConnection;
import com.vaadin.flow.shared.Registration;
Expand Down Expand Up @@ -1670,7 +1671,11 @@ public List<HasElement> getActiveRouterTargetsChain() {
}

static final String SERVER_CONNECTED = "this.serverConnected($0)";
public static final String CLIENT_NAVIGATE_TO = "window.dispatchEvent(new CustomEvent('vaadin-router-go', {detail: new URL($0, document.baseURI)}))";
public static final String CLIENT_NAVIGATE_TO = """
const url = new URL($0, document.baseURI);
url["clientNavigation"] = true;
window.dispatchEvent(new CustomEvent('vaadin-router-go', { detail: url}));
""";

public Element wrapperElement;
private NavigationState clientViewNavigationState;
Expand Down Expand Up @@ -1768,10 +1773,6 @@ public void browserNavigate(BrowserNavigateEvent event) {
}

final String trimmedRoute = PathUtil.trimPath(event.route);
if (!trimmedRoute.equals(event.route)) {
// See InternalRedirectHandler invoked via Router.
getPage().getHistory().replaceState(null, trimmedRoute);
}
final Location location = new Location(trimmedRoute,
QueryParameters.fromString(event.query));
NavigationTrigger navigationTrigger;
Expand Down Expand Up @@ -1808,7 +1809,28 @@ public void browserNavigate(BrowserNavigateEvent event) {
} else if (isPostponed()) {
serverPaused();
} else {
acknowledgeClient();
// acknowledge client, but cancel if session not open
serverConnected(
!getSession().getState().equals(VaadinSessionState.OPEN));
replaceStateIfDiffersAndNoReplacePending(event.route, trimmedRoute);
}
}

/**
* Do a history replaceState if the trimmed route differs from the event
* route and there is no pending replaceState command.
*
* @param route
* the event.route
* @param trimmedRoute
* the trimmed route
*/
private void replaceStateIfDiffersAndNoReplacePending(String route,
String trimmedRoute) {
if (!trimmedRoute.equals(route) && !getInternals()
.containsPendingJavascript("window.history.replaceState")) {
// See InternalRedirectHandler invoked via Router.
getPage().getHistory().replaceState(null, trimmedRoute);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,19 +101,15 @@ protected Optional<Integer> handleTriggeredBeforeEvent(

@Override
protected boolean shouldPushHistoryState(NavigationEvent event) {
if (event.getUI().getInternals().getSession().getConfiguration()
.isReactEnabled()) {
return super.shouldPushHistoryState(event);
}
if (NavigationTrigger.CLIENT_SIDE.equals(event.getTrigger())
|| NavigationTrigger.ROUTER_LINK.equals(event.getTrigger())) {
return true;
} else {
// For react router the server should push history state for
// server rendering even when client previously updated url with
// vaadin-router.
return super.shouldPushHistoryState(event)
|| (event.getUI().getInternals().getSession()
.getConfiguration().isReactEnabled()
&& NavigationTrigger.CLIENT_SIDE
.equals(event.getTrigger()));
}
return super.shouldPushHistoryState(event);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,19 @@ Stream<PendingJavaScriptInvocation> getPendingJavaScriptInvocations() {
.filter(invocation -> !invocation.isCanceled());
}

/**
* Filter pendingJsInvocations to see if an invocation expression is set
* with given filter string.
*
* @param containsFilter
* string to filter invocation expressions with
* @return true if any invocation with given expression is found.
*/
public boolean containsPendingJavascript(String containsFilter) {
return getPendingJavaScriptInvocations().anyMatch(js -> js
.getInvocation().getExpression().contains(containsFilter));
}

/**
* Records the page title set with {@link Page#setTitle(String)}.
* <p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,11 @@ function vaadinRouterGlobalClickHandler(event) {
}

// if none of the above, convert the click into a navigation event
const {pathname, search, hash} = anchor;
if (fireRouterEvent('go', {pathname, search, hash})) {
const {pathname, href, baseURI, search, hash} = anchor;
// Normalize away base from pathname. e.g. /react should remove base /view from /view/react
let normalizedPathname = href.slice(0, baseURI.length) == baseURI ? href.slice(baseURI.length) : pathname;
normalizedPathname = normalizedPathname.startsWith("/") ? normalizedPathname: "/" + normalizedPathname;
if (fireRouterEvent('go', {pathname: normalizedPathname, search, hash, clientNavigation: true})) {
event.preventDefault();
// for a click event, the scroll is reset to the top position.
if (event && event.type === 'click') {
Expand All @@ -154,8 +157,13 @@ function navigateEventHandler(event) {
if (event && event.preventDefault) {
event.preventDefault();
}
// Normalize path against baseURI if href available.
let normalizedPathname = event.detail.href && event.detail.href.slice(0, document.baseURI.length) == document.baseURI ?
event.detail.href.slice(document.baseURI.length) : event.detail.pathname;
normalizedPathname = normalizedPathname.startsWith("/") ? normalizedPathname: "/"+normalizedPathname;

// @ts-ignore
let matched = matchRoutes(Array.from(routes), event.detail.pathname);
let matched = matchRoutes(Array.from(routes), normalizedPathname);
prevNavigation = lastNavigation;

// if navigation event route targets a flow view do beforeEnter for the
Expand All @@ -176,6 +184,12 @@ function navigateEventHandler(event) {
// @ts-ignore
redirect: (path) => {
navigation(path, {replace: false});
},
continue: () => {
if(window.location.pathname !== event.detail.pathname) {
window.history.pushState(window.history.state, '', event.detail.pathname);
window.dispatchEvent(new PopStateEvent('popstate', {state: 'vaadin-router-ignore'}));
}
}
},
router,
Expand All @@ -190,7 +204,14 @@ function navigateEventHandler(event) {
continue() {
mountedContainer?.parentNode?.removeChild(mountedContainer);
mountedContainer = undefined;
navigation(event.detail.pathname, {replace: false});
// clientNavigation flag denotes navigation to a client route through
// a link or server navigate. If clientNavigation is not given the
// navigation is through a history event and we only call the router popstate event.
if (event.detail.clientNavigation) {
navigation(normalizedPathname, {replace: false});
} else {
popstateListener.listener(new PopStateEvent('popstate', {state: 'vaadin-router-ignore'}));
}
}
}, router);
} else {
Expand Down Expand Up @@ -254,6 +275,10 @@ export default function Flow() {
}
}
flow.serverSideRoutes[0].action({pathname, search}).then((container) => {
// Update last navigation when coming into serverside as we might come
// in using the forward/back buttons.
lastNavigation = pathname;
lastNavigationSearch = search;
const outlet = ref.current?.parentNode;
if (outlet && outlet !== container.parentNode) {
outlet.append(container);
Expand Down Expand Up @@ -433,7 +458,7 @@ export const createWebComponent = (tag: string, props?: Properties, onload?: ()
*
* @param routes optional routes are for adding own route definition, giving routes will skip FS routes
* @param serverSidePosition optional position where server routes should be put.
* If non given they go to the root of the routes [].
* If non given they go to the root of the routes [].
*
* @returns RouteObject[] with combined routes
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import com.vaadin.flow.server.MockServletServiceSessionSetup;
import com.vaadin.flow.server.VaadinService;
import com.vaadin.flow.server.VaadinSession;
import com.vaadin.flow.server.VaadinSessionState;

public class JavaScriptBootstrapUITest {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,8 @@ public MockServletServiceSessionSetup(boolean sessionAvailable)

Mockito.when(wrappedSession.getHttpSession())
.thenReturn(httpSession);
Mockito.when(session.getState())
.thenReturn(VaadinSessionState.OPEN);

Mockito.when(session.getService()).thenAnswer(i -> service);
Mockito.when(session.hasLock()).thenReturn(true);
Expand Down
8 changes: 8 additions & 0 deletions flow-tests/test-react-router/src/main/frontend/ReactView.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export default function ReactView() {

return (
<>
<p id="react">This is a simple view for a React route</p>
</>
);
}
12 changes: 12 additions & 0 deletions flow-tests/test-react-router/src/main/frontend/routes.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import ReactView from "Frontend/ReactView";
import { serverSideRoutes } from "Frontend/generated/flow/Flow";
import { createBrowserRouter, RouteObject } from "react-router-dom";


export const routes = [
{path: '/react', element: <ReactView/>, handle: { title: 'React Test View' }},
...serverSideRoutes
] as RouteObject[];

export default createBrowserRouter([...routes], { basename: new URL(document.baseURI).pathname });

Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ public class NavigationView extends Div {
public static final String ROUTER_LINK_ID = "router-link-navigation";
public static final String POSTPONE_ID = "postpone-view-link";

public static final String REACT_ANCHOR_ID = "anchor-react-navigation";
public static final String REACT_ID = "react-navigation";

public NavigationView() {
Anchor anchorNavigation = new Anchor("com.vaadin.flow.AnchorView",
"Navigate to AnchorView");
Expand All @@ -49,6 +52,18 @@ public NavigationView() {

add(new Span("NavigationView"), new Div(), anchorNavigation, new Div(),
serverNavigation, new Div(), link, new Div(), postponeView);

// React navigation
Anchor reactAnchorNavigation = new Anchor("react",
"Navigate to react with Anchor");
reactAnchorNavigation.setId(REACT_ANCHOR_ID);
NativeButton reactServerNavigation = new NativeButton(
"Navigate to react through Server", event -> {
event.getSource().getUI().get().navigate("react");
});
reactServerNavigation.setId(REACT_ID);

add(new Div(), reactAnchorNavigation, new Div(), reactServerNavigation);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import com.vaadin.flow.component.html.testbench.AnchorElement;
import com.vaadin.flow.component.html.testbench.NativeButtonElement;
import com.vaadin.flow.component.html.testbench.ParagraphElement;
import com.vaadin.flow.component.html.testbench.SpanElement;
import com.vaadin.flow.testutil.ChromeBrowserTest;

Expand Down Expand Up @@ -230,4 +231,49 @@ public void testNavigationBrowserHistoryBack_serverNavigation() {
Assert.assertEquals("NavigationView",
$(SpanElement.class).first().getText());
}

@Test
public void testreactNavigationBrowserHistoryBack_anchor() {
open();

Assert.assertEquals("NavigationView",
$(SpanElement.class).first().getText());

$(AnchorElement.class).id(NavigationView.REACT_ANCHOR_ID).click();
Assert.assertEquals("This is a simple view for a React route",
$(ParagraphElement.class).id("react").getText());
getDriver().navigate().back();
Assert.assertEquals("NavigationView",
$(SpanElement.class).first().getText());

$(AnchorElement.class).id(NavigationView.REACT_ANCHOR_ID).click();
Assert.assertEquals("This is a simple view for a React route",
$(ParagraphElement.class).id("react").getText());
getDriver().navigate().back();
Assert.assertEquals("NavigationView",
$(SpanElement.class).first().getText());
}

@Test
public void testReactNavigationBrowserHistoryBack_serverNavigation() {
open();

Assert.assertEquals("NavigationView",
$(SpanElement.class).first().getText());

$(NativeButtonElement.class).id(NavigationView.REACT_ID).click();
Assert.assertEquals("This is a simple view for a React route",
$(ParagraphElement.class).id("react").getText());
getDriver().navigate().back();
Assert.assertEquals("NavigationView",
$(SpanElement.class).first().getText());

$(NativeButtonElement.class).id(NavigationView.REACT_ID).click();
Assert.assertEquals("This is a simple view for a React route",
$(ParagraphElement.class).id("react").getText());
getDriver().navigate().back();
Assert.assertEquals("NavigationView",
$(SpanElement.class).first().getText());
}

}

0 comments on commit 5c58117

Please sign in to comment.