From 9bc7760539339b3be38ebc63d137b16a83d7afe5 Mon Sep 17 00:00:00 2001 From: Anna Miroshnik Date: Thu, 2 Oct 2014 16:02:55 +0400 Subject: Re-adding content in Table causes table to lose scroll position (#14581) At this moment behavior of restoring scroll position more like as in 7.2.6. But restoring is only in case of removeAll() - addAll() (or add() one time - then restore index is reset). In 7.2.6 restoring of scroll position was the result of client defect (scrolling in lazyScroller was not changed if variable "firstvisible" fromserver was 0). This "defect" was fixed in one of the patches. Change-Id: I2e2fb8749ec95f3409caeacafff46c4c29159e74 --- ...ableRepairsScrollPositionOnReAddingAllRows.java | 130 ++++++++++++++++++--- ...RepairsScrollPositionOnReAddingAllRowsTest.java | 119 +++++++++++++++++-- 2 files changed, 228 insertions(+), 21 deletions(-) (limited to 'uitest') diff --git a/uitest/src/com/vaadin/tests/components/table/TableRepairsScrollPositionOnReAddingAllRows.java b/uitest/src/com/vaadin/tests/components/table/TableRepairsScrollPositionOnReAddingAllRows.java index d9cbf007df..df06580dae 100644 --- a/uitest/src/com/vaadin/tests/components/table/TableRepairsScrollPositionOnReAddingAllRows.java +++ b/uitest/src/com/vaadin/tests/components/table/TableRepairsScrollPositionOnReAddingAllRows.java @@ -11,6 +11,12 @@ import com.vaadin.ui.Button; import com.vaadin.ui.Button.ClickListener; import com.vaadin.ui.Table; +/** + * Scroll position should be restored when removing and re-adding all rows in + * Table. + * + * @author Vaadin Ltd + */ public class TableRepairsScrollPositionOnReAddingAllRows extends AbstractTestUI { private static final long serialVersionUID = 1L; @@ -19,32 +25,130 @@ public class TableRepairsScrollPositionOnReAddingAllRows extends AbstractTestUI protected void setup(VaadinRequest request) { final BeanItemContainer cont = new BeanItemContainer( TableItem.class); - final List itemList = new ArrayList(); + final List restoringItemList = new ArrayList(); - Button button1 = new Button("ReAdd rows"); - button1.setId("button1"); - button1.addClickListener(new ClickListener() { + final Table table = new Table(); + table.setWidth("400px"); + table.setPageLength(-1); + table.setContainerDataSource(cont); + table.setSelectable(true); + + Button buttonRestore = new Button("Restore table rows"); + buttonRestore.setId("buttonRestore"); + buttonRestore.addClickListener(new ClickListener() { @Override public void buttonClick(com.vaadin.ui.Button.ClickEvent event) { cont.removeAllItems(); - cont.addAll(itemList); + cont.addAll(restoringItemList); } }); + Button buttonReAddAllViaAddAll = new Button("Re-add rows all at once"); + buttonReAddAllViaAddAll.setId("buttonReAddAllViaAddAll"); + buttonReAddAllViaAddAll.addClickListener(new ClickListener() { + + @Override + public void buttonClick(com.vaadin.ui.Button.ClickEvent event) { + List originalItemIds = new ArrayList(cont + .getItemIds()); + cont.removeAllItems(); + cont.addAll(originalItemIds); + } + }); + + Button buttonReplaceByAnotherCollectionViaAddAll = new Button( + "Replace by another items (via addAll())"); + buttonReplaceByAnotherCollectionViaAddAll + .setId("buttonReplaceByAnotherCollectionViaAddAll"); + buttonReplaceByAnotherCollectionViaAddAll + .addClickListener(new ClickListener() { + + @Override + public void buttonClick( + com.vaadin.ui.Button.ClickEvent event) { + cont.removeAllItems(); + // create new collection (of different items) with other + // size + List itemList = new ArrayList(); + for (int i = 0; i < 79; i++) { + TableItem ti = new TableItem(); + ti.setName("AnotherItem1_" + i); + itemList.add(ti); + } + cont.addAll(itemList); + } + }); + + Button buttonReplaceByAnotherCollectionViaAdd = new Button( + "Replace by another items (via add(), add()..)"); + buttonReplaceByAnotherCollectionViaAdd + .setId("buttonReplaceByAnotherCollectionViaAdd"); + buttonReplaceByAnotherCollectionViaAdd + .addClickListener(new ClickListener() { + + @Override + public void buttonClick( + com.vaadin.ui.Button.ClickEvent event) { + cont.removeAllItems(); + for (int i = 0; i < 81; i++) { + TableItem ti = new TableItem(); + ti.setName("AnotherItem2_" + i); + // add one by one in container + cont.addBean(ti); + } + } + }); + + Button buttonReplaceBySubsetOfSmallerSize = new Button( + "Replace rows by sub-set of smaller size (size not enought for restoring scroll position)"); + buttonReplaceBySubsetOfSmallerSize + .setId("buttonReplaceBySubsetOfSmallerSize"); + buttonReplaceBySubsetOfSmallerSize + .addClickListener(new ClickListener() { + + @Override + public void buttonClick( + com.vaadin.ui.Button.ClickEvent event) { + cont.removeAllItems(); + cont.addAll(restoringItemList.subList(0, 20)); + } + }); + + Button buttonReplaceByWholeSubsetPlusOnNew = new Button( + "Replace rows by whole subset plus one new item"); + buttonReplaceByWholeSubsetPlusOnNew + .setId("buttonReplaceByWholeSubsetPlusOnNew"); + buttonReplaceByWholeSubsetPlusOnNew + .addClickListener(new ClickListener() { + + @Override + public void buttonClick( + com.vaadin.ui.Button.ClickEvent event) { + cont.removeAllItems(); + + List list = new ArrayList( + restoringItemList); + TableItem ti = new TableItem(); + ti.setName("AnotherItem3_" + 80); + list.add(ti); + cont.addAll(list); + } + }); + for (int i = 0; i < 80; i++) { TableItem ti = new TableItem(); - ti.setName("Name_" + i); - itemList.add(ti); + ti.setName("Item_" + i); + restoringItemList.add(ti); cont.addBean(ti); } - final Table table = new Table(); - table.setPageLength(-1); - table.setContainerDataSource(cont); - table.setSelectable(true); - - getLayout().addComponent(button1); + getLayout().addComponent(buttonReAddAllViaAddAll); + getLayout().addComponent(buttonReplaceByAnotherCollectionViaAddAll); + getLayout().addComponent(buttonReplaceByAnotherCollectionViaAdd); + getLayout().addComponent(buttonReplaceBySubsetOfSmallerSize); + getLayout().addComponent(buttonReplaceByWholeSubsetPlusOnNew); + getLayout().addComponent(buttonRestore); getLayout().addComponent(table); } diff --git a/uitest/src/com/vaadin/tests/components/table/TableRepairsScrollPositionOnReAddingAllRowsTest.java b/uitest/src/com/vaadin/tests/components/table/TableRepairsScrollPositionOnReAddingAllRowsTest.java index 2fbdd8b581..a3e7f29dfe 100644 --- a/uitest/src/com/vaadin/tests/components/table/TableRepairsScrollPositionOnReAddingAllRowsTest.java +++ b/uitest/src/com/vaadin/tests/components/table/TableRepairsScrollPositionOnReAddingAllRowsTest.java @@ -31,6 +31,12 @@ import com.vaadin.testbench.screenshot.ImageComparison; import com.vaadin.testbench.screenshot.ReferenceNameGenerator; import com.vaadin.tests.tb3.MultiBrowserTest; +/** + * Scroll position should be restored when removing and re-adding all rows in + * Table. + * + * @author Vaadin Ltd + */ public class TableRepairsScrollPositionOnReAddingAllRowsTest extends MultiBrowserTest { @@ -39,29 +45,126 @@ public class TableRepairsScrollPositionOnReAddingAllRowsTest extends throws InterruptedException { openTestURL(); - WebElement buttonReAddRows = findElement(By.id("button1")); + WebElement row0 = $(TableElement.class).first().getCell(0, 0); + int rowLocation0 = row0.getLocation().getY(); + // case 1 scrollUp(); waitUntilNot(new ExpectedCondition() { @Override public Boolean apply(WebDriver input) { - return $(TableElement.class).first().getCell(49, 0) == null; + return $(TableElement.class).first().getCell(48, 0) == null; } }, 10); - WebElement row = $(TableElement.class).first().getCell(49, 0); + WebElement row = $(TableElement.class).first().getCell(48, 0); int rowLocation = row.getLocation().getY(); - buttonReAddRows.click(); + // This button is for re-adding all rows (original itemIds) at once + // (removeAll() + addAll()) + hitButton("buttonReAddAllViaAddAll"); - row = $(TableElement.class).first().getCell(49, 0); + row = $(TableElement.class).first().getCell(48, 0); int newRowLocation = row.getLocation().getY(); // ranged check because IE9 consistently misses the mark by 1 pixel assertThat( - "Scroll position should be the same as before Re-Adding all rows", - (double) newRowLocation, closeTo(rowLocation, 1)); + "Scroll position should be the same as before Re-Adding rows via addAll()", + (double) newRowLocation, + closeTo(rowLocation, row.getSize().height + 1)); + + // case 2 + scrollUp(); + + waitUntilNot(new ExpectedCondition() { + @Override + public Boolean apply(WebDriver input) { + return $(TableElement.class).first().getCell(48, 0) == null; + } + }, 10); + + row = $(TableElement.class).first().getCell(48, 0); + rowLocation = row.getLocation().getY(); + + // This button is for replacing all rows at once (removeAll() + + // addAll()) + hitButton("buttonReplaceByAnotherCollectionViaAddAll"); + + row = $(TableElement.class).first().getCell(48, 0); + newRowLocation = row.getLocation().getY(); + + // ranged check because IE9 consistently misses the mark by 1 pixel + assertThat( + "Scroll position should be the same as before Replacing rows via addAll()", + (double) newRowLocation, + closeTo(rowLocation, row.getSize().height + 1)); + + // case 3 + // This button is for replacing all rows one by one (removeAll() + add() + // + add()..) + hitButton("buttonReplaceByAnotherCollectionViaAdd"); + + row = $(TableElement.class).first().getCell(0, 0); + newRowLocation = row.getLocation().getY(); + + // ranged check because IE9 consistently misses the mark by 1 pixel + assertThat("Scroll position should be 0", (double) newRowLocation, + closeTo(rowLocation0, 1)); + + // case 4 + // This button is for restoring initial list and for scrolling to 0 + // position + hitButton("buttonRestore"); + scrollUp(); + + waitUntilNot(new ExpectedCondition() { + @Override + public Boolean apply(WebDriver input) { + return $(TableElement.class).first().getCell(48, 0) == null; + } + }, 10); + + // This button is for replacing all rows at once but the count of rows + // is less then first index to scroll + hitButton("buttonReplaceBySubsetOfSmallerSize"); + + row = $(TableElement.class).first().getCell(5, 0); + + newRowLocation = row.getLocation().getY(); + + // ranged check because IE9 consistently misses the mark by 1 pixel + assertThat("Scroll position should be 0", (double) newRowLocation, + closeTo(rowLocation0, 1)); + + // case 5 + // This button is for restoring initial list and for scrolling to 0 + // position + hitButton("buttonRestore"); + scrollUp(); + + waitUntilNot(new ExpectedCondition() { + @Override + public Boolean apply(WebDriver input) { + return $(TableElement.class).first().getCell(48, 0) == null; + } + }, 10); + + row = $(TableElement.class).first().getCell(48, 0); + rowLocation = row.getLocation().getY(); + + // This button is for replacing by whole original sub-set of items plus + // one new + hitButton("buttonReplaceByWholeSubsetPlusOnNew"); + + row = $(TableElement.class).first().getCell(48, 0); + newRowLocation = row.getLocation().getY(); + + // ranged check because IE9 consistently misses the mark by 1 pixel + assertThat("Scroll position should be the same as before Replacing", + (double) newRowLocation, + closeTo(rowLocation, row.getSize().height + 1)); + } private void scrollUp() { @@ -69,6 +172,6 @@ public class TableRepairsScrollPositionOnReAddingAllRowsTest extends By.className("v-table-body-wrapper")); JavascriptExecutor js = new TestBenchCommandExecutor(getDriver(), new ImageComparison(), new ReferenceNameGenerator()); - js.executeScript("arguments[0].scrollTop = " + 1200, actualElement); + js.executeScript("arguments[0].scrollTop = " + 1205, actualElement); } } -- cgit v1.2.3