-
Notifications
You must be signed in to change notification settings - Fork 72
feat: Add horizontal scrolling support to GridElement (#8046) #8047
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
ba95e31
c7936bb
1cae056
59377ce
6998eb6
64d2d66
3ccbaf3
136e74a
5db362e
b9d3572
f85ca37
6cc27a0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| /* | ||
| * Copyright 2000-2025 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.flow.component.grid.it; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| import com.vaadin.flow.component.grid.ColumnRendering; | ||
| import com.vaadin.flow.component.grid.Grid; | ||
| import com.vaadin.flow.component.grid.Grid.Column; | ||
| import com.vaadin.flow.component.html.Div; | ||
| import com.vaadin.flow.router.Route; | ||
|
|
||
| /** | ||
| * Test page for Grid column scrolling functionality. | ||
| */ | ||
| @Route("vaadin-grid/grid-column-scroll") | ||
| public class GridColumnScrollPage extends Div { | ||
|
|
||
| public GridColumnScrollPage() { | ||
| Grid<TestItem> grid = new Grid<>(); | ||
|
|
||
| // Set width to ensure not all columns are visible at once | ||
| grid.setWidth("800px"); | ||
|
|
||
| // Enable lazy column rendering | ||
| grid.setColumnRendering(ColumnRendering.LAZY); | ||
|
|
||
| // Add many columns to ensure horizontal scrolling is needed | ||
| for (int i = 0; i < 20; i++) { | ||
| final int columnIndex = i; | ||
| Column<TestItem> column = grid | ||
| .addColumn(item -> item.getValue(columnIndex)) | ||
| .setHeader("Column " + i).setWidth("150px"); | ||
| column.setKey("col" + i); | ||
| } | ||
|
|
||
| // Add test data | ||
| List<TestItem> items = new ArrayList<>(); | ||
| for (int row = 0; row < 100; row++) { | ||
| items.add(new TestItem(row)); | ||
| } | ||
| grid.setItems(items); | ||
|
|
||
| add(grid); | ||
| } | ||
|
|
||
| public static class TestItem { | ||
| private final int rowIndex; | ||
|
|
||
| public TestItem(int rowIndex) { | ||
| this.rowIndex = rowIndex; | ||
| } | ||
|
|
||
| public String getValue(int columnIndex) { | ||
| return "R" + rowIndex + "C" + columnIndex; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,159 @@ | ||
| /* | ||
| * Copyright 2000-2025 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.flow.component.grid.it; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| import org.junit.Assert; | ||
| import org.junit.Before; | ||
| import org.junit.Test; | ||
|
|
||
| import com.vaadin.flow.component.grid.testbench.GridColumnElement; | ||
| import com.vaadin.flow.component.grid.testbench.GridElement; | ||
| import com.vaadin.flow.component.grid.testbench.GridTHTDElement; | ||
| import com.vaadin.flow.testutil.TestPath; | ||
| import com.vaadin.tests.AbstractComponentIT; | ||
|
|
||
| /** | ||
| * Test for scrolling to columns in Grid, especially important when | ||
| * columnRendering is set to lazy. | ||
| */ | ||
| @TestPath("vaadin-grid/grid-column-scroll") | ||
| public class GridScrollToColumnIT extends AbstractComponentIT { | ||
|
|
||
| private GridElement grid; | ||
|
|
||
| @Before | ||
| public void init() { | ||
| open(); | ||
| waitForDevServer(); | ||
| grid = $(GridElement.class).first(); | ||
| } | ||
|
|
||
| @Test | ||
| public void testScrollToColumn_bringsColumnIntoView() { | ||
| // Assuming grid has many columns and some are out of view | ||
| List<GridColumnElement> columns = grid.getAllColumns(); | ||
| Assert.assertTrue("Grid should have more than 5 columns for this test", | ||
| columns.size() > 5); | ||
|
|
||
| // Get a column that might be out of view | ||
| GridColumnElement lastColumn = columns.get(columns.size() - 1); | ||
|
|
||
| // Scroll to the last column | ||
| grid.scrollToColumn(lastColumn); | ||
|
|
||
| // Verify the column is now in view | ||
| Assert.assertTrue("Column should be in view after scrolling", | ||
| grid.isColumnInView(lastColumn)); | ||
| } | ||
|
|
||
| @Test | ||
| public void testScrollToColumnByIndex_bringsColumnIntoView() { | ||
| List<GridColumnElement> columns = grid.getVisibleColumns(); | ||
| Assert.assertTrue("Grid should have more than 5 columns for this test", | ||
| columns.size() > 5); | ||
|
|
||
| int lastIndex = columns.size() - 1; | ||
|
|
||
| // Scroll to the last column by index | ||
| grid.scrollToColumn(lastIndex); | ||
|
|
||
| // Verify we can get the cell without it being null | ||
| GridTHTDElement cell = grid.getCell(0, lastIndex); | ||
| Assert.assertNotNull( | ||
| "Cell should not be null after scrolling to column", cell); | ||
| } | ||
|
|
||
| @Test | ||
| public void testIsColumnInView_detectsVisibleColumns() { | ||
| List<GridColumnElement> columns = grid.getVisibleColumns(); | ||
| Assert.assertFalse("Grid should have columns", columns.isEmpty()); | ||
|
|
||
| // First column should typically be in view | ||
| GridColumnElement firstColumn = columns.get(0); | ||
| Assert.assertTrue("First column should be in view", | ||
| grid.isColumnInView(firstColumn)); | ||
| } | ||
|
|
||
| @Test | ||
| public void testGetCell_automaticallyScrollsColumnIntoView() { | ||
| List<GridColumnElement> columns = grid.getAllColumns(); | ||
| Assert.assertTrue("Grid should have more than 10 columns for this test", | ||
| columns.size() > 10); | ||
|
|
||
| // Try to get a cell from a column that's likely out of view | ||
| GridColumnElement farColumn = columns.get(columns.size() - 1); | ||
|
|
||
| // This should automatically scroll the column into view | ||
| GridTHTDElement cell = grid.getCell(0, farColumn); | ||
|
|
||
| Assert.assertNotNull( | ||
| "Cell should not be null after automatic scrolling", cell); | ||
| Assert.assertTrue("Column should be in view after getCell", | ||
| grid.isColumnInView(farColumn)); | ||
| } | ||
|
|
||
| @Test | ||
| public void testScrollToColumn_withLazyColumnRendering() { | ||
| // This test specifically addresses the issue mentioned in #8046 | ||
| // When columnRendering is lazy, cells out of view return null | ||
|
|
||
| List<GridColumnElement> columns = grid.getAllColumns(); | ||
| Assert.assertTrue("Grid should have more than 5 columns for this test", | ||
| columns.size() > 5); | ||
|
|
||
| // First, try to get cells from the first row | ||
| for (int i = 0; i < columns.size(); i++) { | ||
| GridColumnElement column = columns.get(i); | ||
|
|
||
| // Ensure column is scrolled into view | ||
| if (!grid.isColumnInView(column)) { | ||
| grid.scrollToColumn(column); | ||
| } | ||
|
|
||
| // Now the cell should be accessible | ||
| GridTHTDElement cell = grid.getRow(0).getCell(column); | ||
| Assert.assertNotNull("Cell at column " + i | ||
| + " should not be null after scrolling", cell); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void testScrollToColumn_multipleScrolls() { | ||
| List<GridColumnElement> columns = grid.getVisibleColumns(); | ||
| Assert.assertTrue("Grid should have more than 10 columns for this test", | ||
| columns.size() > 10); | ||
|
|
||
| // Scroll to last column | ||
| GridColumnElement lastColumn = columns.get(columns.size() - 1); | ||
| grid.scrollToColumn(lastColumn); | ||
| Assert.assertTrue("Last column should be in view", | ||
| grid.isColumnInView(lastColumn)); | ||
|
|
||
| // Scroll back to first column | ||
| GridColumnElement firstColumn = columns.get(0); | ||
| grid.scrollToColumn(firstColumn); | ||
| Assert.assertTrue("First column should be in view", | ||
| grid.isColumnInView(firstColumn)); | ||
|
|
||
| // Scroll to middle column | ||
| GridColumnElement middleColumn = columns.get(columns.size() / 2); | ||
| grid.scrollToColumn(middleColumn); | ||
| Assert.assertTrue("Middle column should be in view", | ||
| grid.isColumnInView(middleColumn)); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -60,6 +60,66 @@ void scrollToRowByFlatIndex(int rowFlatIndex) { | |||
| waitUntilLoadingFinished(); | ||||
| } | ||||
|
|
||||
| /** | ||||
| * Scrolls horizontally to bring the specified column into view. This is | ||||
| * useful when working with grids that have lazy column rendering. | ||||
| * | ||||
| * @param column | ||||
| * the column to scroll into view | ||||
| */ | ||||
| public void scrollToColumn(GridColumnElement column) { | ||||
| // Scroll to column using its sizer cell offset | ||||
| executeScript(""" | ||||
| const grid = arguments[0]; | ||||
| const columnId = arguments[1]; | ||||
| const columns = grid._getColumns(); | ||||
| const col = columns.find(c => c.__generatedTbId === columnId); | ||||
| if (col && col._sizerCell) { | ||||
| grid.$.table.scrollLeft = col._sizerCell.offsetLeft; | ||||
| grid.__updateColumnsBodyContentHidden(); | ||||
| } | ||||
| """, this, column.get__generatedId()); | ||||
|
|
||||
| // Wait for rendering to complete | ||||
| waitUntilLoadingFinished(); | ||||
|
|
||||
| // Wait for column to be in view | ||||
| waitUntil(driver -> isColumnInView(column)); | ||||
|
|
||||
| } | ||||
|
|
||||
| /** | ||||
| * Scrolls horizontally to bring the column at the specified index into | ||||
| * view. This is useful when working with grids that have lazy column | ||||
| * rendering. | ||||
| * | ||||
| * @param columnIndex | ||||
| * the index of the column to scroll into view | ||||
| */ | ||||
| public void scrollToColumn(int columnIndex) { | ||||
| GridColumnElement column = getVisibleColumns().get(columnIndex); | ||||
| scrollToColumn(column); | ||||
| } | ||||
|
|
||||
| /** | ||||
| * Checks if the specified column is currently in the visible viewport. | ||||
| * | ||||
| * @param column | ||||
| * the column to check | ||||
| * @return {@code true} if the column is visible, {@code false} otherwise | ||||
| */ | ||||
| public boolean isColumnInView(GridColumnElement column) { | ||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can keep this private for now, similar to how it is with |
||||
| Boolean result = (Boolean) executeScript( | ||||
| """ | ||||
| const grid = arguments[0]; | ||||
| const columnId = arguments[1]; | ||||
| const col = grid._getColumns().find(c => c.__generatedTbId === columnId); | ||||
| return col ? grid.__isColumnInViewport(col) : false; | ||||
| """, | ||||
| this, column.get__generatedId()); | ||||
| return Boolean.TRUE.equals(result); | ||||
| } | ||||
|
|
||||
| /** | ||||
| * Gets the page size used when fetching data. | ||||
| * | ||||
|
|
@@ -115,7 +175,7 @@ public GridTHTDElement getCell(int rowIndex, int colIndex) { | |||
| /** | ||||
| * Gets the grid cell for the given row and column. | ||||
| * <p> | ||||
| * Automatically scrolls the given row into view | ||||
| * Automatically scrolls the given row and column into view | ||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This JavaDoc update should also be applied to the JavaDoc of the other overload. |
||||
| * | ||||
| * @param rowIndex | ||||
| * the row index | ||||
|
|
@@ -128,6 +188,11 @@ public GridTHTDElement getCell(int rowIndex, GridColumnElement column) { | |||
| scrollToRowByFlatIndex(rowIndex); | ||||
| } | ||||
|
|
||||
| // Also scroll column into view if needed | ||||
| if (!isColumnInView(column)) { | ||||
| scrollToColumn(column); | ||||
| } | ||||
|
|
||||
| GridTRElement row = getRow(rowIndex); | ||||
| return row.getCell(column); | ||||
| } | ||||
|
|
@@ -145,13 +210,15 @@ public GridTHTDElement getCell(int rowIndex, GridColumnElement column) { | |||
| public GridTHTDElement getCell(String contents) | ||||
| throws NoSuchElementException { | ||||
|
|
||||
| String script = "const grid = arguments[0];" | ||||
| + "const contents = arguments[1];" | ||||
| + "const rowsInDom = Array.from(arguments[0].$.items.children);" | ||||
| + "var tds = [];" | ||||
| + "rowsInDom.forEach(function(tr) { Array.from(tr.children).forEach(function(td) { tds.push(td);})});" | ||||
| + "const matches = tds.filter(function(td) { return td._content.textContent == contents});" | ||||
| + "return matches.length ? matches[0] : null;"; | ||||
| String script = """ | ||||
| const grid = arguments[0]; | ||||
| const contents = arguments[1]; | ||||
| const rowsInDom = Array.from(arguments[0].$.items.children); | ||||
| var tds = []; | ||||
| rowsInDom.forEach(function(tr) { Array.from(tr.children).forEach(function(td) { tds.push(td);})}); | ||||
| const matches = tds.filter(function(td) { return td._content.textContent == contents}); | ||||
| return matches.length ? matches[0] : null; | ||||
| """; | ||||
| TestBenchElement td = (TestBenchElement) executeScript(script, this, | ||||
| contents); | ||||
| if (td == null) { | ||||
|
|
@@ -213,11 +280,13 @@ public List<GridTRElement> getRows(int firstRowIndex, int lastRowIndex) | |||
| + (rowCount - 1) + " but were " + firstRowIndex | ||||
| + " and " + lastRowIndex); | ||||
| } | ||||
| String script = "var grid = arguments[0];" | ||||
| + "var firstRowIndex = arguments[1];" | ||||
| + "var lastRowIndex = arguments[2];" | ||||
| + "var rowsInDom = grid._getRenderedRows();" | ||||
| + "return Array.from(rowsInDom).filter((row) => { return row.index >= firstRowIndex && row.index <= lastRowIndex;});"; | ||||
| String script = """ | ||||
| var grid = arguments[0]; | ||||
| var firstRowIndex = arguments[1]; | ||||
| var lastRowIndex = arguments[2]; | ||||
| var rowsInDom = grid._getRenderedRows(); | ||||
| return Array.from(rowsInDom).filter((row) => { return row.index >= firstRowIndex && row.index <= lastRowIndex;}); | ||||
| """; | ||||
| Object rows = executeScript(script, this, firstRowIndex, lastRowIndex); | ||||
| if (rows != null) { | ||||
| return ((ArrayList<?>) rows).stream().map( | ||||
|
|
@@ -286,18 +355,20 @@ public List<GridColumnElement> getAllColumns() { | |||
| } | ||||
|
|
||||
| protected void generatedColumnIdsIfNeeded() { | ||||
| String generateIds = "const grid = arguments[0];" | ||||
| + "if (!grid.__generatedTbId) {"// | ||||
| + " grid.__generatedTbId = 1;"// | ||||
| + "}" // | ||||
| + "grid._getColumns().forEach(function(column) {" | ||||
| + " if (!column.__generatedTbId) {" | ||||
| + " column.__generatedTbId = grid.__generatedTbId++;" // | ||||
| + " }" // | ||||
| + "});"; | ||||
| String generateIds = """ | ||||
| const grid = arguments[0]; | ||||
| if (!grid.__generatedTbId) { | ||||
| grid.__generatedTbId = 1; | ||||
| } | ||||
| grid._getColumns().forEach(function(column) { | ||||
| if (!column.__generatedTbId) { | ||||
| column.__generatedTbId = grid.__generatedTbId++; | ||||
| } | ||||
| }); | ||||
| return grid._getColumns().length; | ||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
This new line seems to serve no purpose. |
||||
| """; | ||||
|
|
||||
| executeScript(generateIds, this); | ||||
| // | ||||
| } | ||||
|
|
||||
| /** | ||||
|
|
||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tests seem to be testing whether the GridElement API is working rather than the Grid itself. I think we can remove the page and the test class.