From 5bd1f5df892236476b1ba3043c3e14dac698a660 Mon Sep 17 00:00:00 2001 From: Vaadin Bot Date: Mon, 3 May 2021 17:10:00 +0300 Subject: [PATCH] fix: Ensure editor closing when page refreshed while using PreserveOnRefresh (#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 --- .../grid/it/PreserveOnRefreshPage.java | 49 ++++++++++++++++++- .../grid/it/PreserveOnRefreshIT.java | 15 ++++++ .../com/vaadin/flow/component/grid/Grid.java | 6 ++- 3 files changed, 67 insertions(+), 3 deletions(-) diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/grid/it/PreserveOnRefreshPage.java b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/grid/it/PreserveOnRefreshPage.java index f11ad62525f..e81c7387d7b 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/grid/it/PreserveOnRefreshPage.java +++ b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/main/java/com/vaadin/flow/component/grid/it/PreserveOnRefreshPage.java @@ -15,10 +15,15 @@ */ 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; @@ -26,12 +31,52 @@ @PreserveOnRefresh public class PreserveOnRefreshPage extends Div { + int count = 0; + public PreserveOnRefreshPage() { Grid 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 firstNameColumn = grid.addColumn(Person::getFirstName) + .setHeader("First Name"); + + // define editor & binder for editor + Binder binder = new Binder<>(Person.class); + Editor 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/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/PreserveOnRefreshIT.java b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/PreserveOnRefreshIT.java index a68633a23ef..d5dbe8c5f6d 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/PreserveOnRefreshIT.java +++ b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-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("footer")); } + @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/vaadin-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/grid/Grid.java b/vaadin-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/grid/Grid.java index b07aa7c8587..f16085c983c 100755 --- a/vaadin-grid-flow-parent/vaadin-grid-flow/src/main/java/com/vaadin/flow/component/grid/Grid.java +++ b/vaadin-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 editor = getEditor(); if (editor != null) { - getEditor().closeEditor(); + if (getEditor().isBuffered()) { + getEditor().cancel(); + } else { + getEditor().closeEditor(); + } } } finally { editorFactory = factory;