From c390b386ad0da0e0eac29665f0db77bd30cb5e5f Mon Sep 17 00:00:00 2001 From: Teemu Suo-Anttila Date: Thu, 7 Jan 2016 15:45:26 +0200 Subject: Fix ScrollbarBundle to not round down pixel values (#19421) Since Escalator now uses subpixels in most cases, scrollbar calculation fails when ScrollbarBundle calls Math.floor for the offset size. Removed rounding, added a pixel epsilon check that we don't make a scrollbar when it's not really needed. Change-Id: I228fd7dd48d4506e5b2b7bb799291c9a122ea3fe --- .../client/widget/escalator/ScrollbarBundle.java | 14 +------ .../grid/GridMultiSelectionScrollBar.java | 34 +++++++++++++++++ .../grid/GridMultiSelectionScrollBarTest.java | 44 ++++++++++++++++++++++ 3 files changed, 80 insertions(+), 12 deletions(-) create mode 100644 uitest/src/com/vaadin/tests/components/grid/GridMultiSelectionScrollBar.java create mode 100644 uitest/src/com/vaadin/tests/components/grid/GridMultiSelectionScrollBarTest.java diff --git a/client/src/com/vaadin/client/widget/escalator/ScrollbarBundle.java b/client/src/com/vaadin/client/widget/escalator/ScrollbarBundle.java index 2b33d7103f..958029889d 100644 --- a/client/src/com/vaadin/client/widget/escalator/ScrollbarBundle.java +++ b/client/src/com/vaadin/client/widget/escalator/ScrollbarBundle.java @@ -402,11 +402,6 @@ public abstract class ScrollbarBundle implements DeferredWorker { /** * Sets the length of the scrollbar. - *

- * Note: Even though {@code double} values are used, they are - * currently only used as integers as large {@code int} (or small but fast - * {@code long}). This means, all values are truncated to zero decimal - * places. * * @param px * the length of the scrollbar in pixels @@ -437,7 +432,7 @@ public abstract class ScrollbarBundle implements DeferredWorker { } private void setOffsetSizeNow(double px) { - internalSetOffsetSize(Math.max(0, truncate(px))); + internalSetOffsetSize(Math.max(0, px)); recalculateMaxScrollPos(); forceScrollbar(showsScrollHandle()); fireVisibilityChangeIfNeeded(); @@ -595,11 +590,6 @@ public abstract class ScrollbarBundle implements DeferredWorker { /** * Sets the amount of pixels the scrollbar needs to be able to scroll * through. - *

- * Note: Even though {@code double} values are used, they are - * currently only used as integers as large {@code int} (or small but fast - * {@code long}). This means, all values are truncated to zero decimal - * places. * * @param px * the number of pixels the scrollbar should be able to scroll @@ -727,7 +717,7 @@ public abstract class ScrollbarBundle implements DeferredWorker { * @return true iff the scrollbar's handle is visible */ public boolean showsScrollHandle() { - return getOffsetSize() < getScrollSize(); + return getScrollSize() - getOffsetSize() > WidgetUtil.PIXEL_EPSILON; } public void recalculateMaxScrollPos() { diff --git a/uitest/src/com/vaadin/tests/components/grid/GridMultiSelectionScrollBar.java b/uitest/src/com/vaadin/tests/components/grid/GridMultiSelectionScrollBar.java new file mode 100644 index 0000000000..be1f32164c --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/GridMultiSelectionScrollBar.java @@ -0,0 +1,34 @@ +/* + * 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.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Grid; + +public class GridMultiSelectionScrollBar extends AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + Grid grid = new Grid(); + grid.addColumn("X").setWidth(39.25d); + grid.addColumn("Hello"); + grid.addColumn("World"); + grid.setFrozenColumnCount(1); + addComponent(grid); + } + +} diff --git a/uitest/src/com/vaadin/tests/components/grid/GridMultiSelectionScrollBarTest.java b/uitest/src/com/vaadin/tests/components/grid/GridMultiSelectionScrollBarTest.java new file mode 100644 index 0000000000..cf85f4eecd --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/grid/GridMultiSelectionScrollBarTest.java @@ -0,0 +1,44 @@ +/* + * 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.junit.Assert.assertTrue; + +import java.io.IOException; + +import org.junit.Test; + +import com.vaadin.testbench.elements.GridElement; +import com.vaadin.tests.tb3.MultiBrowserTest; + +public class GridMultiSelectionScrollBarTest extends MultiBrowserTest { + + @Test + public void testNoVisibleScrollBar() throws IOException { + setDebug(true); + openTestURL(); + + assertTrue( + "Horizontal scrollbar should not be visible.", + $(GridElement.class).first().getHorizontalScroller() + .getAttribute("style").toLowerCase() + .contains("display: none;")); + + // Just to make sure nothing odd happened. + assertNoErrorNotifications(); + } + +} -- cgit v1.2.3