diff options
12 files changed, 337 insertions, 21 deletions
diff --git a/client/src/main/java/com/vaadin/client/widget/escalator/RowContainer.java b/client/src/main/java/com/vaadin/client/widget/escalator/RowContainer.java index 26a2f294b7..7ef6ff6bbf 100644 --- a/client/src/main/java/com/vaadin/client/widget/escalator/RowContainer.java +++ b/client/src/main/java/com/vaadin/client/widget/escalator/RowContainer.java @@ -236,6 +236,14 @@ public interface RowContainer { public int getRowCount(); /** + * For internal use only. May be removed or replaced in the future. + * + * @since + * @return {@code true} if row height calculations have been scheduled + */ + public boolean isAutodetectingRowHeightLater(); + + /** * The default height of the rows in this RowContainer. * * @param px 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 a3ecfab902..393708f7da 100644 --- a/client/src/main/java/com/vaadin/client/widgets/Escalator.java +++ b/client/src/main/java/com/vaadin/client/widgets/Escalator.java @@ -1274,6 +1274,8 @@ public class Escalator extends Widget private boolean initialColumnSizesCalculated = false; + private boolean autodetectingRowHeightLater = false; + public AbstractRowContainer( final TableSectionElement rowContainerElement) { root = rowContainerElement; @@ -2115,14 +2117,21 @@ public class Escalator extends Widget } public void autodetectRowHeightLater() { + autodetectingRowHeightLater = true; Scheduler.get().scheduleFinally(() -> { if (defaultRowHeightShouldBeAutodetected && isAttached()) { autodetectRowHeightNow(); defaultRowHeightShouldBeAutodetected = false; } + autodetectingRowHeightLater = false; }); } + @Override + public boolean isAutodetectingRowHeightLater() { + return autodetectingRowHeightLater; + } + private void fireRowHeightChangedEventFinally() { if (!rowHeightChangedEventFired) { rowHeightChangedEventFired = true; diff --git a/client/src/main/java/com/vaadin/client/widgets/Grid.java b/client/src/main/java/com/vaadin/client/widgets/Grid.java index e6efe4cbc0..f42312c1d6 100755 --- a/client/src/main/java/com/vaadin/client/widgets/Grid.java +++ b/client/src/main/java/com/vaadin/client/widgets/Grid.java @@ -76,10 +76,7 @@ import com.google.gwt.user.client.ui.MenuItem; import com.google.gwt.user.client.ui.PopupPanel; import com.google.gwt.user.client.ui.ResizeComposite; import com.google.gwt.user.client.ui.Widget; -import com.vaadin.client.BrowserInfo; -import com.vaadin.client.DeferredWorker; -import com.vaadin.client.Focusable; -import com.vaadin.client.WidgetUtil; +import com.vaadin.client.*; import com.vaadin.client.WidgetUtil.Reference; import com.vaadin.client.data.DataChangeHandler; import com.vaadin.client.data.DataSource; @@ -3342,6 +3339,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>, */ private class AutoColumnWidthsRecalculator { private double lastCalculatedInnerWidth = -1; + private double lastCalculatedInnerHeight = -1; private final ScheduledCommand calculateCommand = new ScheduledCommand() { @@ -3436,6 +3434,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>, // Update latest width to prevent recalculate on height change. lastCalculatedInnerWidth = escalator.getInnerWidth(); + lastCalculatedInnerHeight = getEscalatorInnerHeight(); } private boolean columnsAreGuaranteedToBeWiderThanGrid() { @@ -9148,6 +9147,18 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>, recalculateColumnWidths(); } + + if (getEscalatorInnerHeight() != autoColumnWidthsRecalculator.lastCalculatedInnerHeight) { + Scheduler.get().scheduleFinally(() -> { + // Trigger re-calculation of all row positions. + RowContainer.BodyRowContainer body = getEscalator() + .getBody(); + if (!body.isAutodetectingRowHeightLater()) { + body.setDefaultRowHeight(body.getDefaultRowHeight()); + } + }); + } + // Vertical resizing could make editor positioning invalid so it // needs to be recalculated on resize if (isEditorActive()) { @@ -9161,6 +9172,11 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>, }); } + private double getEscalatorInnerHeight() { + return new ComputedStyle(getEscalator().getTableWrapper()) + .getHeightIncludingBorderPadding(); + } + /** * Grid does not support adding Widgets this way. * <p> diff --git a/compatibility-client/src/main/java/com/vaadin/v7/client/widget/escalator/RowContainer.java b/compatibility-client/src/main/java/com/vaadin/v7/client/widget/escalator/RowContainer.java index 5e3ac96efd..c9c5dfb903 100644 --- a/compatibility-client/src/main/java/com/vaadin/v7/client/widget/escalator/RowContainer.java +++ b/compatibility-client/src/main/java/com/vaadin/v7/client/widget/escalator/RowContainer.java @@ -221,6 +221,22 @@ public interface RowContainer { public int getRowCount(); /** + * This method calculates the current row count directly from the DOM. + * <p> + * While the container is stable, this value should equal to + * {@link #getRowCount()}, but while row counts are being updated, these two + * values might differ for a short while. + * <p> + * Any extra content, such as spacers for the body, should not be included + * in this count. + * + * @since + * + * @return the actual DOM count of rows + */ + public int getDomRowCount(); + + /** * The default height of the rows in this RowContainer. * * @param px 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 70689b2482..e50682f2c1 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 @@ -1291,22 +1291,6 @@ public class Escalator extends Widget } /** - * This method calculates the current row count directly from the DOM. - * <p> - * While Escalator is stable, this value should equal to - * {@link #getRowCount()}, but while row counts are being updated, these - * two values might differ for a short while. - * <p> - * Any extra content, such as spacers for the body, should not be - * included in this count. - * - * @since 7.5.0 - * - * @return the actual DOM count of rows - */ - public abstract int getDomRowCount(); - - /** * {@inheritDoc} * <p> * <em>Implementation detail:</em> This method does no DOM modifications diff --git a/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Grid.java b/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Grid.java index 5ab014d413..8af5818eb1 100644 --- a/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Grid.java +++ b/compatibility-client/src/main/java/com/vaadin/v7/client/widgets/Grid.java @@ -3936,7 +3936,7 @@ public class Grid<T> extends ResizeComposite implements HasSelectionHandlers<T>, private void setHeightToHeaderCellHeight() { RowContainer header = grid.escalator.getHeader(); - if (header.getRowCount() == 0 + if (header.getDomRowCount() == 0 || !header.getRowElement(0).hasChildNodes()) { getLogger().info( "No header cell available when calculating sidebar button height"); diff --git a/themes/src/main/themes/VAADIN/themes/valo/components/_escalator.scss b/themes/src/main/themes/VAADIN/themes/valo/components/_escalator.scss index 895d9ab975..b30c3b3641 100644 --- a/themes/src/main/themes/VAADIN/themes/valo/components/_escalator.scss +++ b/themes/src/main/themes/VAADIN/themes/valo/components/_escalator.scss @@ -85,6 +85,10 @@ top: 0; left: 0; } + + .#{$primaryStyleName}-header > .#{$primaryStyleName}-row { + position: relative; + } } .#{$primaryStyleName}-row { diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/CompatibilityGridInDetailsRow.java b/uitest/src/main/java/com/vaadin/tests/components/grid/CompatibilityGridInDetailsRow.java new file mode 100644 index 0000000000..b34a0dee77 --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/components/grid/CompatibilityGridInDetailsRow.java @@ -0,0 +1,74 @@ +package com.vaadin.tests.components.grid; + +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Component; +import com.vaadin.v7.event.ItemClickEvent; +import com.vaadin.v7.event.ItemClickEvent.ItemClickListener; +import com.vaadin.v7.ui.Grid; +import com.vaadin.v7.ui.Grid.RowReference; + +@SuppressWarnings("deprecation") +public class CompatibilityGridInDetailsRow extends AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + Grid fg = new Grid(); + fg.setId("grid1"); + fg.setSizeFull(); + fg.addColumn("col1", String.class); + fg.addColumn("col2", String.class); + fg.addRow("Temp 1", "Temp 2"); + fg.addRow("Temp 3", "Temp 4"); + fg.setDetailsGenerator(new Grid.DetailsGenerator() { + @Override + public Component getDetails(RowReference rowReference) { + Grid gd = new Grid(); + gd.setId("grid2"); + gd.setSizeFull(); + gd.addHeaderRowAt(0); + gd.addColumn("Column 1", String.class); + gd.addColumn("Column 2", String.class); + gd.getColumn("Column 2").setHidable(true); + gd.addColumn("Column 3", String.class); + gd.addColumn("Column 4", String.class); + gd.addColumn("id", Integer.class); + gd.addRow("Nicolaus Copernicus", "Nicolaus Copernicus", + "Nicolaus Copernicus", "Nicolaus Copernicus", 1543); + gd.addRow("Nicolaus Copernicus", "Nicolaus Copernicus", + "Nicolaus Copernicus", "Nicolaus Copernicus", 1543); + gd.addRow("Nicolaus Copernicus", "Nicolaus Copernicus", + "Nicolaus Copernicus", "Nicolaus Copernicus", 1543); + gd.addRow("Nicolaus Copernicus", "Nicolaus Copernicus", + "Nicolaus Copernicus", "Nicolaus Copernicus", 1543); + + return gd; + } + }); + + fg.addItemClickListener(new ItemClickListener() { + @Override + public void itemClick(ItemClickEvent event) { + if (event.isDoubleClick()) { + Object itemId = event.getItemId(); + fg.setDetailsVisible(itemId, !fg.isDetailsVisible(itemId)); + } + } + }); + + getLayout().addComponent(fg); + } + + @Override + protected String getTestDescription() { + return "A nested Grid with multirow header should display all headers and " + + "opening the details row shouldn't cause a client-side exception " + + "when the nested Grid has hideable rows."; + } + + @Override + protected Integer getTicketNumber() { + return 7674; + } + +} diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridInDetailsRow.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridInDetailsRow.java new file mode 100644 index 0000000000..72c4cf4660 --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridInDetailsRow.java @@ -0,0 +1,76 @@ +package com.vaadin.tests.components.grid; + +import com.vaadin.annotations.Widgetset; +import com.vaadin.server.VaadinRequest; +import com.vaadin.shared.ui.grid.HeightMode; +import com.vaadin.tests.data.bean.Person; +import com.vaadin.ui.Component; +import com.vaadin.ui.Grid; +import com.vaadin.ui.Grid.ItemClick; +import com.vaadin.ui.VerticalLayout; +import com.vaadin.ui.components.grid.DetailsGenerator; +import com.vaadin.ui.components.grid.HeaderRow; +import com.vaadin.ui.components.grid.ItemClickListener; + +@Widgetset("com.vaadin.DefaultWidgetSet") +public class GridInDetailsRow extends SimpleGridUI { + + int index = 0; + + @Override + protected void setup(VaadinRequest request) { + getLayout().addComponent(createGrid()); + } + + @Override + protected Grid<Person> createGrid() { + Grid<Person> grid = super.createGrid(); + grid.setId("grid" + index); + ++index; + grid.setSizeFull(); + grid.setHeightUndefined(); + grid.setHeightMode(HeightMode.UNDEFINED); + + HeaderRow hr0 = grid.addHeaderRowAt(0); + hr0.getCell(grid.getColumns().get(0)).setText("Name"); + hr0.getCell(grid.getColumns().get(1)).setText("Age"); + HeaderRow hr1 = grid.getDefaultHeaderRow(); + hr1.getCell(grid.getColumns().get(0)).setText("Foo"); + hr1.getCell(grid.getColumns().get(1)).setText("Bar"); + grid.getColumns().get(1).setHidable(true); + + grid.setDetailsGenerator(new DetailsGenerator<Person>() { + @Override + public Component apply(Person t) { + VerticalLayout layout = new VerticalLayout(); + layout.setMargin(true); + Grid<Person> gd = createGrid(); + layout.addComponent(gd); + return layout; + } + }); + + grid.addItemClickListener(new ItemClickListener<Person>() { + + @Override + public void itemClick(ItemClick<Person> event) { + if (event.getMouseEventDetails().isDoubleClick()) { + Person item = event.getItem(); + grid.setDetailsVisible(item, !grid.isDetailsVisible(item)); + } + } + }); + return grid; + } + + @Override + protected String getTestDescription() { + return "A nested Grid with multirow header should display all headers and " + + "the body rows shouldn't get stuck to default row height."; + } + + @Override + protected Integer getTicketNumber() { + return 7674; + } +} diff --git a/uitest/src/main/java/com/vaadin/tests/widgetset/client/grid/EscalatorProxy.java b/uitest/src/main/java/com/vaadin/tests/widgetset/client/grid/EscalatorProxy.java index 4a19ed8915..c7aca31f38 100644 --- a/uitest/src/main/java/com/vaadin/tests/widgetset/client/grid/EscalatorProxy.java +++ b/uitest/src/main/java/com/vaadin/tests/widgetset/client/grid/EscalatorProxy.java @@ -172,6 +172,11 @@ public class EscalatorProxy extends Escalator { } @Override + public boolean isAutodetectingRowHeightLater() { + return rowContainer.isAutodetectingRowHeightLater(); + } + + @Override public void setDefaultRowHeight(double px) throws IllegalArgumentException { rowContainer.setDefaultRowHeight(px); diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/CompatibilityGridInDetailsRowTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/CompatibilityGridInDetailsRowTest.java new file mode 100644 index 0000000000..bc9b72f9f7 --- /dev/null +++ b/uitest/src/test/java/com/vaadin/tests/components/grid/CompatibilityGridInDetailsRowTest.java @@ -0,0 +1,73 @@ +package com.vaadin.tests.components.grid; + +import static org.hamcrest.Matchers.greaterThan; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; + +import java.util.List; + +import org.junit.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.WebElement; + +import com.vaadin.testbench.elements.GridElement; +import com.vaadin.testbench.elements.GridElement.GridCellElement; +import com.vaadin.testbench.elements.GridElement.GridRowElement; +import com.vaadin.tests.tb3.MultiBrowserTest; + +public class CompatibilityGridInDetailsRowTest extends MultiBrowserTest { + @Test + public void testNestedGridMultiRowHeaderPositions() { + openTestURL(); + + GridElement grid = $(GridElement.class).first(); + GridRowElement row = grid.getRow(1); + row.doubleClick(); + + waitForElementPresent(By.className("v-grid-spacer")); + + GridElement nestedGrid = $(GridElement.class).id("grid2"); + assertEquals("Incorrect header row count.", 2, + nestedGrid.getHeaderCount()); + GridCellElement headerCell00 = nestedGrid.getHeaderCell(0, 0); + GridCellElement headerCell11 = nestedGrid.getHeaderCell(1, 1); + + assertThat("Incorrect X-position.", headerCell11.getLocation().getX(), + greaterThan(headerCell00.getLocation().getX())); + assertThat("Incorrect Y-position.", headerCell11.getLocation().getY(), + greaterThan(headerCell00.getLocation().getY())); + } + + @Test + public void testNestedGridRowHeights() { + openTestURL(); + + GridElement grid = $(GridElement.class).first(); + GridRowElement row = grid.getRow(1); + row.doubleClick(); + + waitForElementPresent(By.className("v-grid-spacer")); + + GridElement nestedGrid = $(GridElement.class).id("grid2"); + grid.findElement(By.className("v-grid-sidebar-button")).click(); + + assertNotNull( + "There are no options for toggling column visibility but there should be.", + getColumnHidingToggle(nestedGrid)); + } + + /** + * Returns the first toggle inside the sidebar for hiding a column, or null + * if not found. + */ + protected WebElement getColumnHidingToggle(GridElement grid) { + WebElement sidebar = findElement(By.className("v-grid-sidebar-popup")); + List<WebElement> elements = sidebar + .findElements(By.className("column-hiding-toggle")); + for (WebElement e : elements) { + return e; + } + return null; + } +} diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridInDetailsRowTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridInDetailsRowTest.java new file mode 100644 index 0000000000..51e2a56d65 --- /dev/null +++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridInDetailsRowTest.java @@ -0,0 +1,51 @@ +package com.vaadin.tests.components.grid; + +import static org.hamcrest.Matchers.greaterThan; +import static org.junit.Assert.assertThat; + +import org.junit.Test; +import org.openqa.selenium.By; + +import com.vaadin.testbench.elements.GridElement; +import com.vaadin.testbench.elements.GridElement.GridCellElement; +import com.vaadin.testbench.elements.GridElement.GridRowElement; +import com.vaadin.tests.tb3.MultiBrowserTest; + +public class GridInDetailsRowTest extends MultiBrowserTest { + @Test + public void testNestedGridMultiRowHeaderPositions() { + openTestURL(); + + GridElement grid = $(GridElement.class).first(); + GridRowElement row = grid.getRow(2); + row.doubleClick(); + + waitForElementPresent(By.className("v-grid-spacer")); + + GridElement nestedGrid = $(GridElement.class).id("grid1"); + GridCellElement headerCell00 = nestedGrid.getHeaderCell(0, 0); + GridCellElement headerCell11 = nestedGrid.getHeaderCell(1, 1); + + assertThat("Incorrect X-position.", headerCell11.getLocation().getX(), + greaterThan(headerCell00.getLocation().getX())); + assertThat("Incorrect Y-position.", headerCell11.getLocation().getY(), + greaterThan(headerCell00.getLocation().getY())); + } + + @Test + public void testNestedGridRowHeights() { + openTestURL(); + + GridElement grid = $(GridElement.class).first(); + GridRowElement row = grid.getRow(2); + row.doubleClick(); + + waitForElementPresent(By.className("v-grid-spacer")); + + GridElement nestedGrid = $(GridElement.class).id("grid1"); + GridCellElement cell = nestedGrid.getCell(0, 0); + + assertThat("Incorrect row height.", cell.getSize().height, + greaterThan(30)); + } +} |