diff --git a/flow-client/src/main/frontend/Flow.ts b/flow-client/src/main/frontend/Flow.ts index 0af39f19f73..479e5d1516b 100644 --- a/flow-client/src/main/frontend/Flow.ts +++ b/flow-client/src/main/frontend/Flow.ts @@ -275,17 +275,19 @@ export class Flow { const tag = `flow-container-${appId.toLowerCase()}`; this.container = flowRoot.$[appId] = document.createElement(tag); this.container.id = appId; - - // It might be that components created from server expect that their content has been rendered. - // Appending eagerly the container we avoid these kind of errors. - // Note that the client router will move this container to the outlet if the navigation succeed - this.container.style.display = 'none'; - document.body.appendChild(this.container); } // hide flow progress indicator this.loadingFinished(); } + + // It might be that components created from server expect that their content has been rendered. + // Appending eagerly the container we avoid these kind of errors. + // Note that the client router will move this container to the outlet if the navigation succeed + if (this.container && !this.container.isConnected) { + this.container.style.display = 'none'; + document.body.appendChild(this.container); + } return this.response!; } diff --git a/flow-client/src/test/frontend/FlowTests.ts b/flow-client/src/test/frontend/FlowTests.ts index 8511b7fcbf4..07683620303 100644 --- a/flow-client/src/test/frontend/FlowTests.ts +++ b/flow-client/src/test/frontend/FlowTests.ts @@ -5,7 +5,11 @@ const { assert } = intern.getPlugin('chai'); const { sinon } = intern.getPlugin('sinon') as { sinon: SinonStatic }; // API to test -import { Flow, NavigationParameters } from '../../main/frontend/Flow'; +import { + Flow, + NavigationParameters, + PreventAndRedirectCommands +} from '../../main/frontend/Flow'; import { ConnectionState, ConnectionStateStore } from '@vaadin/common-frontend'; // Intern does not serve webpack chunks, adding deps here in order to // produce one chunk, because dynamic imports in Flow.ts will not work. @@ -763,6 +767,43 @@ suite('Flow', () => { $wnd.dispatchEvent(new Event('online')); // caught by DefaultConnectionStateHandler assert.equal($wnd.Vaadin.connectionState.state, ConnectionState.RECONNECTING); }); + + test("should pre-attach container element on every navigation", async () => { + stubServerRemoteFunction('foobar-12345'); + mockInitResponse('foobar-12345'); + + const flow = new Flow(); + const route = flow.serverSideRoutes[0]; + + const flowRouteParams = {pathname: 'Foo', search: ''}; + const otherRouteParams = {pathname: 'Lorem', search: ''}; + + // Initial navigation + const container = await route.action(flowRouteParams); + assert.isTrue(container.isConnected); + assert.equal(container.style.display, 'none'); + + // @ts-ignore + await container.onBeforeEnter(flowRouteParams, {}); + assert.isTrue(container.isConnected); + assert.equal(container.style.display, ''); + + // Leave + + // @ts-ignore + await container.onBeforeLeave(otherRouteParams, {}); + // The router detaches the container, possibly because it renders a client-side view + container.parentNode!.removeChild(container); + + await route.action(flowRouteParams); + assert.isTrue(container.isConnected); + assert.equal(container.style.display, 'none'); + + // @ts-ignore + await container.onBeforeEnter(flowRouteParams, {}); + assert.isTrue(container.isConnected); + assert.equal(container.style.display, ''); + }); }); function stubServerRemoteFunction( diff --git a/flow-tests/test-ccdm-flow-navigation/src/main/java/com/vaadin/flow/navigate/HelloWorldView.java b/flow-tests/test-ccdm-flow-navigation/src/main/java/com/vaadin/flow/navigate/HelloWorldView.java index 3bef40ccb6b..a43d7bbc39e 100644 --- a/flow-tests/test-ccdm-flow-navigation/src/main/java/com/vaadin/flow/navigate/HelloWorldView.java +++ b/flow-tests/test-ccdm-flow-navigation/src/main/java/com/vaadin/flow/navigate/HelloWorldView.java @@ -16,16 +16,22 @@ package com.vaadin.flow.navigate; +import com.vaadin.flow.component.Text; +import com.vaadin.flow.component.html.NativeButton; +import com.vaadin.flow.component.html.Paragraph; import com.vaadin.flow.component.html.Span; import com.vaadin.flow.router.PageTitle; import com.vaadin.flow.router.Route; -import com.vaadin.flow.component.html.NativeButton; @Route(value = "hello") @PageTitle("Hello World") public class HelloWorldView extends Span { - public static final String NAVIGATE_ABOUT = "navigate-about"; + public static final String IS_CONNECTED_ON_INIT = "is-connected-on-init"; + public static final String IS_CONNECTED_ON_ATTACH = "is-connected-on-attach"; + + private final Span isConnectedOnInit = new Span(""); + private final Span isConnectedOnAttach = new Span(""); public HelloWorldView() { setId("hello-world-view"); @@ -33,6 +39,20 @@ public HelloWorldView() { e -> getUI().get().navigate("about")); toAbout.setId(NAVIGATE_ABOUT); add(toAbout); + + isConnectedOnInit.setId(IS_CONNECTED_ON_INIT); + updateIsConnected(isConnectedOnInit); + add(new Paragraph(new Text("Connected on init: "), isConnectedOnInit)); + + isConnectedOnAttach.setId(IS_CONNECTED_ON_ATTACH); + isConnectedOnAttach.addAttachListener( + event -> updateIsConnected(isConnectedOnAttach)); + add(new Paragraph(new Text("Connected on attach: "), + isConnectedOnAttach)); } + private void updateIsConnected(Span output) { + output.getElement() + .executeJs("this.textContent=String(this.isConnected)"); + } } diff --git a/flow-tests/test-ccdm-flow-navigation/src/test/java/com/vaadin/flow/navigate/NavigateBetweenViewsIT.java b/flow-tests/test-ccdm-flow-navigation/src/test/java/com/vaadin/flow/navigate/NavigateBetweenViewsIT.java index abf1f770f7d..2cd948c5e6c 100644 --- a/flow-tests/test-ccdm-flow-navigation/src/test/java/com/vaadin/flow/navigate/NavigateBetweenViewsIT.java +++ b/flow-tests/test-ccdm-flow-navigation/src/test/java/com/vaadin/flow/navigate/NavigateBetweenViewsIT.java @@ -21,8 +21,11 @@ import org.openqa.selenium.By; import com.vaadin.flow.component.html.testbench.NativeButtonElement; +import com.vaadin.flow.component.html.testbench.SpanElement; import com.vaadin.flow.testutil.ChromeBrowserTest; +import static com.vaadin.flow.navigate.HelloWorldView.IS_CONNECTED_ON_ATTACH; +import static com.vaadin.flow.navigate.HelloWorldView.IS_CONNECTED_ON_INIT; import static com.vaadin.flow.navigate.HelloWorldView.NAVIGATE_ABOUT; public class NavigateBetweenViewsIT extends ChromeBrowserTest { @@ -70,8 +73,55 @@ public void openTsView_navigateToFlow_navigationSuccessful() { $(NativeButtonElement.class).id(NAVIGATE_ABOUT).isDisplayed()); } + @Test + public void openFlowView_isConnectedOnAttach() { + getDriver().get(getRootURL() + "/hello"); + waitForDevServer(); + + Assert.assertThat(getDriver().getCurrentUrl(), + CoreMatchers.endsWith("/hello")); + + assertIsConnected(); + + // Navigate away and back + $(NativeButtonElement.class).id(NAVIGATE_ABOUT).click(); + waitUntil(input -> $("about-view").first().$("a").id("navigate-hello") + .isDisplayed()); + + getInShadowRoot(findElement(By.tagName("about-view")), + By.id("navigate-hello")).click(); + + assertIsConnected(); + } + + @Test + public void openTsView_navigateToFlowView_isConnectedOnAttach() { + getDriver().get(getRootURL() + "/"); + waitForDevServer(); + + waitUntil(input -> $("about-view").first().$("a").id("navigate-hello") + .isDisplayed()); + + getInShadowRoot(findElement(By.tagName("about-view")), + By.id("navigate-hello")).click(); + + getCommandExecutor().waitForVaadin(); + + assertIsConnected(); + } + @Override protected String getRootURL() { return super.getRootURL() + "/context-path"; } + + private void assertIsConnected() { + assertIsConnectedById(IS_CONNECTED_ON_INIT); + assertIsConnectedById(IS_CONNECTED_ON_ATTACH); + } + + private void assertIsConnectedById(String id) { + Assert.assertTrue(Boolean.parseBoolean( + waitUntil(driver -> $(SpanElement.class).id(id)).getText())); + } }