fix: use frozen panel size to calculate the visible rows/cols on scroll #7719
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Use the top and left frozen panel size to calculate the amount of headers and rows to be rendered when the user scrolls. Currently, the size of the frozen panels are not accounted which causes some of the columns and rows to not be rendered even when there's available space for that. Fixes #7507
|
One more test that would be helpful is one where a frozen row and/or column is also hidden, to make sure all the math still works. Some of our spreadsheets hide the first two rows when displaying in the browser as they hold headers already present in the ui, but we unhide them on download so they show in excel. Many workbooks freeze the top 5 or so rows, including those hidden ones. |
That was a good call. Thanks to your comment, I was able to find another issue with hidden columns/rows in the freeze pane area. The calculation for the left/right shift was not working as expected. Added a fix for that. Thanks! |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug where frozen panel sizes were not properly accounted for when calculating the number of visible rows and columns during scrolling. This led to some columns and rows not being rendered even when there was available space for them.
- Corrects the calculation logic for determining visible columns and rows by including frozen panel dimensions
- Refactors scroll handling methods to improve clarity and maintainability
- Adds comprehensive test coverage for freeze pane functionality with various scrolling scenarios
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| SheetWidget.java | Core fix to include frozen panel sizes in scroll calculations and refactored scroll handling methods |
| FreezePaneIT.java | Added extensive test coverage for freeze pane scrolling behavior and column resizing |
| CustomEditorIT.java | Added tests for custom editor behavior in frozen cells |
| TestFixtures.java | Added new test fixtures for custom editors and column hiding |
| HideSecondColumnFixture.java | New test fixture to hide the second column for testing purposes |
| CustomEditorRowFixture.java | New test fixture for testing custom editors in frozen rows |
Comments suppressed due to low confidence (1)
vaadin-spreadsheet-flow-parent/vaadin-spreadsheet-flow-integration-tests/src/test/java/com/vaadin/flow/component/spreadsheet/test/FreezePaneIT.java:221
- The error message states 'Should have the last column header visible' but the test is checking for the last row header (row200). The message should be 'Should have the last row header visible'.
Assert.assertNotNull("Should have the last column header visible",
.../main/java/com/vaadin/flow/component/spreadsheet/tests/fixtures/HideSecondColumnFixture.java
Outdated
Show resolved
Hide resolved
...tegration-tests/src/test/java/com/vaadin/flow/component/spreadsheet/test/CustomEditorIT.java
Outdated
Show resolved
Hide resolved
ugur-vaadin
reviewed
Jul 25, 2025
...c/main/java/com/vaadin/flow/component/spreadsheet/tests/fixtures/CustomEditorRowFixture.java
Show resolved
Hide resolved
ugur-vaadin
approved these changes
Jul 25, 2025
|
vaadin-bot
pushed a commit
that referenced
this pull request
Jul 28, 2025
…ll (#7719) * fix: use frozen panel size to calculate the visible rows/cols on scroll Use the top and left frozen panel size to calculate the amount of headers and rows to be rendered when the user scrolls. Currently, the size of the frozen panels are not accounted which causes some of the columns and rows to not be rendered even when there's available space for that. Fixes #7507 * test: add tests for the frozen pane * test: add tests for frozen pane with custom editors * test: add tests for frozen pane with custom editors * test: split key commands * test: change selector in expected row headers count * test: align the test execution in EscWithArrowKeys test * fix: adjust top/left shift calculation * test: temporary disable one assertion to check CI * test: undo last commit and disable test for the first column * test: use selectcell instead of clickcell * test: scale down always visible custom editor test * address copilot review comments
vaadin-bot
pushed a commit
that referenced
this pull request
Jul 28, 2025
…ll (#7719) * fix: use frozen panel size to calculate the visible rows/cols on scroll Use the top and left frozen panel size to calculate the amount of headers and rows to be rendered when the user scrolls. Currently, the size of the frozen panels are not accounted which causes some of the columns and rows to not be rendered even when there's available space for that. Fixes #7507 * test: add tests for the frozen pane * test: add tests for frozen pane with custom editors * test: add tests for frozen pane with custom editors * test: split key commands * test: change selector in expected row headers count * test: align the test execution in EscWithArrowKeys test * fix: adjust top/left shift calculation * test: temporary disable one assertion to check CI * test: undo last commit and disable test for the first column * test: use selectcell instead of clickcell * test: scale down always visible custom editor test * address copilot review comments
DiegoCardoso
added a commit
that referenced
this pull request
Jul 28, 2025
…ll (#7719) (#7756) * fix: use frozen panel size to calculate the visible rows/cols on scroll Use the top and left frozen panel size to calculate the amount of headers and rows to be rendered when the user scrolls. Currently, the size of the frozen panels are not accounted which causes some of the columns and rows to not be rendered even when there's available space for that. Fixes #7507 * test: add tests for the frozen pane * test: add tests for frozen pane with custom editors * test: add tests for frozen pane with custom editors * test: split key commands * test: change selector in expected row headers count * test: align the test execution in EscWithArrowKeys test * fix: adjust top/left shift calculation * test: temporary disable one assertion to check CI * test: undo last commit and disable test for the first column * test: use selectcell instead of clickcell * test: scale down always visible custom editor test * address copilot review comments Co-authored-by: Diego Cardoso <diego@vaadin.com>
DiegoCardoso
added a commit
that referenced
this pull request
Jul 28, 2025
…ll (#7719) (#7757) * fix: use frozen panel size to calculate the visible rows/cols on scroll Use the top and left frozen panel size to calculate the amount of headers and rows to be rendered when the user scrolls. Currently, the size of the frozen panels are not accounted which causes some of the columns and rows to not be rendered even when there's available space for that. Fixes #7507 * test: add tests for the frozen pane * test: add tests for frozen pane with custom editors * test: add tests for frozen pane with custom editors * test: split key commands * test: change selector in expected row headers count * test: align the test execution in EscWithArrowKeys test * fix: adjust top/left shift calculation * test: temporary disable one assertion to check CI * test: undo last commit and disable test for the first column * test: use selectcell instead of clickcell * test: scale down always visible custom editor test * address copilot review comments Co-authored-by: Diego Cardoso <diego@vaadin.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Use the top and left frozen panel size to calculate the number of headers and rows to be rendered when the user scrolls. Currently, the size of the frozen panels are not accounted which causes some of the columns and rows to not be rendered even when there's available space for that.
Fixes #7507
Type of change