From 684e619dd79e233ba798aa116810dd6585df3e7c Mon Sep 17 00:00:00 2001 From: Artur Signell Date: Tue, 2 Jun 2015 22:40:00 +0300 Subject: [PATCH] Calculate row width correctly when subpixel workaround/fix is active (#17934) Change-Id: I5fd535bf6622eaf47c5eb5fc509245e558d0a284 --- .../com/vaadin/client/widgets/Escalator.java | 20 +++- .../grid/GridSubPixelProblemWrapping.java | 96 +++++++++++++++++++ .../grid/GridSubPixelProblemWrappingTest.java | 67 +++++++++++++ 3 files changed, 178 insertions(+), 5 deletions(-) create mode 100644 uitest/src/com/vaadin/tests/components/grid/GridSubPixelProblemWrapping.java create mode 100644 uitest/src/com/vaadin/tests/components/grid/GridSubPixelProblemWrappingTest.java diff --git a/client/src/com/vaadin/client/widgets/Escalator.java b/client/src/com/vaadin/client/widgets/Escalator.java index 7cce34fa22..514fce26dc 100644 --- a/client/src/com/vaadin/client/widgets/Escalator.java +++ b/client/src/com/vaadin/client/widgets/Escalator.java @@ -879,6 +879,7 @@ public class Escalator extends Widget implements RequiresResize, .getCalculatedColumnsWidth(Range.between( columnConfiguration.getFrozenColumnCount(), columnConfiguration.getColumnCount())); + unfrozenPixels -= subpixelBrowserBugDetector.getActiveAdjustment(); double frozenPixels = scrollContentWidth - unfrozenPixels; double hScrollOffsetWidth = tableWrapperWidth - frozenPixels; horizontalScrollbar.setOffsetSize(hScrollOffsetWidth); @@ -4146,7 +4147,8 @@ public class Escalator extends Widget implements RequiresResize, * @return the width of a row, in pixels */ public double calculateRowWidth() { - return getCalculatedColumnsWidth(Range.between(0, getColumnCount())); + return getCalculatedColumnsWidth(Range.between(0, getColumnCount())) + - subpixelBrowserBugDetector.getActiveAdjustment(); } private void assertArgumentsAreValidAndWithinRange(final int index, @@ -4443,7 +4445,7 @@ public class Escalator extends Widget implements RequiresResize, private class SubpixelBrowserBugDetector { private static final double SUBPIXEL_ADJUSTMENT = .1; - private boolean hasAlreadyBeenFixed = false; + private boolean fixActive = false; /** * This is a fix essentially for Firefox and how it handles subpixels. @@ -4460,15 +4462,23 @@ public class Escalator extends Widget implements RequiresResize, * {@value #SUBPIXEL_ADJUSTMENT}px narrower. */ public void checkAndFix() { - if (!hasAlreadyBeenFixed && hasSubpixelBrowserBug()) { + if (!fixActive && hasSubpixelBrowserBug()) { fixSubpixelBrowserBug(); - hasAlreadyBeenFixed = true; + fixActive = true; + } + } + + private double getActiveAdjustment() { + if (fixActive) { + return -SUBPIXEL_ADJUSTMENT; + } else { + return 0.0; } } public void invalidateFix() { adjustBookkeepingPixels(SUBPIXEL_ADJUSTMENT); - hasAlreadyBeenFixed = false; + fixActive = false; } private boolean hasSubpixelBrowserBug() { diff --git a/uitest/src/com/vaadin/tests/components/grid/GridSubPixelProblemWrapping.java b/uitest/src/com/vaadin/tests/components/grid/GridSubPixelProblemWrapping.java new file mode 100644 index 0000000000..1e921bb18c --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/GridSubPixelProblemWrapping.java @@ -0,0 +1,96 @@ +/* + * 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.Random; + +import com.vaadin.annotations.Theme; +import com.vaadin.data.util.BeanItemContainer; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Button; +import com.vaadin.ui.Button.ClickEvent; +import com.vaadin.ui.Button.ClickListener; +import com.vaadin.ui.Grid; +import com.vaadin.ui.Grid.SelectionMode; + +@Theme("valo") +public class GridSubPixelProblemWrapping extends AbstractTestUI { + + Random r = new Random(); + + public static class DataObject { + String foo; + String Bar; + + public DataObject(Random r) { + foo = r.nextInt() + ""; + Bar = r.nextInt() + ""; + } + + public DataObject(String foo, String bar) { + this.foo = foo; + Bar = bar; + } + + public String getFoo() { + return foo; + } + + public void setFoo(String foo) { + this.foo = foo; + } + + public String getBar() { + return Bar; + } + + public void setBar(String bar) { + Bar = bar; + } + + } + + Button button = new Button("Click", new ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + addDAO(); + } + }); + + private BeanItemContainer container; + private int counter = 0; + + @Override + protected void setup(VaadinRequest request) { + addComponent(button); + container = new BeanItemContainer(DataObject.class); + container.addBean(new DataObject("Foo", "Bar")); + Grid grid = new Grid(container); + grid.getColumn("foo").setWidth(248.525); + grid.setSelectionMode(SelectionMode.SINGLE); + grid.setEditorEnabled(true); + grid.setWidth("500px"); + + addComponent(grid); + } + + private void addDAO() { + counter++; + container.addBean(new DataObject("Foo" + counter, "Bar" + counter)); + + } +} diff --git a/uitest/src/com/vaadin/tests/components/grid/GridSubPixelProblemWrappingTest.java b/uitest/src/com/vaadin/tests/components/grid/GridSubPixelProblemWrappingTest.java new file mode 100644 index 0000000000..7aacfa8548 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/GridSubPixelProblemWrappingTest.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.remote.DesiredCapabilities; + +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.testbench.elements.GridElement; +import com.vaadin.testbench.elements.GridElement.GridRowElement; +import com.vaadin.testbench.parallel.Browser; +import com.vaadin.tests.tb3.MultiBrowserTest; + +public class GridSubPixelProblemWrappingTest extends MultiBrowserTest { + + @Override + public List getBrowsersToTest() { + List l = super.getBrowsersToTest(); + // Currently broken because of #18214 + l.remove(Browser.IE9.getDesiredCapabilities()); + return l; + } + + @Test + public void addedRowShouldNotWrap() { + openTestURL(); + + GridElement grid = $(GridElement.class).first(); + + // Cells in first row should be at the same y coordinate as the row + assertRowAndCellTops(grid, 0); + + // Add a row + $(ButtonElement.class).first().click(); + + // Cells in the first row should be at the same y coordinate as the row + assertRowAndCellTops(grid, 0); + // Cells in the second row should be at the same y coordinate as the row + assertRowAndCellTops(grid, 1); + } + + private void assertRowAndCellTops(GridElement grid, int rowIndex) { + GridRowElement row = grid.getRow(rowIndex); + int rowTop = row.getLocation().y; + + int cell0Top = grid.getCell(rowIndex, 0).getLocation().y; + int cell1Top = grid.getCell(rowIndex, 1).getLocation().y; + Assert.assertEquals(rowTop, cell0Top); + Assert.assertEquals(rowTop, cell1Top); + } +} -- 2.39.5