From bf3d0d830343f5e1250c20ed79c8a6cf81311541 Mon Sep 17 00:00:00 2001 From: Tatu Lund Date: Tue, 1 Apr 2025 13:03:22 +0300 Subject: [PATCH 1/5] fix: Ensure row is visible before getting it --- .../com/vaadin/flow/component/grid/testbench/GridElement.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java b/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java index 438ac6c1297..b96a9827dba 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java +++ b/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java @@ -243,6 +243,9 @@ public List getRows(int firstRowIndex, int lastRowIndex) * if no row with given index exists */ public GridTRElement getRow(int rowIndex) throws IndexOutOfBoundsException { + if (!isRowInView(rowIndex)) { + scrollToFlatRow(rowIndex); + } var rows = getRows(rowIndex, rowIndex); return rows.size() == 1 ? rows.get(0) : null; } From fa0baf49ac2e6db8047626b29639887da6394073 Mon Sep 17 00:00:00 2001 From: Tatu Lund Date: Tue, 1 Apr 2025 17:01:26 +0300 Subject: [PATCH 2/5] Fix test The previous version needed to use scrollTo as workaround, and now expected needs to be calculated before adding row as it is high enough to cause scroll to called internally. --- .../component/grid/it/ComponentColumnWithHeightIT.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/ComponentColumnWithHeightIT.java b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/ComponentColumnWithHeightIT.java index ccae958aa5c..73b5873c440 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/ComponentColumnWithHeightIT.java +++ b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/ComponentColumnWithHeightIT.java @@ -55,14 +55,13 @@ public void shouldPositionItemsCorrectlyAfterUpdatingComponentRenderers() { @Test public void shouldPositionItemsCorrectlyAfterScrollingToEnd() { int initialLastRow = grid.getRowCount() - 1; - grid.scrollToRow(initialLastRow); + var row = grid.getRow(initialLastRow); + int expected = row.getLocation().y + row.getSize().height; add.click(); // Expect the y position of the last row to equal the y position + the // height of the previous row - Assert.assertEquals( - grid.getRow(initialLastRow).getLocation().y - + grid.getRow(initialLastRow).getSize().height, + Assert.assertEquals(expected, grid.getRow(initialLastRow + 1).getLocation().y); } } From 85f1d8f05bd558ff9b5c7d2cfd969a1e8d11ba7d Mon Sep 17 00:00:00 2001 From: Tatu Lund Date: Tue, 1 Apr 2025 18:18:28 +0300 Subject: [PATCH 3/5] Add getRow(int, boolean) to skip scrolling --- .../component/grid/testbench/GridElement.java | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java b/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java index b96a9827dba..3ab6f595a53 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java +++ b/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java @@ -243,7 +243,23 @@ public List getRows(int firstRowIndex, int lastRowIndex) * if no row with given index exists */ public GridTRElement getRow(int rowIndex) throws IndexOutOfBoundsException { - if (!isRowInView(rowIndex)) { + return getRow(rowIndex, true); + } + + /** + * Gets the tr element for the given row index. + * + * @param rowIndex + * the row index + * @param scroll + * scroll to index to reveal the row + * @return the tr element for the row + * @throws IndexOutOfBoundsException + * if no row with given index exists + */ + public GridTRElement getRow(int rowIndex, boolean scroll) + throws IndexOutOfBoundsException { + if (scroll && !isRowInView(rowIndex)) { scrollToFlatRow(rowIndex); } var rows = getRows(rowIndex, rowIndex); From 7785a0351e414ff0815667464d73528fdc5c2c99 Mon Sep 17 00:00:00 2001 From: Tatu Lund Date: Tue, 1 Apr 2025 18:19:17 +0300 Subject: [PATCH 4/5] Do not allow scrolling in the test --- .../flow/component/grid/it/ComponentColumnWithHeightIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/ComponentColumnWithHeightIT.java b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/ComponentColumnWithHeightIT.java index 73b5873c440..d41ae81db8d 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/ComponentColumnWithHeightIT.java +++ b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/ComponentColumnWithHeightIT.java @@ -62,6 +62,6 @@ public void shouldPositionItemsCorrectlyAfterScrollingToEnd() { // Expect the y position of the last row to equal the y position + the // height of the previous row Assert.assertEquals(expected, - grid.getRow(initialLastRow + 1).getLocation().y); + grid.getRow(initialLastRow + 1, false).getLocation().y); } } From 2d8749e96b57d4e08f777c11f276ae0c84d2398f Mon Sep 17 00:00:00 2001 From: ugur-vaadin Date: Wed, 7 May 2025 18:21:49 +0300 Subject: [PATCH 5/5] refactor: do not scroll by default on getrows and update javadocs --- .../grid/it/ComponentColumnWithHeightIT.java | 10 +++++----- .../component/grid/testbench/GridElement.java | 18 ++++++++++++------ 2 files changed, 17 insertions(+), 11 deletions(-) diff --git a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/ComponentColumnWithHeightIT.java b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/ComponentColumnWithHeightIT.java index d41ae81db8d..e5440fb6e9e 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/ComponentColumnWithHeightIT.java +++ b/vaadin-grid-flow-parent/vaadin-grid-flow-integration-tests/src/test/java/com/vaadin/flow/component/grid/it/ComponentColumnWithHeightIT.java @@ -54,14 +54,14 @@ public void shouldPositionItemsCorrectlyAfterUpdatingComponentRenderers() { @Test public void shouldPositionItemsCorrectlyAfterScrollingToEnd() { - int initialLastRow = grid.getRowCount() - 1; - var row = grid.getRow(initialLastRow); - int expected = row.getLocation().y + row.getSize().height; + var initialLastRow = grid.getRowCount() - 1; + var row = grid.getRow(initialLastRow, true); + var expectedPosition = row.getLocation().y + row.getSize().height; add.click(); // Expect the y position of the last row to equal the y position + the // height of the previous row - Assert.assertEquals(expected, - grid.getRow(initialLastRow + 1, false).getLocation().y); + Assert.assertEquals(expectedPosition, + grid.getRow(initialLastRow + 1).getLocation().y); } } diff --git a/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java b/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java index 3ab6f595a53..6f29d8ee58e 100644 --- a/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java +++ b/vaadin-grid-flow-parent/vaadin-grid-testbench/src/main/java/com/vaadin/flow/component/grid/testbench/GridElement.java @@ -234,26 +234,32 @@ public List getRows(int firstRowIndex, int lastRowIndex) } /** - * Gets the tr element for the given row index. + * Gets the {@code tr} element for the given row index. * * @param rowIndex * the row index - * @return the tr element for the row + * @return the {@code tr} element for the row, or {@code null} if the row is + * not in viewport * @throws IndexOutOfBoundsException * if no row with given index exists */ public GridTRElement getRow(int rowIndex) throws IndexOutOfBoundsException { - return getRow(rowIndex, true); + return getRow(rowIndex, false); } /** - * Gets the tr element for the given row index. + * Gets the {@code tr} element for the given row index. + *

+ * Returns {@code null} if the row is not in viewport and the provided + * {@code scroll} parameter is {@code false}. * * @param rowIndex * the row index * @param scroll - * scroll to index to reveal the row - * @return the tr element for the row + * whether to scroll to the row index + * @return the {@code tr} element for the row, or {@code null} if the row is + * not in viewport and the provided {@code scroll} parameter is + * {@code false} * @throws IndexOutOfBoundsException * if no row with given index exists */