From 81a1c293f0e407db4a5443b85e57d32cddd397a7 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Leif=20=C3=85strand?= Date: Fri, 15 Nov 2013 09:46:42 +0000 Subject: [PATCH] Revert "Fixed lost scrollLeft when row count changed in Table (#12652)." This reverts commit 533ddcda271b7226b38c035adf3073062c562653. Seems like the caused regressions are far from trivial to resolve. A new approach might be needed to resolve this issue. Change-Id: I88cf75608e8d47fffab5a366a8ad1b3b70c1c11f --- client/src/com/vaadin/client/Util.java | 37 ------ .../com/vaadin/client/ui/VScrollTable.java | 47 +------- ...rizontalScrollPositionOnItemSetChange.html | 110 ------------------ ...rizontalScrollPositionOnItemSetChange.java | 101 ---------------- 4 files changed, 1 insertion(+), 294 deletions(-) delete mode 100644 uitest/src/com/vaadin/tests/components/table/TableHorizontalScrollPositionOnItemSetChange.html delete 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 7c7978be09..8972670232 100644 --- a/client/src/com/vaadin/client/Util.java +++ b/client/src/com/vaadin/client/Util.java @@ -447,35 +447,6 @@ 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. * @@ -497,8 +468,6 @@ 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() { @@ -522,12 +491,6 @@ 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 d1d73f4e91..c56a2a8772 100644 --- a/client/src/com/vaadin/client/ui/VScrollTable.java +++ b/client/src/com/vaadin/client/ui/VScrollTable.java @@ -276,10 +276,6 @@ 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 */ @@ -1009,15 +1005,6 @@ 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. */ @@ -6877,45 +6864,13 @@ 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 deleted file mode 100644 index 6fd54ba0ca..0000000000 --- a/uitest/src/com/vaadin/tests/components/table/TableHorizontalScrollPositionOnItemSetChange.html +++ /dev/null @@ -1,110 +0,0 @@ - - - - - - -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 deleted file mode 100644 index 1f59a84428..0000000000 --- a/uitest/src/com/vaadin/tests/components/table/TableHorizontalScrollPositionOnItemSetChange.java +++ /dev/null @@ -1,101 +0,0 @@ -/* - * 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; - } - -} -- 2.39.5