diff options
8 files changed, 457 insertions, 11 deletions
diff --git a/client/src/com/vaadin/client/LayoutManagerIE8.java b/client/src/com/vaadin/client/LayoutManagerIE8.java index 4464c3bee8..479155d0e6 100644 --- a/client/src/com/vaadin/client/LayoutManagerIE8.java +++ b/client/src/com/vaadin/client/LayoutManagerIE8.java @@ -53,6 +53,10 @@ public class LayoutManagerIE8 extends LayoutManager { } else { measuredSizes.remove(element); } + // clear any values that are saved to the element + if (super.getMeasuredSize(element, null) != null) { + super.setMeasuredSize(element, null); + } } @Override @@ -62,6 +66,13 @@ public class LayoutManagerIE8 extends LayoutManager { if (measured != null) { return measured; } else { + // check if saved to the element instead + MeasuredSize measuredSize = super.getMeasuredSize(element, null); + if (measuredSize != null) { + // move the value back to the map + setMeasuredSize(element, measuredSize); + return measuredSize; + } return defaultSize; } } @@ -72,13 +83,17 @@ public class LayoutManagerIE8 extends LayoutManager { // #12688: IE8 was leaking memory when adding&removing components. // Uses IE specific logic to figure if an element has been removed from - // DOM or not. For removed elements the measured size is discarded. + // DOM or not. For removed elements the measured size is stored within + // the element in case the element gets re-attached. Node rootNode = Document.get().getBody(); Iterator<Element> i = measuredSizes.keySet().iterator(); while (i.hasNext()) { Element e = i.next(); if (!rootNode.isOrHasChild(e)) { + // Store in element in case is still needed. + // Not attached, so reflow isn't a problem. + super.setMeasuredSize(e, measuredSizes.get(e)); i.remove(); } } diff --git a/client/src/com/vaadin/client/connectors/GridConnector.java b/client/src/com/vaadin/client/connectors/GridConnector.java index 053487620a..c6075d3bd4 100644 --- a/client/src/com/vaadin/client/connectors/GridConnector.java +++ b/client/src/com/vaadin/client/connectors/GridConnector.java @@ -54,11 +54,11 @@ import com.vaadin.client.ui.ConnectorFocusAndBlurHandler; import com.vaadin.client.ui.SimpleManagedLayout; import com.vaadin.client.widget.grid.CellReference; import com.vaadin.client.widget.grid.CellStyleGenerator; -import com.vaadin.client.widget.grid.DetailsGenerator; import com.vaadin.client.widget.grid.EditorHandler; import com.vaadin.client.widget.grid.EventCellReference; import com.vaadin.client.widget.grid.RowReference; import com.vaadin.client.widget.grid.RowStyleGenerator; +import com.vaadin.client.widget.grid.HeightAwareDetailsGenerator; import com.vaadin.client.widget.grid.events.BodyClickHandler; import com.vaadin.client.widget.grid.events.BodyDoubleClickHandler; import com.vaadin.client.widget.grid.events.ColumnReorderEvent; @@ -489,13 +489,45 @@ public class GridConnector extends AbstractHasComponentsConnector implements } }; - private class CustomDetailsGenerator implements DetailsGenerator { + private class CustomDetailsGenerator implements HeightAwareDetailsGenerator { private final Map<String, ComponentConnector> idToDetailsMap = new HashMap<String, ComponentConnector>(); private final Map<String, Integer> idToRowIndex = new HashMap<String, Integer>(); @Override public Widget getDetails(int rowIndex) { + String id = getId(rowIndex); + if (id == null) { + return null; + } + ComponentConnector componentConnector = idToDetailsMap.get(id); + idToRowIndex.put(id, rowIndex); + + return componentConnector.getWidget(); + } + + @Override + public double getDetailsHeight(int rowIndex) { + String id = getId(rowIndex); + ComponentConnector componentConnector = idToDetailsMap.get(id); + + getLayoutManager().setNeedsMeasureRecursively(componentConnector); + getLayoutManager().layoutNow(); + + return getLayoutManager().getOuterHeightDouble( + componentConnector.getWidget().getElement()); + } + + /** + * Fetches id from the row object that corresponds with the given + * rowIndex. + * + * @since + * @param rowIndex + * the index of the row for which to fetch the id + * @return id of the row if such id exists, {@code null} otherwise + */ + private String getId(int rowIndex) { JsonObject row = getWidget().getDataSource().getRow(rowIndex); if (!row.hasKey(GridState.JSONKEY_DETAILS_VISIBLE) @@ -504,11 +536,7 @@ public class GridConnector extends AbstractHasComponentsConnector implements return null; } - String id = row.getString(GridState.JSONKEY_DETAILS_VISIBLE); - ComponentConnector componentConnector = idToDetailsMap.get(id); - idToRowIndex.put(id, rowIndex); - - return componentConnector.getWidget(); + return row.getString(GridState.JSONKEY_DETAILS_VISIBLE); } public void updateConnectorHierarchy(List<ServerConnector> children) { diff --git a/client/src/com/vaadin/client/widget/grid/HeightAwareDetailsGenerator.java b/client/src/com/vaadin/client/widget/grid/HeightAwareDetailsGenerator.java new file mode 100644 index 0000000000..fb7dd2c8ea --- /dev/null +++ b/client/src/com/vaadin/client/widget/grid/HeightAwareDetailsGenerator.java @@ -0,0 +1,45 @@ +/* + * 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.client.widget.grid; + +/** + * {@link DetailsGenerator} that is aware of content heights. + * <p> + * <b>FOR INTERNAL USE ONLY!</b> This class exists only for the sake of a + * temporary workaround and might be removed or renamed at any time. + * </p> + * + * @since + * @author Vaadin Ltd + */ +@Deprecated +public interface HeightAwareDetailsGenerator extends DetailsGenerator { + + /** + * This method is called for whenever a details row's height needs to be + * calculated. + * <p> + * <b>FOR INTERNAL USE ONLY!</b> This method exists only for the sake of a + * temporary workaround and might be removed or renamed at any time. + * </p> + * + * @since + * @param rowIndex + * the index of the row for which to calculate details row height + * @return height of the details row + */ + public double getDetailsHeight(int rowIndex); +} diff --git a/client/src/com/vaadin/client/widgets/Grid.java b/client/src/com/vaadin/client/widgets/Grid.java index 05b90e204d..e3c51830fe 100644 --- a/client/src/com/vaadin/client/widgets/Grid.java +++ b/client/src/com/vaadin/client/widgets/Grid.java @@ -119,6 +119,7 @@ import com.vaadin.client.widget.grid.EventCellReference; import com.vaadin.client.widget.grid.RendererCellReference; import com.vaadin.client.widget.grid.RowReference; import com.vaadin.client.widget.grid.RowStyleGenerator; +import com.vaadin.client.widget.grid.HeightAwareDetailsGenerator; import com.vaadin.client.widget.grid.events.AbstractGridKeyEventHandler; import com.vaadin.client.widget.grid.events.AbstractGridMouseEventHandler; import com.vaadin.client.widget.grid.events.BodyClickHandler; @@ -3530,11 +3531,17 @@ public class Grid<T> extends ResizeComposite implements * height, but the spacer cell (td) has the borders, which * should go on top of the previous row and next row. */ - double requiredHeightBoundingClientRectDouble = WidgetUtil - .getRequiredHeightBoundingClientRectDouble(element); + double contentHeight; + if (detailsGenerator instanceof HeightAwareDetailsGenerator) { + HeightAwareDetailsGenerator sadg = (HeightAwareDetailsGenerator) detailsGenerator; + contentHeight = sadg.getDetailsHeight(rowIndex); + } else { + contentHeight = WidgetUtil + .getRequiredHeightBoundingClientRectDouble(element); + } double borderTopAndBottomHeight = WidgetUtil .getBorderTopAndBottomThickness(spacerElement); - double measuredHeight = requiredHeightBoundingClientRectDouble + double measuredHeight = contentHeight + borderTopAndBottomHeight; assert getElement().isOrHasChild(spacerElement) : "The spacer element wasn't in the DOM during measurement, but was assumed to be."; spacerHeight = measuredHeight; diff --git a/uitest/src/com/vaadin/tests/components/grid/GridDetailsLayoutExpand.java b/uitest/src/com/vaadin/tests/components/grid/GridDetailsLayoutExpand.java new file mode 100644 index 0000000000..be82b49f0c --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/GridDetailsLayoutExpand.java @@ -0,0 +1,103 @@ +/* + * 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.DetailsGenerator; +import com.vaadin.ui.Grid.RowReference; +import com.vaadin.ui.HorizontalLayout; +import com.vaadin.ui.Label; + +/** + * Tests the layouting of Grid's details row when it contains a HorizontalLayout + * with expand ratios. + * + * @author Vaadin Ltd + */ +@SuppressWarnings("serial") +public class GridDetailsLayoutExpand extends AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + final Grid grid = new Grid(); + grid.setSizeFull(); + grid.addColumn("name", String.class); + grid.addColumn("born", Integer.class); + + grid.addRow("Nicolaus Copernicus", 1543); + grid.addRow("Galileo Galilei", 1564); + grid.addRow("Johannes Kepler", 1571); + + addComponent(grid); + + grid.setDetailsGenerator(new DetailsGenerator() { + @Override + public Component getDetails(final RowReference rowReference) { + final HorizontalLayout detailsLayout = new HorizontalLayout(); + detailsLayout.setSizeFull(); + detailsLayout.setHeightUndefined(); + + // Label 1 first element of the detailsLayout, taking 200 pixels + final Label lbl1 = new Label("test1"); + lbl1.setWidth("200px"); + detailsLayout.addComponent(lbl1); + + // layout2 second element of the detailsLayout, taking the rest + // of the available space + final HorizontalLayout layout2 = new HorizontalLayout(); + layout2.setSizeFull(); + layout2.setHeightUndefined(); + detailsLayout.addComponent(layout2); + detailsLayout.setExpandRatio(layout2, 1); + + // 2 Labels added to the layout2 + final Label lbl2 = new Label("test2"); + lbl2.setId("lbl2"); + layout2.addComponent(lbl2); + + final Label lbl3 = new Label("test3"); + lbl3.setId("lbl3"); + layout2.addComponent(lbl3); + + return detailsLayout; + } + }); + + grid.addItemClickListener(new ItemClickListener() { + @Override + public void itemClick(final ItemClickEvent event) { + final Object itemId = event.getItemId(); + grid.setDetailsVisible(itemId, !grid.isDetailsVisible(itemId)); + } + }); + + } + + @Override + protected Integer getTicketNumber() { + return 18821; + } + + @Override + protected String getTestDescription() { + return "Details row must be the same after opening another details row"; + } +} diff --git a/uitest/src/com/vaadin/tests/components/grid/GridDetailsLayoutExpandTest.java b/uitest/src/com/vaadin/tests/components/grid/GridDetailsLayoutExpandTest.java new file mode 100644 index 0000000000..6ea72540b5 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/GridDetailsLayoutExpandTest.java @@ -0,0 +1,76 @@ +/* + * 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 static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.number.IsCloseTo.closeTo; + +import java.util.List; + +import org.junit.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.remote.DesiredCapabilities; + +import com.vaadin.testbench.elements.GridElement; +import com.vaadin.testbench.elements.LabelElement; +import com.vaadin.testbench.parallel.Browser; +import com.vaadin.testbench.parallel.TestCategory; +import com.vaadin.tests.tb3.MultiBrowserTest; + +/** + * Tests the layouting of Grid's details row when it contains a HorizontalLayout + * with expand ratios. + * + * @author Vaadin Ltd + */ +@TestCategory("grid") +public class GridDetailsLayoutExpandTest extends MultiBrowserTest { + + @Override + public List<DesiredCapabilities> getBrowsersToTest() { + List<DesiredCapabilities> browsersToTest = super.getBrowsersToTest(); + // TODO: remove when #19326 is fixed + browsersToTest.remove(Browser.IE8.getDesiredCapabilities()); + browsersToTest.remove(Browser.IE9.getDesiredCapabilities()); + // for some reason PhantomJS doesn't find the label even if it detects + // the presence + browsersToTest.remove(Browser.PHANTOMJS.getDesiredCapabilities()); + return browsersToTest; + } + + @Test + public void testLabelWidths() { + openTestURL(); + waitForElementPresent(By.className("v-grid")); + + GridElement grid = $(GridElement.class).first(); + int gridWidth = grid.getSize().width; + + grid.getRow(2).click(); + waitForElementPresent(By.id("lbl2")); + + // space left over from first label should be divided equally + double expectedWidth = (double) (gridWidth - 200) / 2; + assertLabelWidth("lbl2", expectedWidth); + assertLabelWidth("lbl3", expectedWidth); + } + + private void assertLabelWidth(String id, double expectedWidth) { + // 1px leeway for calculations + assertThat("Unexpected label width.", (double) $(LabelElement.class) + .id(id).getSize().width, closeTo(expectedWidth, 1d)); + } +} diff --git a/uitest/src/com/vaadin/tests/components/grid/GridLayoutDetailsRow.java b/uitest/src/com/vaadin/tests/components/grid/GridLayoutDetailsRow.java new file mode 100644 index 0000000000..51c2598cc0 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/GridLayoutDetailsRow.java @@ -0,0 +1,95 @@ +/* + * 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.DetailsGenerator; +import com.vaadin.ui.Grid.RowReference; +import com.vaadin.ui.GridLayout; +import com.vaadin.ui.Label; + +/** + * Tests that details row displays GridLayout contents properly. + * + * @author Vaadin Ltd + */ +public class GridLayoutDetailsRow extends AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + final Grid grid = new Grid(); + grid.setSizeFull(); + grid.addColumn("name", String.class); + grid.addColumn("born", Integer.class); + + grid.addRow("Nicolaus Copernicus", 1543); + grid.addRow("Galileo Galilei", 1564); + grid.addRow("Johannes Kepler", 1571); + + addComponent(grid); + + grid.setDetailsGenerator(new DetailsGenerator() { + @Override + public Component getDetails(final RowReference rowReference) { + final GridLayout detailsLayout = new GridLayout(); + detailsLayout.setSizeFull(); + detailsLayout.setHeightUndefined(); + + final Label lbl1 = new Label("test1"); + lbl1.setId("lbl1"); + lbl1.setWidth("200px"); + detailsLayout.addComponent(lbl1); + + final Label lbl2 = new Label("test2"); + lbl2.setId("lbl2"); + detailsLayout.addComponent(lbl2); + + final Label lbl3 = new Label("test3"); + lbl3.setId("lbl3"); + detailsLayout.addComponent(lbl3); + + final Label lbl4 = new Label("test4"); + lbl4.setId("lbl4"); + detailsLayout.addComponent(lbl4); + + return detailsLayout; + } + }); + + grid.addItemClickListener(new ItemClickListener() { + @Override + public void itemClick(final ItemClickEvent event) { + final Object itemId = event.getItemId(); + grid.setDetailsVisible(itemId, !grid.isDetailsVisible(itemId)); + } + }); + } + + @Override + protected String getTestDescription() { + return "GridLayout as part of Grid detail row should be correctly computed/displayed."; + } + + @Override + protected Integer getTicketNumber() { + return 18619; + } +} diff --git a/uitest/src/com/vaadin/tests/components/grid/GridLayoutDetailsRowTest.java b/uitest/src/com/vaadin/tests/components/grid/GridLayoutDetailsRowTest.java new file mode 100644 index 0000000000..9ec54471b0 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/GridLayoutDetailsRowTest.java @@ -0,0 +1,77 @@ +/* + * 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 static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.number.IsCloseTo.closeTo; + +import java.util.List; + +import org.junit.Test; +import org.openqa.selenium.By; +import org.openqa.selenium.remote.DesiredCapabilities; + +import com.vaadin.testbench.elements.GridElement; +import com.vaadin.testbench.elements.GridLayoutElement; +import com.vaadin.testbench.elements.LabelElement; +import com.vaadin.testbench.parallel.Browser; +import com.vaadin.testbench.parallel.TestCategory; +import com.vaadin.tests.tb3.MultiBrowserTest; + +/** + * Tests that details row displays GridLayout contents properly. + * + * @author Vaadin Ltd + */ +@TestCategory("grid") +public class GridLayoutDetailsRowTest extends MultiBrowserTest { + + @Override + public List<DesiredCapabilities> getBrowsersToTest() { + List<DesiredCapabilities> browsersToTest = super.getBrowsersToTest(); + // for some reason PhantomJS doesn't find the label even if it detects + // the presence + browsersToTest.remove(Browser.PHANTOMJS.getDesiredCapabilities()); + return browsersToTest; + } + + @Test + public void testLabelHeights() { + openTestURL(); + waitForElementPresent(By.className("v-grid")); + + GridElement grid = $(GridElement.class).first(); + + grid.getRow(2).click(); + waitForElementPresent(By.id("lbl2")); + + GridLayoutElement gridLayout = $(GridLayoutElement.class).first(); + int gridLayoutHeight = gridLayout.getSize().height; + + // height should be divided equally + double expectedHeight = gridLayoutHeight / 4; + assertLabelHeight("lbl1", expectedHeight); + assertLabelHeight("lbl2", expectedHeight); + assertLabelHeight("lbl3", expectedHeight); + assertLabelHeight("lbl4", expectedHeight); + } + + private void assertLabelHeight(String id, double expectedHeight) { + // 1px leeway for calculations + assertThat("Unexpected label height.", (double) $(LabelElement.class) + .id(id).getSize().height, closeTo(expectedHeight, 1d)); + } +} |