From 102e85b0f54aa37c787a40fb3a30b3c45c7514a0 Mon Sep 17 00:00:00 2001 From: Teppo Kurki Date: Tue, 30 Jun 2015 09:28:29 +0300 Subject: [PATCH] Do not let Escalator set width to spacer row's TD element (#18223) Change-Id: I560bb4663d72cc4f939f2d463ef678fd335e7e8e --- .../com/vaadin/client/widgets/Escalator.java | 31 ++++--- .../components/grid/GridDetailsWidth.java | 91 +++++++++++++++++++ .../components/grid/GridDetailsWidthTest.java | 67 ++++++++++++++ 3 files changed, 175 insertions(+), 14 deletions(-) create mode 100644 uitest/src/com/vaadin/tests/components/grid/GridDetailsWidth.java create mode 100644 uitest/src/com/vaadin/tests/components/grid/GridDetailsWidthTest.java diff --git a/client/src/com/vaadin/client/widgets/Escalator.java b/client/src/com/vaadin/client/widgets/Escalator.java index 1bb3fe3747..436b512294 100644 --- a/client/src/com/vaadin/client/widgets/Escalator.java +++ b/client/src/com/vaadin/client/widgets/Escalator.java @@ -1816,20 +1816,23 @@ public class Escalator extends Widget implements RequiresResize, public void reapplyColumnWidths() { Element row = root.getFirstChildElement(); while (row != null) { - Element cell = row.getFirstChildElement(); - int columnIndex = 0; - while (cell != null) { - final double width = getCalculatedColumnWidthWithColspan( - cell, columnIndex); + // Only handle non-spacer rows + if (!body.spacerContainer.isSpacer(row)) { + Element cell = row.getFirstChildElement(); + int columnIndex = 0; + while (cell != null) { + final double width = getCalculatedColumnWidthWithColspan( + cell, columnIndex); - /* - * TODO Should Escalator implement ProvidesResize at some - * point, this is where we need to do that. - */ - cell.getStyle().setWidth(width, Unit.PX); + /* + * TODO Should Escalator implement ProvidesResize at + * some point, this is where we need to do that. + */ + cell.getStyle().setWidth(width, Unit.PX); - cell = cell.getNextSiblingElement(); - columnIndex++; + cell = cell.getNextSiblingElement(); + columnIndex++; + } } row = row.getNextSiblingElement(); } @@ -4795,7 +4798,7 @@ public class Escalator extends Widget implements RequiresResize, } /** Checks if a given element is a spacer element */ - public boolean isSpacer(TableRowElement focusedRow) { + public boolean isSpacer(Element row) { /* * If this needs optimization, we could do a more heuristic check @@ -4804,7 +4807,7 @@ public class Escalator extends Widget implements RequiresResize, */ for (SpacerImpl spacer : rowIndexToSpacer.values()) { - if (spacer.getRootElement().equals(focusedRow)) { + if (spacer.getRootElement().equals(row)) { return true; } } diff --git a/uitest/src/com/vaadin/tests/components/grid/GridDetailsWidth.java b/uitest/src/com/vaadin/tests/components/grid/GridDetailsWidth.java new file mode 100644 index 0000000000..4955f499ba --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/GridDetailsWidth.java @@ -0,0 +1,91 @@ +/* + * 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; + +import com.vaadin.event.ItemClickEvent; +import com.vaadin.event.ItemClickEvent.ItemClickListener; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Component; +import com.vaadin.ui.Grid; +import com.vaadin.ui.Grid.Column; +import com.vaadin.ui.Grid.DetailsGenerator; +import com.vaadin.ui.Grid.RowReference; +import com.vaadin.ui.HorizontalLayout; +import com.vaadin.ui.TextArea; +import com.vaadin.ui.VerticalLayout; + +public class GridDetailsWidth extends AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + final VerticalLayout layout = new VerticalLayout(); + layout.setMargin(true); + + final Grid grid = new Grid(); + + Column column = grid.addColumn("Hello", String.class); + for (int i = 0; i < 3; i++) { + grid.addRow("Hello " + i); + } + + column.setWidth(600); + grid.setWidth(400, Unit.PIXELS); + + grid.setDetailsGenerator(new DetailsGenerator() { + + @Override + public Component getDetails(RowReference rowReference) { + HorizontalLayout myLayout = new HorizontalLayout(); + TextArea textArea1 = new TextArea(); + TextArea textArea2 = new TextArea(); + textArea1.setSizeFull(); + textArea2.setSizeFull(); + myLayout.addComponent(textArea1); + myLayout.addComponent(textArea2); + myLayout.setWidth(100, Unit.PERCENTAGE); + myLayout.setHeight(null); + myLayout.setMargin(true); + return myLayout; + } + }); + + grid.addItemClickListener(new ItemClickListener() { + + @Override + public void itemClick(ItemClickEvent event) { + grid.setDetailsVisible(event.getItemId(), + !grid.isDetailsVisible(event.getItemId())); + + } + }); + + layout.addComponent(grid); + + addComponent(layout); + } + + @Override + protected Integer getTicketNumber() { + return 18223; + } + + @Override + public String getDescription() { + return "Tests that Escalator will not set explicit widths to the TD element in a details row."; + } + +} diff --git a/uitest/src/com/vaadin/tests/components/grid/GridDetailsWidthTest.java b/uitest/src/com/vaadin/tests/components/grid/GridDetailsWidthTest.java new file mode 100644 index 0000000000..0a6f53820e --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/GridDetailsWidthTest.java @@ -0,0 +1,67 @@ +/* + * 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; + +import java.util.List; + +import org.junit.Assert; +import org.junit.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.WebElement; + +import com.vaadin.testbench.elements.GridElement; +import com.vaadin.tests.tb3.SingleBrowserTest; + +public class GridDetailsWidthTest extends SingleBrowserTest { + + @Test + public void testSpacerTDsHaveNoWidth() { + openTestURL(); + GridElement grid = $(GridElement.class).first(); + + // Open all details rows + grid.getCell(0, 0).click(); + checkSpacersHaveNoWidths(1); + + grid.getCell(1, 0).click(); + checkSpacersHaveNoWidths(2); + + grid.getCell(2, 0).click(); + checkSpacersHaveNoWidths(3); + + // Close all details rows + grid.getCell(2, 0).click(); + checkSpacersHaveNoWidths(2); + + grid.getCell(1, 0).click(); + checkSpacersHaveNoWidths(1); + + grid.getCell(0, 0).click(); + checkSpacersHaveNoWidths(0); + } + + private void checkSpacersHaveNoWidths(int expectedCount) { + List spacers = findElements(By.className("v-grid-spacer")); + Assert.assertEquals("Wrong amount of spacers visible.", expectedCount, + spacers.size()); + for (WebElement spacer : spacers) { + Assert.assertFalse("Spacer element had an unexpected width set.", + spacer.findElement(By.tagName("td")).getAttribute("style") + .contains("width")); + } + } + +} -- 2.39.5