From 533ddcda271b7226b38c035adf3073062c562653 Mon Sep 17 00:00:00 2001 From: Tapio Aali Date: Tue, 5 Nov 2013 12:17:06 +0200 Subject: Fixed lost scrollLeft when row count changed in Table (#12652). Change-Id: I868f56c1e7003c6619859ba46619f4c53ef9744e --- client/src/com/vaadin/client/Util.java | 37 +++++++ client/src/com/vaadin/client/ui/VScrollTable.java | 47 ++++++++- ...bleHorizontalScrollPositionOnItemSetChange.html | 110 +++++++++++++++++++++ ...bleHorizontalScrollPositionOnItemSetChange.java | 101 +++++++++++++++++++ 4 files changed, 294 insertions(+), 1 deletion(-) create mode 100644 uitest/src/com/vaadin/tests/components/table/TableHorizontalScrollPositionOnItemSetChange.html create mode 100644 uitest/src/com/vaadin/tests/components/table/TableHorizontalScrollPositionOnItemSetChange.java diff --git a/client/src/com/vaadin/client/Util.java b/client/src/com/vaadin/client/Util.java index 9cdfa954c6..fd7a354569 100644 --- a/client/src/com/vaadin/client/Util.java +++ b/client/src/com/vaadin/client/Util.java @@ -448,6 +448,35 @@ public class Util { return detectedScrollbarSize; } + /** + * Calculates maximum horizontal scrolling value for the given element. + * + * @since 7.1.9 + * @param element + * which scrollLeft should be calculated + * @return maximum value for scrollLeft of the given element + */ + public static int getMaxScrollLeft(final Element element) { + int scrollWidth = element.getScrollWidth(); + int clientWidth = element.getClientWidth(); + return scrollWidth - clientWidth; + } + + /** + * Checks if scrollLeft of the element is at its maximum value. Returns + * false if the element can't be scrolled horizontally. + * + * @since 7.1.9 + * @param element + * which scrollLeft should be checked + * @return true, if scrollLeft is at maximum (false if element can't be + * scrolled horizontally) + */ + public static boolean isScrollLeftAtMax(final Element element) { + int scrollLeft = element.getScrollLeft(); + return scrollLeft != 0 && scrollLeft == getMaxScrollLeft(element); + } + /** * Run workaround for webkits overflow auto issue. * @@ -469,6 +498,8 @@ public class Util { // check the scrolltop value before hiding the element final int scrolltop = elem.getScrollTop(); final int scrollleft = elem.getScrollLeft(); + final boolean scrollLeftAtMax = isScrollLeftAtMax(elem); + elem.getStyle().setProperty("overflow", "hidden"); Scheduler.get().scheduleDeferred(new Command() { @@ -492,6 +523,12 @@ public class Util { elem.setScrollTop(scrollvalue); } + // keep horizontal scroll at max if it was before vertical + // scroll bar was added/removed + if (scrollLeftAtMax) { + elem.setScrollLeft(getMaxScrollLeft(elem)); + } + // fix for #6940 : Table horizontal scroll sometimes not // updated when collapsing/expanding columns // Also appeared in Safari 5.1 with webkit 534 (#7667) diff --git a/client/src/com/vaadin/client/ui/VScrollTable.java b/client/src/com/vaadin/client/ui/VScrollTable.java index 0304acd590..48fd85144f 100644 --- a/client/src/com/vaadin/client/ui/VScrollTable.java +++ b/client/src/com/vaadin/client/ui/VScrollTable.java @@ -276,6 +276,10 @@ public class VScrollTable extends FlowPanel implements HasWidgets, */ private int detachedScrollPosition = 0; + // fields used in fixing erroneously lost scrollLeft + int lastScrollBodyHeight = 0; + boolean lastScrollLeftWasAtMax = false; + /** * Represents a select range of rows */ @@ -1005,6 +1009,15 @@ public class VScrollTable extends FlowPanel implements HasWidgets, initialContentReceived = true; sizeNeedsInit = true; scrollBody.restoreRowVisibility(); + + // At least FireFox requires that scrollLeft is restored deferred after + // scrollBody is recreated + Scheduler.get().scheduleFinally(new ScheduledCommand() { + @Override + public void execute() { + restoreScrollLeft(); + } + }); } /** For internal use only. May be removed or replaced in the future. */ @@ -6864,13 +6877,45 @@ public class VScrollTable extends FlowPanel implements HasWidgets, return s; } + /** + * Tries to restore horizontal scroll position if it was lost due to change + * in the height of scrollBody (#12652). + */ + private void restoreScrollLeft() { + int upcomingScrollLeft = scrollLeft; + + if (lastScrollLeftWasAtMax) { + upcomingScrollLeft = Util.getMaxScrollLeft(scrollBodyPanel + .getElement()); + } + scrollBodyPanel.getElement().setScrollLeft(upcomingScrollLeft); + } + + /** + * Checks if restore of scrollLeft is needed by checking if height of the + * scrollBody has changed. + * + * @return true, if restore is required + */ + private boolean isScrollLeftRestoreRequired() { + return (scrollBody.getElement().getClientHeight() != lastScrollBodyHeight); + } + /** * This method has logic which rows needs to be requested from server when * user scrolls */ - @Override public void onScroll(ScrollEvent event) { + // restore in initializeRows() doesn't work right with Chrome + if (isScrollLeftRestoreRequired()) { + restoreScrollLeft(); + } + + lastScrollBodyHeight = scrollBody.getElement().getClientHeight(); + lastScrollLeftWasAtMax = Util.isScrollLeftAtMax(scrollBodyPanel + .getElement()); + scrollLeft = scrollBodyPanel.getElement().getScrollLeft(); scrollTop = scrollBodyPanel.getScrollPosition(); /* diff --git a/uitest/src/com/vaadin/tests/components/table/TableHorizontalScrollPositionOnItemSetChange.html b/uitest/src/com/vaadin/tests/components/table/TableHorizontalScrollPositionOnItemSetChange.html new file mode 100644 index 0000000000..6fd54ba0ca --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/table/TableHorizontalScrollPositionOnItemSetChange.html @@ -0,0 +1,110 @@ + + + + + + +TableHorizontalScrollPositionOnItemSetChange + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
TableHorizontalScrollPositionOnItemSetChange
open/run/com.vaadin.tests.components.table.TableHorizontalScrollPositionOnItemSetChange?restartApplication
scrollLeftvaadin=runcomvaadintestscomponentstableTableHorizontalScrollPositionOnItemSetChange::PID_Shorscrolltable/domChild[1]326
pause500
clickvaadin=runcomvaadintestscomponentstableTableHorizontalScrollPositionOnItemSetChange::PID_Slessitems/domChild[0]/domChild[0]
pause500
screenCaptureleft-scroll-position-stays-in-middle-with-fewer-items
clickvaadin=runcomvaadintestscomponentstableTableHorizontalScrollPositionOnItemSetChange::PID_Smoreitems/domChild[0]/domChild[0]
pause500
screenCaptureleft-scroll-position-stays-in-middle-with-more-items
scrollLeftvaadin=runcomvaadintestscomponentstableTableHorizontalScrollPositionOnItemSetChange::PID_Shorscrolltable/domChild[1]653
pause500
clickvaadin=runcomvaadintestscomponentstableTableHorizontalScrollPositionOnItemSetChange::PID_Slessitems/domChild[0]/domChild[0]
pause500
screenCaptureleft-scroll-position-stays-max-with-fewer-items
clickvaadin=runcomvaadintestscomponentstableTableHorizontalScrollPositionOnItemSetChange::PID_Smoreitems/domChild[0]/domChild[0]
pause500
screenCaptureleft-scroll-position-stays-max-with-more-items
scrollLeftvaadin=runcomvaadintestscomponentstableTableHorizontalScrollPositionOnItemSetChange::PID_Shorscrolltable/domChild[1]0
+ + diff --git a/uitest/src/com/vaadin/tests/components/table/TableHorizontalScrollPositionOnItemSetChange.java b/uitest/src/com/vaadin/tests/components/table/TableHorizontalScrollPositionOnItemSetChange.java new file mode 100644 index 0000000000..1f59a84428 --- /dev/null +++ b/uitest/src/com/vaadin/tests/components/table/TableHorizontalScrollPositionOnItemSetChange.java @@ -0,0 +1,101 @@ +/* + * Copyright 2000-2013 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.table; + +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.HorizontalLayout; +import com.vaadin.ui.Table; +import com.vaadin.ui.VerticalLayout; + +public class TableHorizontalScrollPositionOnItemSetChange extends + AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + VerticalLayout layout = new VerticalLayout(); + layout.setMargin(true); + layout.setSpacing(true); + setContent(layout); + + final Table table = new Table(); + table.setWidth("640px"); + table.setHeight("243px"); + table.setId("horscrolltable"); + layout.addComponent(table); + + for (int i = 0; i < 15; i++) { + table.addContainerProperty("Column " + i, String.class, null); + } + + for (int i = 0; i < 60; i++) { + table.addItem(); + } + + Button lessItems = new Button("Less items", new Button.ClickListener() { + + @Override + public void buttonClick(Button.ClickEvent event) { + table.removeAllItems(); + for (int i = 0; i < 5; i++) { + table.addItem(); + } + } + }); + lessItems.setId("lessitems"); + + Button moreItems = new Button("More items", new Button.ClickListener() { + + @Override + public void buttonClick(Button.ClickEvent event) { + table.removeAllItems(); + for (int i = 0; i < 50; i++) { + table.addItem(); + } + } + }); + moreItems.setId("moreitems"); + + Button clearItems = new Button("Clear all", new Button.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + table.removeAllItems(); + } + }); + + HorizontalLayout buttonLayout = new HorizontalLayout(); + buttonLayout.setSpacing(true); + layout.addComponent(buttonLayout); + + buttonLayout.addComponent(lessItems); + buttonLayout.addComponent(moreItems); + buttonLayout.addComponent(clearItems); + clearItems.setId("clearitems"); + } + + @Override + protected String getTestDescription() { + return "Horizontal scrolling position should not be lost if amount of items changes in Table."; + } + + @Override + protected Integer getTicketNumber() { + return 12652; + } + +} -- cgit v1.2.3