From 17baaf01b7d5b942856f5be8ba21f3c2918b45e5 Mon Sep 17 00:00:00 2001 From: Anna Koskinen Date: Thu, 23 Jul 2020 13:27:11 +0300 Subject: [PATCH] Add column width recalculation when vertical scrollbar hidden/shown. (#12058) - If the Grid has any columns with non-fixed widths, the presence of a vertical scrollbar affects the column width calculations. Horizontal scrollbar should only be shown when actually needed. --- ...ticalScrollbarVisibilityChangeHandler.java | 94 +++++++++++++++++++ .../com/vaadin/client/widgets/Escalator.java | 45 +++++++++ .../java/com/vaadin/client/widgets/Grid.java | 14 ++- .../tests/components/grid/GridSizeChange.java | 6 +- .../tests/VerifyBrowserVersionTest.java | 2 +- .../components/grid/GridSizeChangeTest.java | 6 ++ 6 files changed, 160 insertions(+), 7 deletions(-) create mode 100644 client/src/main/java/com/vaadin/client/widget/grid/events/VerticalScrollbarVisibilityChangeHandler.java diff --git a/client/src/main/java/com/vaadin/client/widget/grid/events/VerticalScrollbarVisibilityChangeHandler.java b/client/src/main/java/com/vaadin/client/widget/grid/events/VerticalScrollbarVisibilityChangeHandler.java new file mode 100644 index 0000000000..1d24fccd2e --- /dev/null +++ b/client/src/main/java/com/vaadin/client/widget/grid/events/VerticalScrollbarVisibilityChangeHandler.java @@ -0,0 +1,94 @@ +/* + * Copyright 2000-2018 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.events; + +import com.google.gwt.event.shared.EventHandler; +import com.google.gwt.event.shared.GwtEvent; + +/** + * FOR INTERNAL USE ONLY, MAY GET REMOVED OR MODIFIED AT ANY TIME! + *

+ * Event handler that gets notified when the visibility of the vertical + * scrollbar of the Escalator changes. + * + * @author Vaadin Ltd + */ +public interface VerticalScrollbarVisibilityChangeHandler + extends EventHandler { + + /** + * FOR INTERNAL USE ONLY, MAY GET REMOVED OR MODIFIED AT ANY TIME! + *

+ * Called when the visibility of the vertical scrollbar of the Escalator + * changes. + * + * @param event + * the row visibility change event describing the change + */ + void onVisibilityChange( + VerticalScrollbarVisibilityChangeEvent event); + + /** + * FOR INTERNAL USE ONLY, MAY GET REMOVED OR MODIFIED AT ANY TIME! + *

+ * Event fired when the visibility of the vertical scrollbar of the + * Escalator changes. + * + * @author Vaadin Ltd + */ + public class VerticalScrollbarVisibilityChangeEvent extends + GwtEvent { + /** + * FOR INTERNAL USE ONLY, MAY GET REMOVED OR MODIFIED AT ANY TIME! + *

+ * The type of this event. + */ + public static final Type TYPE = new Type<>(); + + /** + * FOR INTERNAL USE ONLY, MAY GET REMOVED OR MODIFIED AT ANY TIME! + *

+ * Creates a new Escalator vertical scrollbar visibility change event. + * + */ + public VerticalScrollbarVisibilityChangeEvent() { + // NOP + } + + /* + * (non-Javadoc) + * + * @see com.google.gwt.event.shared.GwtEvent#getAssociatedType() + */ + @Override + public Type getAssociatedType() { + return TYPE; + } + + /* + * (non-Javadoc) + * + * @see + * com.google.gwt.event.shared.GwtEvent#dispatch(com.google.gwt.event. + * shared .EventHandler) + */ + @Override + protected void dispatch( + VerticalScrollbarVisibilityChangeHandler handler) { + handler.onVisibilityChange(this); + } + } +} diff --git a/client/src/main/java/com/vaadin/client/widgets/Escalator.java b/client/src/main/java/com/vaadin/client/widgets/Escalator.java index 2eab5d99c4..7774682dad 100644 --- a/client/src/main/java/com/vaadin/client/widgets/Escalator.java +++ b/client/src/main/java/com/vaadin/client/widgets/Escalator.java @@ -98,6 +98,8 @@ import com.vaadin.client.widget.grid.events.EscalatorSizeChangeHandler; import com.vaadin.client.widget.grid.events.EscalatorSizeChangeHandler.EscalatorSizeChangeEvent; import com.vaadin.client.widget.grid.events.ScrollEvent; import com.vaadin.client.widget.grid.events.ScrollHandler; +import com.vaadin.client.widget.grid.events.VerticalScrollbarVisibilityChangeHandler; +import com.vaadin.client.widget.grid.events.VerticalScrollbarVisibilityChangeHandler.VerticalScrollbarVisibilityChangeEvent; import com.vaadin.client.widgets.Escalator.JsniUtil.TouchHandlerBundle; import com.vaadin.shared.Range; import com.vaadin.shared.ui.grid.HeightMode; @@ -7291,6 +7293,29 @@ public class Escalator extends Widget root.appendChild(verticalScrollbar.getElement()); verticalScrollbar.addScrollHandler(scrollHandler); verticalScrollbar.setScrollbarThickness(scrollbarThickness); + verticalScrollbar + .addVisibilityHandler(new ScrollbarBundle.VisibilityHandler() { + + private boolean queued = false; + + @Override + public void visibilityChanged( + ScrollbarBundle.VisibilityChangeEvent event) { + if (queued) { + return; + } + queued = true; + + /* + * We either lost or gained a scrollbar. In either case, + * we may need to update the column widths. + */ + Scheduler.get().scheduleFinally(() -> { + fireVerticalScrollbarVisibilityChangeEvent(); + queued = false; + }); + } + }); root.appendChild(horizontalScrollbar.getElement()); horizontalScrollbar.addScrollHandler(scrollHandler); @@ -7873,6 +7898,26 @@ public class Escalator extends Widget return array; } + /** + * FOR INTERNAL USE ONLY, MAY GET REMOVED OR MODIFIED AT ANY TIME! + *

+ * Adds an event handler that gets notified when the visibility of the + * vertical scrollbar changes. + * + * @param verticalScrollbarVisibilityChangeHandler + * the event handler + * @return a handler registration for the added handler + */ + public HandlerRegistration addVerticalScrollbarVisibilityChangeHandler( + VerticalScrollbarVisibilityChangeHandler verticalScrollbarVisibilityChangeHandler) { + return addHandler(verticalScrollbarVisibilityChangeHandler, + VerticalScrollbarVisibilityChangeEvent.TYPE); + } + + private void fireVerticalScrollbarVisibilityChangeEvent() { + fireEvent(new VerticalScrollbarVisibilityChangeEvent()); + } + /** * FOR INTERNAL USE ONLY, MAY GET REMOVED OR MODIFIED AT ANY TIME! *

diff --git a/client/src/main/java/com/vaadin/client/widgets/Grid.java b/client/src/main/java/com/vaadin/client/widgets/Grid.java index 1226920bd1..996eae41be 100755 --- a/client/src/main/java/com/vaadin/client/widgets/Grid.java +++ b/client/src/main/java/com/vaadin/client/widgets/Grid.java @@ -4294,6 +4294,7 @@ public class Grid extends ResizeComposite implements HasSelectionHandlers, */ private DataSource dataSource; private Registration changeHandler; + private boolean recalculateColumnWidthsNeeded = false; /** * Currently available row range in DataSource. @@ -5319,8 +5320,7 @@ public class Grid extends ResizeComposite implements HasSelectionHandlers, } else { this.hidden = hidden; - int columnIndex = grid.getVisibleColumns() - .indexOf(this); + int columnIndex = grid.getVisibleColumns().indexOf(this); grid.escalator.getColumnConfiguration() .insertColumns(columnIndex, 1); @@ -6408,6 +6408,15 @@ public class Grid extends ResizeComposite implements HasSelectionHandlers, } }); + escalator.addVerticalScrollbarVisibilityChangeHandler(event -> { + if (!(currentDataAvailable.isEmpty() + && escalator.getBody().getRowCount() > 0)) { + recalculateColumnWidths(); + } else { + recalculateColumnWidthsNeeded = true; + } + }); + // Default action on SelectionEvents. Refresh the body so changed // become visible. addSelectionHandler(new SelectionHandler() { @@ -7247,7 +7256,6 @@ public class Grid extends ResizeComposite implements HasSelectionHandlers, this.dataSource = dataSource; changeHandler = dataSource .addDataChangeHandler(new DataChangeHandler() { - private boolean recalculateColumnWidthsNeeded = false; @Override public void dataUpdated(int firstIndex, int numberOfItems) { diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridSizeChange.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridSizeChange.java index b677cee903..3628c4adaf 100644 --- a/uitest/src/main/java/com/vaadin/tests/components/grid/GridSizeChange.java +++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridSizeChange.java @@ -26,7 +26,7 @@ public class GridSizeChange extends AbstractTestUI { protected void setup(VaadinRequest request) { grid = new Grid<>(); data = new ArrayList<>(); - for (int i = 0; i < 10; ++i) { + for (int i = 0; i < 8; ++i) { data.add(i); ++counter; } @@ -39,8 +39,8 @@ public class GridSizeChange extends AbstractTestUI { // set height mode and height grid.setHeightMode(HeightMode.ROW); - grid.setHeightByRows(10); - grid.setWidth(90, Unit.PIXELS); + grid.setHeightByRows(8); + grid.setWidth(100, Unit.PIXELS); // create a tabsheet with one tab and place grid inside VerticalLayout tab = new VerticalLayout(); diff --git a/uitest/src/test/java/com/vaadin/tests/VerifyBrowserVersionTest.java b/uitest/src/test/java/com/vaadin/tests/VerifyBrowserVersionTest.java index 4fc4106bde..c259e34837 100644 --- a/uitest/src/test/java/com/vaadin/tests/VerifyBrowserVersionTest.java +++ b/uitest/src/test/java/com/vaadin/tests/VerifyBrowserVersionTest.java @@ -25,7 +25,7 @@ public class VerifyBrowserVersionTest extends MultiBrowserTest { // Chrome version does not necessarily match the desired version // because of auto updates... browserIdentifier = getExpectedUserAgentString( - getDesiredCapabilities()) + "83"; + getDesiredCapabilities()) + "84"; } else if (BrowserUtil.isFirefox(getDesiredCapabilities())) { browserIdentifier = getExpectedUserAgentString( getDesiredCapabilities()) + "75"; diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridSizeChangeTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridSizeChangeTest.java index 3b8e485ed9..48a56b5291 100644 --- a/uitest/src/test/java/com/vaadin/tests/components/grid/GridSizeChangeTest.java +++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridSizeChangeTest.java @@ -41,23 +41,27 @@ public class GridSizeChangeTest extends MultiBrowserTest { assertGridWithinTabSheet(); ensureVerticalScrollbar(true); + ensureHorizontalScrollbar(false); $(ButtonElement.class).caption("Remove row").first().click(); // height matches rows -> no scrollbar assertGridWithinTabSheet(); ensureVerticalScrollbar(false); + ensureHorizontalScrollbar(false); $(ButtonElement.class).caption("Reduce width").first().click(); // column too wide -> scrollbar assertGridWithinTabSheet(); + ensureVerticalScrollbar(false); ensureHorizontalScrollbar(true); $(ButtonElement.class).caption("Increase width").first().click(); // column fits -> no scrollbar assertGridWithinTabSheet(); + ensureVerticalScrollbar(false); ensureHorizontalScrollbar(false); $(ButtonElement.class).caption("Add row").first().click(); @@ -65,12 +69,14 @@ public class GridSizeChangeTest extends MultiBrowserTest { assertGridWithinTabSheet(); ensureVerticalScrollbar(true); + ensureHorizontalScrollbar(false); $(ButtonElement.class).caption("Increase height").first().click(); // height matches rows -> no scrollbar assertGridWithinTabSheet(); ensureVerticalScrollbar(false); + ensureHorizontalScrollbar(false); } private void ensureVerticalScrollbar(boolean displayed) { -- 2.39.5