fix: Ensure editor closing when page refreshed while using PreserveOn…

…Refresh (#865) (CP: 20.0) (#890) If Editor is not closed on detach and preserve on refresh is used, the ghost editor will cause an exception. fixes: vaadin/vaadin-grid#2176 Co-authored-by: Tatu Lund <[email protected]>
vaadin · May 3, 2021 · 5bd1f5d · 5bd1f5d
1 parent af8f4d7
commit 5bd1f5d
Show file tree

Hide file tree

Showing 3 changed files with 67 additions and 3 deletions.
diff --git a/...egration-tests/src/main/java/com/vaadin/flow/component/grid/it/PreserveOnRefreshPage.java b/...egration-tests/src/main/java/com/vaadin/flow/component/grid/it/PreserveOnRefreshPage.java
@@ -15,23 +15,68 @@
  */
 package com.vaadin.flow.component.grid.it;
 
+import com.vaadin.flow.component.button.Button;
 import com.vaadin.flow.component.grid.Grid;
+import com.vaadin.flow.component.grid.Grid.Column;
+import com.vaadin.flow.component.grid.editor.Editor;
 import com.vaadin.flow.component.html.Div;
 import com.vaadin.flow.component.html.Span;
+import com.vaadin.flow.component.textfield.TextField;
 import com.vaadin.flow.data.bean.Person;
+import com.vaadin.flow.data.binder.Binder;
 import com.vaadin.flow.router.PreserveOnRefresh;
 import com.vaadin.flow.router.Route;
 
 @Route("vaadin-grid/preserve-on-refresh")
 @PreserveOnRefresh
 public class PreserveOnRefreshPage extends Div {
 
+    int count = 0;
+
     public PreserveOnRefreshPage() {
         Grid<Person> grid = new Grid<>();
-        grid.setItems(new Person("foo", 20));
+        Person foo = new Person("foo", 20);
+        grid.setItems(foo);
         grid.addComponentColumn(person -> new Span(person.getFirstName()))
                 .setHeader(new Span("header")).setFooter(new Span("footer"));
-        add(grid);
+
+        // Add editable column
+        Column<Person> firstNameColumn = grid.addColumn(Person::getFirstName)
+                .setHeader("First Name");
+
+        // define editor & binder for editor
+        Binder<Person> binder = new Binder<>(Person.class);
+        Editor<Person> editor = grid.getEditor();
+        editor.setBinder(binder);
+        editor.setBuffered(true);
+
+        // define editor components for columns
+        TextField firstNameField = new TextField();
+        binder.bind(firstNameField, Person::getFirstName, Person::setFirstName);
+        firstNameColumn.setEditorComponent(firstNameField);
+
+        Button button = new Button("Edit");
+        button.setId("edit-button");
+        button.addClickListener(event -> {
+            grid.getEditor().editItem(foo);
+        });
+
+        // Grid editor will fire close event, this will be tested
+        grid.getEditor().addCloseListener(event -> {
+            Span close = new Span("Closed");
+            close.setId("closed");
+            add(close);
+        });
+
+        // Grid editor will fire open event, this will be tested after
+        // refresh
+        grid.getEditor().addOpenListener(event -> {
+            count++;
+            Span open = new Span("Open: " + count);
+            open.setId("open-" + count);
+            add(open);
+        });
+        add(grid, button);
     }
 
 }
diff --git a/...ntegration-tests/src/test/java/com/vaadin/flow/component/grid/it/PreserveOnRefreshIT.java b/...ntegration-tests/src/test/java/com/vaadin/flow/component/grid/it/PreserveOnRefreshIT.java
@@ -23,6 +23,8 @@
 import com.vaadin.flow.component.grid.testbench.GridElement;
 import com.vaadin.tests.AbstractComponentIT;
 import com.vaadin.flow.testutil.TestPath;
+import org.openqa.selenium.By;
+import org.openqa.selenium.WebElement;
 
 @TestPath("vaadin-grid/preserve-on-refresh")
 public class PreserveOnRefreshIT extends AbstractComponentIT {
@@ -59,6 +61,19 @@ public void refresh_footerComponentRendered() {
                 CoreMatchers.containsString("<span>footer</span>"));
     }
 
+    @Test
+    public void refresh_editorOpen() {
+        findElement(By.id("edit-button")).click();
+        getDriver().navigate().refresh();
+        WebElement closed = findElement(By.id("closed"));
+        Assert.assertEquals(closed.getText(), "Closed");
+
+        // Test that editor still works after refresh
+        findElement(By.id("edit-button")).click();
+        WebElement open = findElement(By.id("open-2"));
+        Assert.assertEquals(open.getText(), "Open: 2");
+    }
+
     private GridElement getGrid() {
         return $(GridElement.class).first();
     }

diff --git a/...-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/grid/Grid.java b/...-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/grid/Grid.java
@@ -3719,7 +3719,11 @@ protected void onDataProviderChange() {
         try {
             Editor<T> editor = getEditor();
             if (editor != null) {
-                getEditor().closeEditor();
+                if (getEditor().isBuffered()) {
+                    getEditor().cancel();
+                } else {
+                    getEditor().closeEditor();
+                }
             }
         } finally {
             editorFactory = factory;