From 806637b7d060e890f2f4d0c75f326896bc22b1f8 Mon Sep 17 00:00:00 2001 From: Artur Date: Mon, 6 Mar 2017 13:20:35 +0200 Subject: [PATCH] Fix exception when no columns are shown (#8733) * Fix exception when no columns are shown or all columns are frozen Fixes #8329 --- .../com/vaadin/client/widgets/Escalator.java | 26 ++++++++++------ .../grid/GridInitiallyHiddenColumns.java | 8 +++-- .../components/grid/InitialFrozenColumns.java | 31 ++++++++++++------- .../grid/GridInitiallyHiddenColumnsTest.java | 20 ++++++++++++ .../grid/InitialFrozenColumnsTest.java | 16 +++++++++- 5 files changed, 77 insertions(+), 24 deletions(-) rename uitest/src/main/java/com/vaadin/{v7 => }/tests/components/grid/InitialFrozenColumns.java (55%) rename uitest/src/test/java/com/vaadin/{v7 => }/tests/components/grid/InitialFrozenColumnsTest.java (73%) diff --git a/client/src/main/java/com/vaadin/client/widgets/Escalator.java b/client/src/main/java/com/vaadin/client/widgets/Escalator.java index 239ed9b90b..7eb99badf7 100644 --- a/client/src/main/java/com/vaadin/client/widgets/Escalator.java +++ b/client/src/main/java/com/vaadin/client/widgets/Escalator.java @@ -701,13 +701,13 @@ public class Escalator extends Widget /*-{ var vScroll = esc.@com.vaadin.client.widgets.Escalator::verticalScrollbar; var vScrollElem = vScroll.@com.vaadin.client.widget.escalator.ScrollbarBundle::getElement()(); - + var hScroll = esc.@com.vaadin.client.widgets.Escalator::horizontalScrollbar; var hScrollElem = hScroll.@com.vaadin.client.widget.escalator.ScrollbarBundle::getElement()(); - + return $entry(function(e) { var target = e.target; - + // in case the scroll event was native (i.e. scrollbars were dragged, or // the scrollTop/Left was manually modified), the bundles have old cache // values. We need to make sure that the caches are kept up to date. @@ -728,29 +728,29 @@ public class Escalator extends Widget return $entry(function(e) { var deltaX = e.deltaX ? e.deltaX : -0.5*e.wheelDeltaX; var deltaY = e.deltaY ? e.deltaY : -0.5*e.wheelDeltaY; - + // Delta mode 0 is in pixels; we don't need to do anything... - + // A delta mode of 1 means we're scrolling by lines instead of pixels // We need to scale the number of lines by the default line height if(e.deltaMode === 1) { var brc = esc.@com.vaadin.client.widgets.Escalator::body; deltaY *= brc.@com.vaadin.client.widgets.Escalator.AbstractRowContainer::getDefaultRowHeight()(); } - + // Other delta modes aren't supported if((e.deltaMode !== undefined) && (e.deltaMode >= 2 || e.deltaMode < 0)) { var msg = "Unsupported wheel delta mode \"" + e.deltaMode + "\""; - + // Print warning message esc.@com.vaadin.client.widgets.Escalator::logWarning(*)(msg); } - + // IE8 has only delta y if (isNaN(deltaY)) { deltaY = -0.5*e.wheelDelta; } - + @com.vaadin.client.widgets.Escalator.JsniUtil::moveScrollFromEvent(*)(esc, deltaX, deltaY, e); }); }-*/; @@ -4097,6 +4097,10 @@ public class Escalator extends Widget */ @Override public void removeColumns(final int index, final int numberOfColumns) { + if (numberOfColumns == 0) { + return; + } + // Validate assertArgumentsAreValidAndWithinRange(index, numberOfColumns); @@ -4225,6 +4229,10 @@ public class Escalator extends Widget */ @Override public void insertColumns(final int index, final int numberOfColumns) { + if (numberOfColumns == 0) { + return; + } + // Validate if (index < 0 || index > getColumnCount()) { throw new IndexOutOfBoundsException("The given index(" + index diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridInitiallyHiddenColumns.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridInitiallyHiddenColumns.java index 1c0a662abb..d040aa54f3 100644 --- a/uitest/src/main/java/com/vaadin/tests/components/grid/GridInitiallyHiddenColumns.java +++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridInitiallyHiddenColumns.java @@ -26,6 +26,7 @@ import com.vaadin.tests.data.bean.Person; import com.vaadin.tests.util.PortableRandom; import com.vaadin.tests.util.TestDataGenerator; import com.vaadin.ui.Grid; +import com.vaadin.ui.Grid.Column; import com.vaadin.ui.renderers.NumberRenderer; public class GridInitiallyHiddenColumns extends AbstractTestUIWithLog { @@ -36,8 +37,11 @@ public class GridInitiallyHiddenColumns extends AbstractTestUIWithLog { grid.addColumn(Person::getFirstName).setHidden(true).setHidable(true) .setCaption("First Name"); - grid.addColumn(Person::getLastName).setHidable(true) - .setCaption("Last Name"); + Column col2 = grid.addColumn(Person::getLastName) + .setHidable(true).setCaption("Last Name"); + if (request.getParameter("allHidden") != null) { + col2.setHidden(true); + } grid.addColumn(Person::getAge, new NumberRenderer()).setHidden(true) .setHidable(true).setCaption("Age"); diff --git a/uitest/src/main/java/com/vaadin/v7/tests/components/grid/InitialFrozenColumns.java b/uitest/src/main/java/com/vaadin/tests/components/grid/InitialFrozenColumns.java similarity index 55% rename from uitest/src/main/java/com/vaadin/v7/tests/components/grid/InitialFrozenColumns.java rename to uitest/src/main/java/com/vaadin/tests/components/grid/InitialFrozenColumns.java index 07a41337f2..ec49d24dc1 100644 --- a/uitest/src/main/java/com/vaadin/v7/tests/components/grid/InitialFrozenColumns.java +++ b/uitest/src/main/java/com/vaadin/tests/components/grid/InitialFrozenColumns.java @@ -13,27 +13,34 @@ * License for the specific language governing permissions and limitations under * the License. */ -package com.vaadin.v7.tests.components.grid; +package com.vaadin.tests.components.grid; import com.vaadin.server.VaadinRequest; import com.vaadin.tests.components.AbstractReindeerTestUI; -import com.vaadin.v7.ui.Grid; -import com.vaadin.v7.ui.Grid.SelectionMode; +import com.vaadin.tests.data.bean.Person; +import com.vaadin.tests.data.bean.Sex; +import com.vaadin.ui.Grid; +import com.vaadin.ui.Grid.SelectionMode; public class InitialFrozenColumns extends AbstractReindeerTestUI { @Override protected void setup(VaadinRequest request) { - Grid grid = new Grid(); + Grid grid = new Grid<>(Person.class); grid.setSelectionMode(SelectionMode.NONE); - - grid.addColumn("foo").setWidth(200); - grid.addColumn("bar").setWidth(200); - grid.addColumn("baz").setWidth(200); - - grid.addRow("a", "b", "c"); - - grid.setFrozenColumnCount(2); + grid.setColumns(); + grid.addColumn("firstName").setWidth(200); + grid.addColumn("lastName").setWidth(200); + grid.addColumn("email").setWidth(200); + + grid.setItems( + new Person("First", "last", "email", 242, Sex.UNKNOWN, null)); + + int frozen = 2; + if (request.getParameter("frozen") != null) { + frozen = Integer.parseInt(request.getParameter("frozen")); + } + grid.setFrozenColumnCount(frozen); addComponent(grid); } diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridInitiallyHiddenColumnsTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridInitiallyHiddenColumnsTest.java index 017ef14e2b..1a909e7a32 100644 --- a/uitest/src/test/java/com/vaadin/tests/components/grid/GridInitiallyHiddenColumnsTest.java +++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridInitiallyHiddenColumnsTest.java @@ -50,6 +50,26 @@ public class GridInitiallyHiddenColumnsTest extends SingleBrowserTest { } + @Test + public void ensureCorrectlyRenderedAllInitiallyHidden() { + openTestURL("debug&allHidden"); + GridElement grid = $(GridElement.class).first(); + + getSidebarOpenButton(grid).click(); + getColumnHidingToggle(grid, "First Name").click(); + getColumnHidingToggle(grid, "Last Name").click(); + getColumnHidingToggle(grid, "Age").click(); + getSidebarOpenButton(grid).click(); + + Assert.assertEquals("Umberto", grid.getCell(0, 0).getText()); + Assert.assertEquals("Rowling", grid.getCell(0, 1).getText()); + Assert.assertEquals("40", grid.getCell(0, 2).getText()); + Assert.assertEquals("Alex", grid.getCell(1, 0).getText()); + Assert.assertEquals("Barks", grid.getCell(1, 1).getText()); + Assert.assertEquals("25", grid.getCell(1, 2).getText()); + + } + // TODO: as to the getX methods reuse ones from GridBasicFeaturesTest? protected WebElement getSidebarOpenButton(GridElement grid) { diff --git a/uitest/src/test/java/com/vaadin/v7/tests/components/grid/InitialFrozenColumnsTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/InitialFrozenColumnsTest.java similarity index 73% rename from uitest/src/test/java/com/vaadin/v7/tests/components/grid/InitialFrozenColumnsTest.java rename to uitest/src/test/java/com/vaadin/tests/components/grid/InitialFrozenColumnsTest.java index fec0087168..58abe75731 100644 --- a/uitest/src/test/java/com/vaadin/v7/tests/components/grid/InitialFrozenColumnsTest.java +++ b/uitest/src/test/java/com/vaadin/tests/components/grid/InitialFrozenColumnsTest.java @@ -13,10 +13,12 @@ * License for the specific language governing permissions and limitations under * the License. */ -package com.vaadin.v7.tests.components.grid; +package com.vaadin.tests.components.grid; import static org.junit.Assert.assertTrue; +import java.util.logging.Level; + import org.junit.Assert; import org.junit.Test; import org.openqa.selenium.WebElement; @@ -39,4 +41,16 @@ public class InitialFrozenColumnsTest extends MultiBrowserTest { WebElement cell = $(GridElement.class).first().getCell(0, 0); assertTrue(cell.getAttribute("class").contains("frozen")); } + + @Test + public void testInitialAllColumnsFrozen() { + setDebug(true); + openTestURL("frozen=3"); + + Assert.assertFalse("Notification was present", + isElementPresent(NotificationElement.class)); + assertNoDebugMessage(Level.SEVERE); + WebElement cell = $(GridElement.class).first().getCell(0, 2); + assertTrue(cell.getAttribute("class").contains("frozen")); + } } -- 2.39.5