From 43c43cfe883edf776657a787332d43aa9482f6bb Mon Sep 17 00:00:00 2001 From: Henrik Paul Date: Fri, 17 Oct 2014 17:16:45 +0300 Subject: [PATCH] Fixes an issue with onUnload() (#13334) Change-Id: I7ed22d97ee861208822f845068b7eee856392c07 --- .../com/vaadin/client/ui/grid/Escalator.java | 35 ++++++++--- .../EscalatorBasicClientFeaturesTest.java | 8 ++- .../basicfeatures/EscalatorBasicsTest.java | 61 +++++++++++++++++++ .../EscalatorBasicClientFeaturesWidget.java | 17 ++++++ 4 files changed, 110 insertions(+), 11 deletions(-) create mode 100644 uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicsTest.java diff --git a/client/src/com/vaadin/client/ui/grid/Escalator.java b/client/src/com/vaadin/client/ui/grid/Escalator.java index be831b5b61..c054331c00 100644 --- a/client/src/com/vaadin/client/ui/grid/Escalator.java +++ b/client/src/com/vaadin/client/ui/grid/Escalator.java @@ -2646,10 +2646,7 @@ public class Escalator extends Widget implements RequiresResize, DeferredWorker return; } - final Range viewportRange = Range.withLength( - getLogicalRowIndex(visualRowOrder.getFirst()), - visualRowOrder.size()); - + final Range viewportRange = getVisibleRowRange(); final Range removedRowsRange = Range .withLength(index, numberOfRows); @@ -2721,12 +2718,12 @@ public class Escalator extends Widget implements RequiresResize, DeferredWorker int escalatorRowCount = bodyElem.getChildCount(); /* - * If we're left with less rows than the number of escalators, - * remove the unused ones. + * remember: the rows have already been subtracted from the row + * count at this point */ - final int escalatorRowsToRemove = escalatorRowCount - - getRowCount(); - if (escalatorRowsToRemove > 0) { + int rowsLeft = getRowCount(); + if (rowsLeft < escalatorRowCount) { + int escalatorRowsToRemove = escalatorRowCount - rowsLeft; for (int i = 0; i < escalatorRowsToRemove; i++) { final TableRowElement tr = visualRowOrder .remove(removedVisualInside.getStart()); @@ -4056,9 +4053,27 @@ public class Escalator extends Widget implements RequiresResize, DeferredWorker scroller.detachMousewheelListener(getElement()); scroller.detachTouchListeners(getElement()); + /* + * We can call paintRemoveRows here, because static ranges are simple to + * remove. + */ header.paintRemoveRows(0, header.getRowCount()); footer.paintRemoveRows(0, footer.getRowCount()); - body.paintRemoveRows(0, body.getRowCount()); + + /* + * We can't call body.paintRemoveRows since it relies on rowCount to be + * updated correctly. Since it isn't, we'll simply and brutally rip out + * the DOM elements (in an elegant way, of course). + */ + int rowsToRemove = bodyElem.getChildCount(); + for (int i = 0; i < rowsToRemove; i++) { + int index = rowsToRemove - i - 1; + TableRowElement tr = bodyElem.getRows().getItem(index); + body.paintRemoveRow(tr, index); + body.removeRowPosition(tr); + } + body.visualRowOrder.clear(); + body.setTopRowLogicalIndex(0); super.onUnload(); } diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java index 8f29a71536..0c58b01062 100644 --- a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicClientFeaturesTest.java @@ -38,6 +38,7 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest protected static final String ADD_ONE_ROW_TO_BEGINNING = "Add one row to beginning"; protected static final String REMOVE_ONE_COLUMN_FROM_BEGINNING = "Remove one column from beginning"; protected static final String REMOVE_ONE_ROW_FROM_BEGINNING = "Remove one row from beginning"; + protected static final String ADD_ONE_OF_EACH_ROW = "Add one of each row"; protected static final String HEADER_ROWS = "Header Rows"; protected static final String BODY_ROWS = "Body Rows"; @@ -46,6 +47,7 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest protected static final String REMOVE_ALL_INSERT_SCROLL = "Remove all, insert 30 and scroll 40px"; protected static final String GENERAL = "General"; + protected static final String DETACH_ESCALATOR = "Detach Escalator"; protected static final String POPULATE_COLUMN_ROW = "Populate Escalator (columns, then rows)"; protected static final String POPULATE_ROW_COLUMN = "Populate Escalator (rows, then columns)"; protected static final String CLEAR_COLUMN_ROW = "Clear (columns, then rows)"; @@ -65,7 +67,11 @@ public abstract class EscalatorBasicClientFeaturesTest extends MultiBrowserTest } protected WebElement getEscalator() { - return getDriver().findElement(By.className("v-escalator")); + try { + return getDriver().findElement(By.className("v-escalator")); + } catch (NoSuchElementException e) { + return null; + } } protected WebElement getHeaderRow(int row) { diff --git a/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicsTest.java b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicsTest.java new file mode 100644 index 0000000000..bac5c24fbe --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/basicfeatures/EscalatorBasicsTest.java @@ -0,0 +1,61 @@ +/* + * Copyright 2000-2014 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.basicfeatures; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; + +import org.junit.Test; + +import com.vaadin.testbench.elements.NotificationElement; + +public class EscalatorBasicsTest extends EscalatorBasicClientFeaturesTest { + + @Test + public void testDetachingAnEmptyEscalator() { + setDebug(true); + openTestURL(); + + selectMenuPath(GENERAL, DETACH_ESCALATOR); + assertEscalatorIsRemovedCorrectly(); + } + + @Test + public void testDetachingASemiPopulatedEscalator() { + setDebug(true); + openTestURL(); + + selectMenuPath(COLUMNS_AND_ROWS, ADD_ONE_OF_EACH_ROW); + selectMenuPath(COLUMNS_AND_ROWS, COLUMNS, ADD_ONE_COLUMN_TO_BEGINNING); + selectMenuPath(GENERAL, DETACH_ESCALATOR); + assertEscalatorIsRemovedCorrectly(); + } + + @Test + public void testDetachingAPopulatedEscalator() { + setDebug(true); + openTestURL(); + + selectMenuPath(GENERAL, POPULATE_COLUMN_ROW); + selectMenuPath(GENERAL, DETACH_ESCALATOR); + assertEscalatorIsRemovedCorrectly(); + } + + private void assertEscalatorIsRemovedCorrectly() { + assertFalse($(NotificationElement.class).exists()); + assertNull(getEscalator()); + } +} diff --git a/uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java b/uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java index 987dcc1bb7..60b9d5cc7d 100644 --- a/uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java +++ b/uitest/src/com/vaadin/tests/widgetset/client/grid/EscalatorBasicClientFeaturesWidget.java @@ -368,6 +368,23 @@ public class EscalatorBasicClientFeaturesWidget extends private void createGeneralMenu() { String[] menupath = { GENERAL_MENU }; + + addMenuCommand("Detach Escalator", new ScheduledCommand() { + @Override + public void execute() { + escalator.removeFromParent(); + } + }, menupath); + + addMenuCommand("Attach Escalator", new ScheduledCommand() { + @Override + public void execute() { + if (!escalator.isAttached()) { + addNorth(escalator, 500); + } + } + }, menupath); + addMenuCommand("Clear (columns, then rows)", new ScheduledCommand() { @Override public void execute() { -- 2.39.5