From bec7308ca3f71122d62f743ba7ac376a66b1b6b2 Mon Sep 17 00:00:00 2001 From: Artur Date: Mon, 13 Mar 2017 08:29:26 +0200 Subject: Fix detach of grid when it contains frozen columns (#8803) * Fix detach of grid when it contains frozen columns Fixes #8748 --- .../client/connectors/grid/ColumnConnector.java | 8 ++- .../vaadin/tests/components/grid/GridDetach.java | 32 --------- .../components/grid/GridRemoveColumnAndDetach.java | 61 ++++++++++++++++++ .../tests/components/grid/GridDetachTest.java | 34 ---------- .../grid/GridRemoveColumnAndDetachTest.java | 75 ++++++++++++++++++++++ 5 files changed, 142 insertions(+), 68 deletions(-) delete mode 100644 uitest/src/main/java/com/vaadin/tests/components/grid/GridDetach.java create mode 100644 uitest/src/main/java/com/vaadin/tests/components/grid/GridRemoveColumnAndDetach.java delete mode 100644 uitest/src/test/java/com/vaadin/tests/components/grid/GridDetachTest.java create mode 100644 uitest/src/test/java/com/vaadin/tests/components/grid/GridRemoveColumnAndDetachTest.java diff --git a/client/src/main/java/com/vaadin/client/connectors/grid/ColumnConnector.java b/client/src/main/java/com/vaadin/client/connectors/grid/ColumnConnector.java index 1b2574f4a8..d619b50b5d 100644 --- a/client/src/main/java/com/vaadin/client/connectors/grid/ColumnConnector.java +++ b/client/src/main/java/com/vaadin/client/connectors/grid/ColumnConnector.java @@ -140,8 +140,12 @@ public class ColumnConnector extends AbstractExtensionConnector { @Override public void onUnregister() { super.onUnregister(); - - parent.removeColumn(column); + if (parent.getParent() != null) { + // If the grid itself was unregistered there is no point in spending + // time to remove columns (and have problems with frozen columns) + // before throwing everything away + parent.removeColumn(column); + } column = null; } diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridDetach.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridDetach.java deleted file mode 100644 index 6c210d6cea..0000000000 --- a/uitest/src/main/java/com/vaadin/tests/components/grid/GridDetach.java +++ /dev/null @@ -1,32 +0,0 @@ -package com.vaadin.tests.components.grid; - -import com.vaadin.annotations.Widgetset; -import com.vaadin.server.VaadinRequest; -import com.vaadin.tests.components.AbstractTestUI; -import com.vaadin.tests.data.bean.Person; -import com.vaadin.ui.Button; -import com.vaadin.ui.Grid; - -/** - * This UI is the application entry point. A UI may either represent a browser - * window (or tab) or some part of a html page where a Vaadin application is - * embedded. - *

- * The UI is initialized using {@link #init(VaadinRequest)}. This method is - * intended to be overridden to add component to the user interface and - * initialize non-component functionality. - */ -@Widgetset("com.vaadin.DefaultWidgetSet") -public class GridDetach extends AbstractTestUI { - - @Override - protected void setup(VaadinRequest vaadinRequest) { - Grid grid = new Grid<>(); - grid.addColumn(Person::getFirstName).setCaption("asd"); - grid.addColumn(Person::getAge).setCaption("foobar"); - - addComponent(grid); - addComponent(new Button("Detach grid", e -> removeComponent(grid))); - } - -} diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridRemoveColumnAndDetach.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridRemoveColumnAndDetach.java new file mode 100644 index 0000000000..cc7d3c17ca --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridRemoveColumnAndDetach.java @@ -0,0 +1,61 @@ +package com.vaadin.tests.components.grid; + +import java.util.stream.Collectors; + +import com.vaadin.annotations.Widgetset; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUIWithLog; +import com.vaadin.tests.data.bean.Person; +import com.vaadin.tests.data.bean.Sex; +import com.vaadin.ui.Button; +import com.vaadin.ui.Grid; + +@Widgetset("com.vaadin.DefaultWidgetSet") +public class GridRemoveColumnAndDetach extends AbstractTestUIWithLog { + + private Grid grid; + + @Override + protected void setup(VaadinRequest vaadinRequest) { + grid = new Grid<>(); + grid.addColumn(Person::getFirstName).setCaption("First").setWidth(200); + grid.addColumn(Person::getLastName).setCaption("Last").setHidden(true) + .setWidth(200); + grid.addColumn(Person::getEmail).setCaption("Email").setWidth(200); + grid.addColumn(Person::getAge).setCaption("foobar").setWidth(400); + + grid.setItems(new Person("1", "2", "3", 4, Sex.FEMALE, null)); + grid.setFrozenColumnCount(3); + logFrozenColumns(); + addComponent(grid); + + Button detachButton = new Button("Detach grid", + e -> removeComponent(grid)); + detachButton.setId("detach"); + addComponent(detachButton); + for (int i = 0; i < 4; i++) { + final int idx = i; + Button button = new Button("Remove col " + i, e -> { + grid.removeColumn(grid.getColumns().get(idx)); + logFrozenColumns(); + }); + button.setId("remove" + i); + addComponent(button); + } + } + + private void logFrozenColumns() { + String msg = "Server side frozen columns: "; + msg += grid.getColumns().stream().limit(grid.getFrozenColumnCount()) + .map(column -> { + String caption = column.getCaption(); + if (column.isHidden()) { + caption += " (hidden)"; + } + return caption; + }).collect(Collectors.joining(", ")); + + log(msg); + } + +} diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridDetachTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridDetachTest.java deleted file mode 100644 index 4d5739acd2..0000000000 --- a/uitest/src/test/java/com/vaadin/tests/components/grid/GridDetachTest.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright 2000-2016 Vaadin Ltd. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not - * use this file except in compliance with the License. You may obtain a copy of - * the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations under - * the License. - */ -package com.vaadin.tests.components.grid; - -import org.junit.Test; - -import com.vaadin.testbench.By; -import com.vaadin.testbench.elements.ButtonElement; -import com.vaadin.tests.tb3.SingleBrowserTest; - -public class GridDetachTest extends SingleBrowserTest { - - @Test - public void gridDetachesWithoutErrors() { - openTestURL("debug"); - $(ButtonElement.class).first().click(); - - assertElementNotPresent(By.className("v-grid")); - assertNoErrorNotifications(); - } -} diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridRemoveColumnAndDetachTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridRemoveColumnAndDetachTest.java new file mode 100644 index 0000000000..930b1c7036 --- /dev/null +++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridRemoveColumnAndDetachTest.java @@ -0,0 +1,75 @@ +/* + * Copyright 2000-2016 Vaadin Ltd. + * + * Licensed under the Apache License, Version 2.0 (the "License"); you may not + * use this file except in compliance with the License. You may obtain a copy of + * the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations under + * the License. + */ +package com.vaadin.tests.components.grid; + +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +import com.vaadin.testbench.By; +import com.vaadin.testbench.TestBenchElement; +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.testbench.elements.GridElement; +import com.vaadin.tests.tb3.SingleBrowserTest; + +public class GridRemoveColumnAndDetachTest extends SingleBrowserTest { + + @Test + public void gridDetachesWithoutErrors() { + openTestURL("debug"); + $(ButtonElement.class).id("detach").click(); + + assertElementNotPresent(By.className("v-grid")); + assertNoErrorNotifications(); + } + + @Test + public void frozenColumnCountAfterRemovingHiddenColumn() { + openTestURL("debug"); + assertVisibleFrozenColumns(2); + $(ButtonElement.class).id("remove1").click(); + assertVisibleFrozenColumns(2); + + } + + @Test + public void frozenColumnCountAfterWhenRemovingFrozenColumn() { + openTestURL("debug"); + assertVisibleFrozenColumns(2); + $(ButtonElement.class).id("remove0").click(); + assertVisibleFrozenColumns(1); + + } + + private void assertVisibleFrozenColumns(int nrFrozenColumns) { + GridElement grid = $(GridElement.class).first(); + for (int i = 0; i < nrFrozenColumns; i++) { + TestBenchElement cell = grid.getCell(0, i); + assertTrue("Column " + i + " should be frozen", + cell.hasClassName("frozen")); + } + assertTrue("Only " + nrFrozenColumns + " should be frozen", grid + .getCell(0, nrFrozenColumns - 1).hasClassName("last-frozen")); + } + + @Test + public void frozenColumnCountAfterWhenRemovingNonFrozenColumn() { + openTestURL("debug"); + assertVisibleFrozenColumns(2); + $(ButtonElement.class).id("remove3").click(); + assertVisibleFrozenColumns(2); + } +} -- cgit v1.2.3