Skip to content

Commit

Permalink
Fix regression that broke PreserveOnRefresh functionality with Naviga…
Browse files Browse the repository at this point in the history
…tor (#11615)

* Fix regression that broke PreserveOnRefresh functionality with Navigator

Fixes #11614

Old patch #11416 calls navigateTo allways when Navigator is present, which is wrong, since it is needed only when navigation state has truly changed.

* Add test to the fix
  • Loading branch information
TatuLund authored and ZheSun88 committed Jun 12, 2019
1 parent 1c9594f commit 45279be
Show file tree
Hide file tree
Showing 3 changed files with 67 additions and 2 deletions.
15 changes: 13 additions & 2 deletions server/src/main/java/com/vaadin/ui/UI.java
Expand Up @@ -161,6 +161,12 @@ public abstract class UI extends AbstractSingleComponentContainer
private LoadingIndicatorConfiguration loadingIndicatorConfiguration = new LoadingIndicatorConfigurationImpl(
this);

/**
* Holder for old navigation state, needed in doRefresh in order not to call
* navigateTo too often
*/
private String oldNavigationState;

/**
* Scroll Y position.
*/
Expand Down Expand Up @@ -871,10 +877,15 @@ public void doRefresh(VaadinRequest request) {
page.updateBrowserWindowSize(newWidth, newHeight, true);

// Navigate if there is navigator, this is needed in case of
// PushStateNavigation
// PushStateNavigation. Call navigateTo only if state have
// truly changed
Navigator navigator = getNavigator();
if (navigator != null) {
navigator.navigateTo(navigator.getState());
if (oldNavigationState == null) oldNavigationState = getNavigator().getState();
if (!navigator.getState().equals(oldNavigationState)) {
navigator.navigateTo(navigator.getState());
oldNavigationState = navigator.getState();
}
}
}

Expand Down
29 changes: 29 additions & 0 deletions uitest/src/main/java/com/vaadin/tests/components/ui/RefreshUI.java
@@ -0,0 +1,29 @@
package com.vaadin.tests.components.ui;

import com.vaadin.annotations.PreserveOnRefresh;
import com.vaadin.navigator.Navigator;
import com.vaadin.navigator.View;
import com.vaadin.server.VaadinRequest;
import com.vaadin.tests.components.AbstractTestUI;
import com.vaadin.ui.Label;
import com.vaadin.ui.VerticalLayout;

@PreserveOnRefresh
public class RefreshUI extends AbstractTestUI {

@Override
protected void setup(VaadinRequest request) {
final Navigator navigator = new Navigator(this, this);
navigator.addView("", MyView.class);
setNavigator(navigator);
}

public static class MyView extends VerticalLayout implements View {
private static int instanceNumber = 0;

public MyView() {
instanceNumber++;
addComponent(new Label("This is instance no " + instanceNumber));
}
}
}
@@ -0,0 +1,25 @@
package com.vaadin.tests.components.ui;

import org.junit.Test;

import com.vaadin.testbench.elements.LabelElement;
import com.vaadin.tests.tb3.MultiBrowserTest;

import static org.junit.Assert.assertEquals;

public class RefreshUITest extends MultiBrowserTest {

@Test
public void testUIRefresh_viewNotRecreated() {
openTestURL();
assertEquals("The Label content is not matching",
"This is instance no 1",
$(LabelElement.class).first().getText());

// Reload the page; UI.refresh should be invoked
openTestURL();
assertEquals("The Label content is not matching",
"This is instance no 1",
$(LabelElement.class).first().getText());
}
}

0 comments on commit 45279be

Please sign in to comment.