Skip to content

Commit

Permalink
Ensure GridLayout rounds available space down instead of up (#15451)
Browse files Browse the repository at this point in the history
Store measured widths and heights as doubles to be able to round later

Change-Id: Id0e91702dd62ba362f53317e8520f85b46f19769
  • Loading branch information
Artur- authored and Vaadin Code Review committed Jun 12, 2015
1 parent 7f39ad3 commit 914eafd
Show file tree
Hide file tree
Showing 8 changed files with 388 additions and 49 deletions.
156 changes: 134 additions & 22 deletions client/src/com/vaadin/client/LayoutManager.java

Large diffs are not rendered by default.

34 changes: 17 additions & 17 deletions client/src/com/vaadin/client/MeasuredSize.java
Expand Up @@ -45,20 +45,20 @@ public boolean isChanged() {
}
}

private int width = -1;
private int height = -1;
private double width = -1;
private double height = -1;

private int[] paddings = new int[4];
private int[] borders = new int[4];
private int[] margins = new int[4];

private FastStringSet dependents = FastStringSet.create();

public int getOuterHeight() {
public double getOuterHeight() {
return height;
}

public int getOuterWidth() {
public double getOuterWidth() {
return width;
}

Expand Down Expand Up @@ -86,17 +86,17 @@ private static int sumHeights(int[] sizes) {
return sizes[0] + sizes[2];
}

public int getInnerHeight() {
public double getInnerHeight() {
return height - sumHeights(margins) - sumHeights(borders)
- sumHeights(paddings);
}

public int getInnerWidth() {
public double getInnerWidth() {
return width - sumWidths(margins) - sumWidths(borders)
- sumWidths(paddings);
}

public boolean setOuterHeight(int height) {
public boolean setOuterHeight(double height) {
if (this.height != height) {
this.height = height;
return true;
Expand All @@ -105,7 +105,7 @@ public boolean setOuterHeight(int height) {
}
}

public boolean setOuterWidth(int width) {
public boolean setOuterWidth(double width) {
if (this.width != width) {
this.width = width;
return true;
Expand Down Expand Up @@ -238,20 +238,20 @@ public MeasureResult measure(Element element) {
Profiler.leave("Measure borders");

Profiler.enter("Measure height");
int requiredHeight = WidgetUtil.getRequiredHeight(element);
int marginHeight = sumHeights(margins);
int oldHeight = height;
int oldWidth = width;
if (setOuterHeight(requiredHeight + marginHeight)) {
double requiredHeight = WidgetUtil.getRequiredHeightDouble(element);
double outerHeight = requiredHeight + sumHeights(margins);
double oldHeight = height;
if (setOuterHeight(outerHeight)) {
debugSizeChange(element, "Height (outer)", oldHeight, height);
heightChanged = true;
}
Profiler.leave("Measure height");

Profiler.enter("Measure width");
int requiredWidth = WidgetUtil.getRequiredWidth(element);
int marginWidth = sumWidths(margins);
if (setOuterWidth(requiredWidth + marginWidth)) {
double requiredWidth = WidgetUtil.getRequiredWidthDouble(element);
double outerWidth = requiredWidth + sumWidths(margins);
double oldWidth = width;
if (setOuterWidth(outerWidth)) {
debugSizeChange(element, "Width (outer)", oldWidth, width);
widthChanged = true;
}
Expand All @@ -270,7 +270,7 @@ private void debugSizeChange(Element element, String sizeChangeType,
}

private void debugSizeChange(Element element, String sizeChangeType,
int changedFrom, int changedTo) {
double changedFrom, double changedTo) {
debugSizeChange(element, sizeChangeType, String.valueOf(changedFrom),
String.valueOf(changedTo));
}
Expand Down
68 changes: 62 additions & 6 deletions client/src/com/vaadin/client/WidgetUtil.java
Expand Up @@ -540,6 +540,29 @@ public static int getRequiredWidth(com.google.gwt.dom.client.Element element) {
return reqWidth;
}

/**
* Gets the border-box width for the given element, i.e. element width +
* border + padding.
*
* @param element
* The element to check
* @return The border-box width for the element
*/
public static double getRequiredWidthDouble(
com.google.gwt.dom.client.Element element) {
double reqWidth = getRequiredWidthBoundingClientRectDouble(element);
if (BrowserInfo.get().isIE() && !BrowserInfo.get().isIE8()) {
double csWidth = getRequiredWidthComputedStyleDouble(element);
if (csWidth > reqWidth && csWidth < (reqWidth + 1)) {
// IE9 rounds reqHeight to integers BUT sometimes reports wrong
// csHeight it seems, so we only use csHeight if it is within a
// rounding error
return csWidth;
}
}
return reqWidth;
}

/**
* Gets the border-box height for the given element, i.e. element height +
* border + padding. Always rounds up to nearest integer.
Expand All @@ -566,6 +589,29 @@ public static int getRequiredHeight(
return reqHeight;
}

/**
* Gets the border-box height for the given element, i.e. element height +
* border + padding.
*
* @param element
* The element to check
* @return The border-box height for the element
*/
public static double getRequiredHeightDouble(
com.google.gwt.dom.client.Element element) {
double reqHeight = getRequiredHeightBoundingClientRectDouble(element);
if (BrowserInfo.get().isIE() && !BrowserInfo.get().isIE8()) {
double csHeight = getRequiredHeightComputedStyleDouble(element);
if (csHeight > reqHeight && csHeight < (reqHeight + 1)) {
// IE9 rounds reqHeight to integers BUT sometimes reports wrong
// csHeight it seems, so we only use csHeight if it is within a
// rounding error
return csHeight;
}
}
return reqHeight;
}

/**
* Calculates the width of the element's bounding rectangle.
* <p>
Expand Down Expand Up @@ -605,34 +651,44 @@ public static native double getRequiredWidthBoundingClientRectDouble(
}
}-*/;

public static native int getRequiredHeightComputedStyle(
public static int getRequiredHeightComputedStyle(
com.google.gwt.dom.client.Element element) {
return (int) Math.ceil(getRequiredHeightComputedStyleDouble(element));
}

public static native double getRequiredHeightComputedStyleDouble(
com.google.gwt.dom.client.Element element)
/*-{
var cs = element.ownerDocument.defaultView.getComputedStyle(element);
var heightPx = cs.height;
if(heightPx == 'auto'){
// Fallback for inline elements
return @com.vaadin.client.WidgetUtil::getRequiredHeightBoundingClientRect(Lcom/google/gwt/dom/client/Element;)(element);
return @com.vaadin.client.WidgetUtil::getRequiredHeightBoundingClientRectDouble(Lcom/google/gwt/dom/client/Element;)(element);
}
var height = parseFloat(heightPx); // Will automatically skip "px" suffix
var border = parseFloat(cs.borderTopWidth) + parseFloat(cs.borderBottomWidth); // Will automatically skip "px" suffix
var padding = parseFloat(cs.paddingTop) + parseFloat(cs.paddingBottom); // Will automatically skip "px" suffix
return Math.ceil(height+border+padding);
return height+border+padding;
}-*/;

public static native int getRequiredWidthComputedStyle(
public static int getRequiredWidthComputedStyle(
com.google.gwt.dom.client.Element element) {
return (int) Math.ceil(getRequiredWidthComputedStyleDouble(element));
}

public static native int getRequiredWidthComputedStyleDouble(
com.google.gwt.dom.client.Element element)
/*-{
var cs = element.ownerDocument.defaultView.getComputedStyle(element);
var widthPx = cs.width;
if(widthPx == 'auto'){
// Fallback for inline elements
return @com.vaadin.client.WidgetUtil::getRequiredWidthBoundingClientRect(Lcom/google/gwt/dom/client/Element;)(element);
return @com.vaadin.client.WidgetUtil::getRequiredWidthBoundingClientRectDouble(Lcom/google/gwt/dom/client/Element;)(element);
}
var width = parseFloat(widthPx); // Will automatically skip "px" suffix
var border = parseFloat(cs.borderLeftWidth) + parseFloat(cs.borderRightWidth); // Will automatically skip "px" suffix
var padding = parseFloat(cs.paddingLeft) + parseFloat(cs.paddingRight); // Will automatically skip "px" suffix
return Math.ceil(width+border+padding);
return width+border+padding;
}-*/;

/**
Expand Down
12 changes: 8 additions & 4 deletions client/src/com/vaadin/client/ui/VGridLayout.java
Expand Up @@ -143,8 +143,10 @@ void expandRows() {
if (!isUndefinedHeight()) {
int usedSpace = calcRowUsedSpace();
int[] actualExpandRatio = calcRowExpandRatio();
int availableSpace = LayoutManager.get(client).getInnerHeight(
getElement());
// Round down to avoid problems with fractions (100.1px available ->
// can use 100, not 101)
int availableSpace = (int) LayoutManager.get(client)
.getInnerHeightDouble(getElement());
int excessSpace = availableSpace - usedSpace;
int distributed = 0;
if (excessSpace > 0) {
Expand Down Expand Up @@ -223,8 +225,10 @@ void expandColumns() {
if (!isUndefinedWidth()) {
int usedSpace = calcColumnUsedSpace();
int[] actualExpandRatio = calcColumnExpandRatio();
int availableSpace = LayoutManager.get(client).getInnerWidth(
getElement());
// Round down to avoid problems with fractions (100.1px available ->
// can use 100, not 101)
int availableSpace = (int) LayoutManager.get(client)
.getInnerWidthDouble(getElement());
int excessSpace = availableSpace - usedSpace;
int distributed = 0;
if (excessSpace > 0) {
Expand Down
@@ -0,0 +1,66 @@
/*
* 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.gridlayout;

import com.vaadin.annotations.Widgetset;
import com.vaadin.server.VaadinRequest;
import com.vaadin.tests.components.AbstractTestUIWithLog;
import com.vaadin.tests.widgetset.TestingWidgetSet;
import com.vaadin.tests.widgetset.server.ScrollableGridLayout;
import com.vaadin.ui.Alignment;
import com.vaadin.ui.Button;
import com.vaadin.ui.GridLayout;

@Widgetset(TestingWidgetSet.NAME)
public class GridLayoutFractionalSizeAndAlignment extends AbstractTestUIWithLog {

@Override
protected void setup(VaadinRequest request) {
widthTest();
heightTest();
}

private void widthTest() {
final GridLayout layout = new ScrollableGridLayout(1, 1);
layout.setMargin(false);
layout.setSpacing(true);

layout.setWidth(525.04f, Unit.PIXELS);

Button button = new Button("Button");

layout.addComponent(button);
layout.setComponentAlignment(button, Alignment.BOTTOM_RIGHT);

addComponent(layout);
}

private void heightTest() {
final GridLayout layout = new ScrollableGridLayout(1, 1);
layout.setMargin(false);
layout.setSpacing(true);

layout.setWidth(525.04f, Unit.PIXELS);
layout.setHeight(525.04f, Unit.PIXELS);

Button button = new Button("Button");

layout.addComponent(button);
layout.setComponentAlignment(button, Alignment.BOTTOM_RIGHT);

addComponent(layout);
}
}
@@ -0,0 +1,31 @@
/*
* 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.gridlayout;

import java.io.IOException;

import org.junit.Test;

import com.vaadin.tests.tb3.MultiBrowserTest;

public class GridLayoutFractionalSizeAndAlignmentTest extends MultiBrowserTest {

@Test
public void ensureNoScrollbarsWithAlignBottomRight() throws IOException {
openTestURL();
compareScreen("noscrollbars");
}
}
@@ -0,0 +1,36 @@
/*
* 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.widgetset.client;

import com.vaadin.client.ConnectorHierarchyChangeEvent;
import com.vaadin.client.ui.VGridLayout;
import com.vaadin.client.ui.gridlayout.GridLayoutConnector;
import com.vaadin.client.ui.layout.MayScrollChildren;
import com.vaadin.shared.ui.Connect;
import com.vaadin.tests.widgetset.server.ScrollableGridLayout;

@Connect(ScrollableGridLayout.class)
public class ScrollableGridLayoutConnector extends GridLayoutConnector
implements MayScrollChildren {
@Override
public void onConnectorHierarchyChange(ConnectorHierarchyChangeEvent event) {
super.onConnectorHierarchyChange(event);

for (VGridLayout.Cell cell : getWidget().widgetToCell.values()) {
cell.slot.getWrapperElement().addClassName("v-scrollable");
}
}
}

0 comments on commit 914eafd

Please sign in to comment.