diff options
3 files changed, 356 insertions, 48 deletions
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 9bee0bbeec..5c3bf593fb 100644 --- a/client/src/main/java/com/vaadin/client/widgets/Escalator.java +++ b/client/src/main/java/com/vaadin/client/widgets/Escalator.java @@ -5102,14 +5102,18 @@ public class Escalator extends Widget case START: // target row at the top of the viewport, include buffer // row if there is room for one - logicalTargetIndex = Math + newTopRowLogicalIndex = Math .max(firstVisibleIndexIfScrollingUp - 1, 0); - newTopRowLogicalIndex = logicalTargetIndex; + if (getVisibleRowRange().contains(newTopRowLogicalIndex)) { + logicalTargetIndex = oldTopRowLogicalIndex + + visualRangeLength; + } else { + logicalTargetIndex = newTopRowLogicalIndex; + } break; default: - throw new IllegalArgumentException( - "Internal: Unsupported ScrollDestination: " - + destination.name()); + String msg = "Internal: Unsupported ScrollDestination: "; + throw new IllegalArgumentException(msg + destination.name()); } // adjust visual range if necessary @@ -5125,12 +5129,8 @@ public class Escalator extends Widget boolean rowsWereMoved = newTopRowLogicalIndex != oldTopRowLogicalIndex; // update scroll position if necessary - double scrollTop = calculateScrollPositionForScrollToRowSpacerOrBoth( - targetRowIndex, destination, padding, scrollType); - if (scrollTop != getScrollTop()) { - setScrollTop(scrollTop); - setBodyScrollPosition(tBodyScrollLeft, scrollTop); - } + adjustScrollPositionForScrollToRowSpacerOrBoth(targetRowIndex, + destination, padding, scrollType); if (rowsWereMoved) { fireRowVisibilityChangeEvent(); @@ -5217,7 +5217,7 @@ public class Escalator extends Widget } /** - * Calculates scroll position for + * Adjusts scroll position for * {@link #scrollToRowSpacerOrBoth(int, ScrollDestination, double, boolean, boolean)}, * reuse at your own peril. * @@ -5225,9 +5225,8 @@ public class Escalator extends Widget * @param destination * @param padding * @param scrollType - * @return expected scroll position */ - private double calculateScrollPositionForScrollToRowSpacerOrBoth( + private void adjustScrollPositionForScrollToRowSpacerOrBoth( int targetRowIndex, ScrollDestination destination, double padding, ScrollType scrollType) { /* @@ -5281,10 +5280,7 @@ public class Escalator extends Widget - sectionHeight + padding; // ensure that we don't overshoot beyond bottom scrollTop = Math.min(scrollTop, - getRowTop(getRowCount() - 1) + getDefaultRowHeight() - + spacerContainer - .getSpacerHeight(getRowCount() - 1) - - sectionHeight); + getRowTop(getRowCount()) - sectionHeight); // if padding is set we want to overshoot or undershoot, // otherwise make sure the top of the row or spacer is // in view @@ -5304,26 +5300,15 @@ public class Escalator extends Widget case END: if (ScrollType.ROW.equals(scrollType) && rowTop + getDefaultRowHeight() - + padding > getScrollTop() + sectionHeight) { - // within visual range but end of row below the viewport - // or not enough padding, shift a little + + padding != getScrollTop() + sectionHeight) { + // row should be at the bottom of the viewport scrollTop = rowTop + getDefaultRowHeight() - sectionHeight + padding; - // ensure that we don't overshoot beyond bottom - scrollTop = Math.min(scrollTop, - getRowTop(getRowCount() - 1) + getDefaultRowHeight() - + spacerContainer - .getSpacerHeight(getRowCount() - 1) - - sectionHeight); } else if (rowTop + getDefaultRowHeight() + spacerHeight - + padding > getScrollTop() + sectionHeight) { - // within visual range but end of spacer below the viewport - // or not enough padding, shift a little + + padding != getScrollTop() + sectionHeight) { + // spacer should be at the bottom of the viewport scrollTop = rowTop + getDefaultRowHeight() + spacerHeight - sectionHeight + padding; - // ensure that we don't overshoot beyond bottom - scrollTop = Math.min(scrollTop, - getRowTop(getRowCount()) - sectionHeight); } else { // we are fine where we are scrollTop = getScrollTop(); @@ -5344,26 +5329,16 @@ public class Escalator extends Widget + (spacerHeight / 2.0); } scrollTop = center - Math.ceil(sectionHeight / 2.0); - // ensure that we don't overshoot beyond bottom - scrollTop = Math.min(scrollTop, - getRowTop(getRowCount() - 1) + getDefaultRowHeight() - + spacerContainer - .getSpacerHeight(getRowCount() - 1) - - sectionHeight); - // ensure that we don't overshoot beyond top - scrollTop = Math.max(0, scrollTop); break; case START: if (!ScrollType.SPACER.equals(scrollType) - && Math.max(rowTop - padding, 0) < getScrollTop()) { - // row top above the viewport or not enough padding, shift a - // little + && Math.max(rowTop - padding, 0) != getScrollTop()) { + // row should be at the top of the viewport scrollTop = Math.max(rowTop - padding, 0); } else if (ScrollType.SPACER.equals(scrollType) && Math.max(rowTop + getDefaultRowHeight() - padding, - 0) < getScrollTop()) { - // spacer top above the viewport or not enough padding, - // shift a little + 0) != getScrollTop()) { + // spacer should be at the top of the viewport scrollTop = Math .max(rowTop + getDefaultRowHeight() - padding, 0); } else { @@ -5373,7 +5348,16 @@ public class Escalator extends Widget default: scrollTop = getScrollTop(); } - return scrollTop; + // ensure that we don't overshoot beyond bottom + scrollTop = Math.min(scrollTop, + getRowTop(getRowCount()) - sectionHeight); + // ensure that we don't overshoot beyond top + scrollTop = Math.max(0, scrollTop); + + if (scrollTop != getScrollTop()) { + setScrollTop(scrollTop); + setBodyScrollPosition(tBodyScrollLeft, scrollTop); + } } @Override diff --git a/uitest/src/main/java/com/vaadin/tests/components/grid/GridScrollDestination.java b/uitest/src/main/java/com/vaadin/tests/components/grid/GridScrollDestination.java new file mode 100644 index 0000000000..f58ffa1fe7 --- /dev/null +++ b/uitest/src/main/java/com/vaadin/tests/components/grid/GridScrollDestination.java @@ -0,0 +1,33 @@ +package com.vaadin.tests.components.grid; + +import java.util.Arrays; +import java.util.stream.IntStream; + +import com.vaadin.server.VaadinRequest; +import com.vaadin.shared.ui.grid.ScrollDestination; +import com.vaadin.tests.components.AbstractTestUI; +import com.vaadin.ui.Button; +import com.vaadin.ui.Grid; +import com.vaadin.ui.NativeSelect; +import com.vaadin.ui.TextField; + +public class GridScrollDestination extends AbstractTestUI { + + @Override + protected void setup(VaadinRequest request) { + Grid<Integer> grid = new Grid<>(); + grid.addColumn(Integer::intValue).setCaption("number"); + grid.setItems(IntStream.range(0, 100).boxed()); + TextField tf = new TextField("row index", "50"); + NativeSelect<ScrollDestination> ns = new NativeSelect<>( + "scroll destination", + Arrays.asList(ScrollDestination.values())); + ns.setValue(ScrollDestination.ANY); + Button button = new Button("Scroll to above row index", (event) -> { + int rowIndex = Integer.parseInt(tf.getValue()); + grid.scrollTo(rowIndex, ns.getValue()); + }); + addComponents(tf, ns, button, grid); + } + +} diff --git a/uitest/src/test/java/com/vaadin/tests/components/grid/GridScrollDestinationTest.java b/uitest/src/test/java/com/vaadin/tests/components/grid/GridScrollDestinationTest.java new file mode 100644 index 0000000000..67a831557d --- /dev/null +++ b/uitest/src/test/java/com/vaadin/tests/components/grid/GridScrollDestinationTest.java @@ -0,0 +1,291 @@ +package com.vaadin.tests.components.grid; + +import static org.hamcrest.number.IsCloseTo.closeTo; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; + +import java.util.List; + +import org.junit.Test; +import org.openqa.selenium.WebElement; + +import com.vaadin.shared.ui.grid.ScrollDestination; +import com.vaadin.testbench.By; +import com.vaadin.testbench.TestBenchElement; +import com.vaadin.testbench.elements.ButtonElement; +import com.vaadin.testbench.elements.GridElement; +import com.vaadin.testbench.elements.NativeSelectElement; +import com.vaadin.tests.tb3.SingleBrowserTest; + +public class GridScrollDestinationTest extends SingleBrowserTest { + + private ButtonElement button; + private GridElement grid; + private TestBenchElement header; + private TestBenchElement tableWrapper; + + @Override + public void setup() throws Exception { + super.setup(); + openTestURL(); + button = $(ButtonElement.class).first(); + grid = $(GridElement.class).first(); + header = grid.getHeader(); + tableWrapper = grid.getTableWrapper(); + } + + private void assertElementAtTop(WebElement row) { + assertThat((double) row.getLocation().getY(), closeTo( + header.getLocation().getY() + header.getSize().getHeight(), + 1d)); + } + + private void assertElementAtBottom(WebElement row) { + assertThat( + (double) row.getLocation().getY() + row.getSize().getHeight(), + closeTo((double) tableWrapper.getLocation().getY() + + tableWrapper.getSize().getHeight(), 1d)); + } + + private void assertElementAtMiddle(WebElement row) { + assertThat((double) row.getLocation() + .getY() + (row.getSize().getHeight() / 2), closeTo( + (double) tableWrapper.getLocation().getY() + + header.getSize().getHeight() + + ((tableWrapper.getSize().getHeight() + - header.getSize().getHeight()) / 2), + 1d)); + } + + @Test + public void destinationAny() { + // ScrollDestination.ANY selected by default + + // scroll down + button.click(); + + // expect the row at the bottom of the viewport + List<WebElement> rows = grid.getBody() + .findElements(By.className("v-grid-row")); + // last rendered row is a buffer row, inspect second to last + WebElement row = rows.get(rows.size() - 2); + assertEquals("50", row.getText()); + + assertElementAtBottom(row); + + // scroll to end + grid.scrollToRow((int) grid.getRowCount() - 1); + + // ensure row 50 is out of visual range, first two rows are out of view + // and getText can't find the contents so inspect the third row + rows = grid.getBody().findElements(By.className("v-grid-row")); + row = rows.get(2); + + assertGreater(row.getText() + " is not greater than 52", + Integer.valueOf(row.getText()), 52); + + // scroll up + button.click(); + + // expect the row at the top of the viewport + rows = grid.getBody().findElements(By.className("v-grid-row")); + // first rendered row is a buffer row, inspect second + row = rows.get(1); + assertEquals("50", row.getText()); + + assertElementAtTop(row); + + // scroll up by a few rows + grid.scrollToRow(45); + + // refresh row references + rows = grid.getBody().findElements(By.className("v-grid-row")); + row = rows.get(6); + assertEquals("50", row.getText()); + + // scroll while already within viewport + button.click(); + + // expect no change since the row is still within viewport + rows = grid.getBody().findElements(By.className("v-grid-row")); + row = rows.get(6); + assertEquals("50", row.getText()); + } + + @Test + public void destinationEnd() { + $(NativeSelectElement.class).first() + .selectByText(ScrollDestination.END.name()); + + // scroll down + button.click(); + + // expect the row at the bottom of the viewport + List<WebElement> rows = grid.getBody() + .findElements(By.className("v-grid-row")); + // last rendered row is a buffer row, inspect second to last + WebElement row = rows.get(rows.size() - 2); + assertEquals("50", row.getText()); + + assertElementAtBottom(row); + + // scroll to end + grid.scrollToRow((int) grid.getRowCount() - 1); + + // ensure row 50 is out of visual range, first two rows are out of view + // and getText can't find the contents so inspect the third row + rows = grid.getBody().findElements(By.className("v-grid-row")); + row = rows.get(2); + + assertGreater(row.getText() + " is not greater than 52", + Integer.valueOf(row.getText()), 52); + + // scroll up + button.click(); + + // expect the row at the bottom of the viewport + rows = grid.getBody().findElements(By.className("v-grid-row")); + // last rendered row is a buffer row, inspect second to last + row = rows.get(rows.size() - 2); + assertEquals("50", row.getText()); + + assertElementAtBottom(row); + + // scroll down by a few rows + grid.scrollToRow(55); + + // refresh row references + rows = grid.getBody().findElements(By.className("v-grid-row")); + row = rows.get(rows.size() - 7); + assertEquals("50", row.getText()); + + // scroll while already within viewport + button.click(); + + // expect the row at the bottom of the viewport again + rows = grid.getBody().findElements(By.className("v-grid-row")); + row = rows.get(rows.size() - 2); + assertEquals("50", row.getText()); + + assertElementAtBottom(row); + } + + @Test + public void destinationStart() { + $(NativeSelectElement.class).first() + .selectByText(ScrollDestination.START.name()); + + // scroll down + button.click(); + + // expect the row at the top of the viewport + List<WebElement> rows = grid.getBody() + .findElements(By.className("v-grid-row")); + // first rendered row is a buffer row, inspect second + WebElement row = rows.get(1); + assertEquals("50", row.getText()); + + assertElementAtTop(row); + + // scroll to end + grid.scrollToRow((int) grid.getRowCount() - 1); + + // ensure row 50 is out of visual range, first two rows are out of view + // and getText can't find the contents so inspect the third row + rows = grid.getBody().findElements(By.className("v-grid-row")); + row = rows.get(2); + + assertGreater(row.getText() + " is not greater than 52", + Integer.valueOf(row.getText()), 52); + + // scroll up + button.click(); + + // expect the row at the top of the viewport + rows = grid.getBody().findElements(By.className("v-grid-row")); + // first rendered row is a buffer row, inspect second + row = rows.get(1); + assertEquals("50", row.getText()); + + assertElementAtTop(row); + + // scroll up by a few rows + grid.scrollToRow(45); + + // refresh row references + rows = grid.getBody().findElements(By.className("v-grid-row")); + row = rows.get(6); + assertEquals("50", row.getText()); + + // scroll while already within viewport + button.click(); + + // expect the row at the top of the viewport again + rows = grid.getBody().findElements(By.className("v-grid-row")); + row = rows.get(1); + assertEquals("50", row.getText()); + + assertElementAtTop(row); + } + + @Test + public void destinationMiddle() { + NativeSelectElement destinationSelect = $(NativeSelectElement.class) + .first(); + destinationSelect.selectByText(ScrollDestination.MIDDLE.name()); + + // scroll down + button.click(); + + // expect the row at the middle of the viewport + List<WebElement> rows = grid.getBody() + .findElements(By.className("v-grid-row")); + // inspect the middle row + WebElement row = rows.get(rows.size() / 2); + assertEquals("50", row.getText()); + + assertElementAtMiddle(row); + + // scroll to end + grid.scrollToRow((int) grid.getRowCount() - 1); + + // ensure row 50 is out of visual range, first two rows are out of view + // and getText can't find the contents so inspect the third row + rows = grid.getBody().findElements(By.className("v-grid-row")); + row = rows.get(2); + + assertGreater(row.getText() + " is not greater than 52", + Integer.valueOf(row.getText()), 52); + + // scroll up + button.click(); + + // expect the row at the middle of the viewport + rows = grid.getBody().findElements(By.className("v-grid-row")); + // first rendered row is a buffer row, inspect second + row = rows.get(rows.size() / 2); + assertEquals("50", row.getText()); + + assertElementAtMiddle(row); + + // scroll down by a few rows + destinationSelect.selectByText(ScrollDestination.START.name()); + button.click(); + + // refresh row references + rows = grid.getBody().findElements(By.className("v-grid-row")); + row = rows.get(1); + assertEquals("50", row.getText()); + + // scroll while already within viewport + destinationSelect.selectByText(ScrollDestination.MIDDLE.name()); + button.click(); + + // expect the row at the top of the viewport again + rows = grid.getBody().findElements(By.className("v-grid-row")); + row = rows.get(rows.size() / 2); + assertEquals("50", row.getText()); + + assertElementAtMiddle(row); + } +} |