From 8b95318c6c700c946478567193cb0e3040c1dad2 Mon Sep 17 00:00:00 2001 From: Teemu Suo-Anttila Date: Wed, 19 Apr 2017 14:27:13 +0300 Subject: Fix issues in Grid with undefined height (#9104) --- .../java/com/vaadin/client/widgets/Escalator.java | 55 ++++++++++------ .../com/vaadin/v7/client/widgets/Escalator.java | 42 +++++++----- .../tests/components/grid/GridUndefinedHeight.java | 39 ++++++++++++ .../components/grid/GridUndefinedHeightTest.java | 74 ++++++++++++++++++++++ 4 files changed, 175 insertions(+), 35 deletions(-) create mode 100644 uitest/src/main/java/com/vaadin/tests/components/grid/GridUndefinedHeight.java create mode 100644 uitest/src/test/java/com/vaadin/tests/components/grid/GridUndefinedHeightTest.java 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 1f223c1d85..d869390f82 100644 --- a/client/src/main/java/com/vaadin/client/widgets/Escalator.java +++ b/client/src/main/java/com/vaadin/client/widgets/Escalator.java @@ -702,13 +702,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. @@ -729,29 +729,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); }); }-*/; @@ -1189,9 +1189,6 @@ public class Escalator extends Widget assertArgumentsAreValidAndWithinRange(index, numberOfRows); rows -= numberOfRows; - if (heightMode == HeightMode.UNDEFINED) { - heightByRows = rows; - } if (!isAttached()) { return; @@ -1207,8 +1204,9 @@ public class Escalator extends Widget * range of logical indices. This may be fewer than {@code numberOfRows} * , even zero, if not all the removed rows are actually visible. *

- * The implementation must call {@link #paintRemoveRow(TableRowElement, int)} - * for each row that is removed from the DOM. + * The implementation must call + * {@link #paintRemoveRow(TableRowElement, int)} for each row that is + * removed from the DOM. * * @param index * the logical index of the first removed row @@ -1315,10 +1313,6 @@ public class Escalator extends Widget } rows += numberOfRows; - if (heightMode == HeightMode.UNDEFINED) { - heightByRows = rows; - } - /* * only add items in the DOM if the widget itself is attached to the * DOM. We can't calculate sizes otherwise. @@ -2676,6 +2670,24 @@ public class Escalator extends Widget return (int) (rowPx / getDefaultRowHeight()); } + @Override + public void insertRows(int index, int numberOfRows) { + super.insertRows(index, numberOfRows); + + if (heightMode == HeightMode.UNDEFINED) { + setHeightByRows(getRowCount()); + } + } + + @Override + public void removeRows(int index, int numberOfRows) { + super.removeRows(index, numberOfRows); + + if (heightMode == HeightMode.UNDEFINED) { + setHeightByRows(getRowCount()); + } + } + @Override protected void paintInsertRows(final int index, final int numberOfRows) { @@ -3000,7 +3012,8 @@ public class Escalator extends Widget y += spacerContainer.getSpacerHeight(i); } - // Execute the registered callback function for newly created rows + // Execute the registered callback function for newly created + // rows Optional.ofNullable(newEscalatorRowCallback) .ifPresent(callback -> callback.accept(addedRows)); @@ -3131,7 +3144,8 @@ public class Escalator extends Widget } // Move rest of the rows to the Escalator's top - Range visualRange = Range.withLength(0, visualRowOrder.size()); + Range visualRange = Range.withLength(0, + visualRowOrder.size()); moveAndUpdateEscalatorRows(visualRange, 0, 0); sortDomElements(); @@ -6447,6 +6461,7 @@ public class Escalator extends Widget * @see #setHeightMode(HeightMode) */ public void setHeightByRows(double rows) throws IllegalArgumentException { + getLogger().warning("HeightByRows: " + rows); if (rows <= 0) { throw new IllegalArgumentException( "The number of rows must be a positive number."); diff --git a/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Escalator.java b/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Escalator.java index f022d691b7..4eea41c0ff 100644 --- a/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Escalator.java +++ b/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Escalator.java @@ -703,13 +703,13 @@ public class Escalator extends Widget /*-{ var vScroll = esc.@com.vaadin.v7.client.widgets.Escalator::verticalScrollbar; var vScrollElem = vScroll.@com.vaadin.v7.client.widget.escalator.ScrollbarBundle::getElement()(); - + var hScroll = esc.@com.vaadin.v7.client.widgets.Escalator::horizontalScrollbar; var hScrollElem = hScroll.@com.vaadin.v7.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. @@ -730,29 +730,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.v7.client.widgets.Escalator::body; deltaY *= brc.@com.vaadin.v7.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.v7.client.widgets.Escalator::logWarning(*)(msg); } - + // IE8 has only delta y if (isNaN(deltaY)) { deltaY = -0.5*e.wheelDelta; } - + @com.vaadin.v7.client.widgets.Escalator.JsniUtil::moveScrollFromEvent(*)(esc, deltaX, deltaY, e); }); }-*/; @@ -1208,9 +1208,6 @@ public class Escalator extends Widget assertArgumentsAreValidAndWithinRange(index, numberOfRows); rows -= numberOfRows; - if (heightMode == HeightMode.UNDEFINED) { - heightByRows = rows; - } if (!isAttached()) { return; @@ -1334,9 +1331,6 @@ public class Escalator extends Widget } rows += numberOfRows; - if (heightMode == HeightMode.UNDEFINED) { - heightByRows = rows; - } /* * only add items in the DOM if the widget itself is attached to the @@ -2513,6 +2507,24 @@ public class Escalator extends Widget super(bodyElement); } + @Override + public void insertRows(int index, int numberOfRows) { + super.insertRows(index, numberOfRows); + + if (heightMode == HeightMode.UNDEFINED) { + heightByRows = getRowCount(); + } + } + + @Override + public void removeRows(int index, int numberOfRows) { + super.removeRows(index, numberOfRows); + + if (heightMode == HeightMode.UNDEFINED) { + heightByRows = getRowCount(); + } + } + @Override public void setStylePrimaryName(String primaryStyleName) { super.setStylePrimaryName(primaryStyleName); diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridUndefinedHeight.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridUndefinedHeight.java new file mode 100644 index 0000000000..77480b3eef --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridUndefinedHeight.java @@ -0,0 +1,39 @@ +package com.vaadin.tests.components.grid; + +import com.vaadin.server.VaadinRequest; +import com.vaadin.shared.ui.grid.HeightMode; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Button; +import com.vaadin.ui.Grid; +import com.vaadin.ui.VerticalLayout; + +public class GridUndefinedHeight extends AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + VerticalLayout layout = new VerticalLayout(); + + Grid grid = new Grid<>(); + grid.setItems("Foo", "Bar", "Baz"); + grid.setHeightMode(HeightMode.UNDEFINED); + grid.addColumn(Object::toString).setCaption("toString()"); + + com.vaadin.v7.ui.Grid oldGrid = new com.vaadin.v7.ui.Grid(); + oldGrid.addColumn("toString", String.class); + oldGrid.addRow("Foo"); + oldGrid.addRow("Bar"); + oldGrid.addRow("Baz"); + oldGrid.setHeightMode( + com.vaadin.v7.shared.ui.grid.HeightMode.UNDEFINED); + + layout.addComponents(grid, oldGrid, new Button("Add header row", e -> { + grid.appendHeaderRow(); + oldGrid.appendHeaderRow(); + })); + layout.setHeight("600px"); + layout.setExpandRatio(grid, 1.0f); + layout.setExpandRatio(oldGrid, 1.0f); + addComponent(layout); + } + +} diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridUndefinedHeightTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridUndefinedHeightTest.java new file mode 100644 index 0000000000..07ccf6a95b --- /dev/null +++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridUndefinedHeightTest.java @@ -0,0 +1,74 @@ +package com.vaadin.tests.components.grid; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; + +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.testbench.elements.GridElement; +import com.vaadin.testbench.parallel.TestCategory; +import com.vaadin.tests.tb3.SingleBrowserTest; + +@TestCategory("grid") +public class GridUndefinedHeightTest extends SingleBrowserTest { + + @Before + public void before() { + setDebug(true); + openTestURL(); + } + + @Test + public void grid_undefined_height() { + GridElement grid = $(GridElement.class).first(); + int oneRow = grid.getRow(0).getSize().getHeight(); + int gridHeight = grid.getSize().getHeight(); + int rows = 4; // Header Row + 3 Body Rows + + Assert.assertEquals("Grid height mismatch", oneRow * rows, gridHeight); + + assertNoErrorNotifications(); + } + + @Test + public void gridv7_undefined_height() { + GridElement grid = $(GridElement.class).get(1); + int oneRow = grid.getRow(0).getSize().getHeight(); + int gridHeight = grid.getSize().getHeight(); + int rows = 4; // Header Row + 3 Body Rows + + Assert.assertEquals("Grid height mismatch", oneRow * rows, gridHeight); + + assertNoErrorNotifications(); + } + + @Test + public void grid_undefined_height_add_header() { + // Add header row to Grid + $(ButtonElement.class).first().click(); + + GridElement grid = $(GridElement.class).first(); + int oneRow = grid.getRow(0).getSize().getHeight(); + int gridHeight = grid.getSize().getHeight(); + int rows = 5; // 2 Header Rows + 3 Body Rows + + Assert.assertEquals("Grid height mismatch", oneRow * rows, gridHeight); + + assertNoErrorNotifications(); + } + + @Test + public void gridv7_undefined_height_add_header() { + // Add header row to Grid + $(ButtonElement.class).first().click(); + + GridElement grid = $(GridElement.class).get(1); + int oneRow = grid.getRow(0).getSize().getHeight(); + int gridHeight = grid.getSize().getHeight(); + int rows = 5; // 2 Header Rows + 3 Body Rows + + Assert.assertEquals("Grid height mismatch", oneRow * rows, gridHeight); + + assertNoErrorNotifications(); + } +} -- cgit v1.2.3