fix: ensure last cell is fully scrolled into view when focused #10468
Conversation
vursen
force-pushed
the
fix-grid-horizontal-scrolling
branch
from
8d7f0a8 to
625baab
Compare
| let leftBoundary = this.$.table.getBoundingClientRect().left; | ||
| let rightBoundary = leftBoundary + this.$.table.clientWidth; |
There was a problem hiding this comment.
It would be good to also have a unit test. Also, visual tests seem to only cover LTR mode.
There was a problem hiding this comment.
Good catch, thanks! Addressed the RTL mode and added tests.
|
| } | ||
|
|
||
| dstCell.focus(); | ||
| dstCell.focus({ preventScroll: true }); |
There was a problem hiding this comment.
This prevents false positive test results, as some browsers scroll here to compensate for the issue, but this behavior isn't consistent and doesn't seem to work in RTL mode.
| launchOptions: { | ||
| channel: 'chrome', | ||
| headless: true, | ||
| ignoreDefaultArgs: ['--hide-scrollbars'], |
There was a problem hiding this comment.
This makes scrollbars visible in headless mode.
|
|
||
| if (dstCellRect.left < leftBoundary) { | ||
| this.$.table.scrollLeft += Math.round(dstCellRect.left - leftBoundary); | ||
| this.$.table.scrollLeft += dstCellRect.left - leftBoundary; |
There was a problem hiding this comment.
Rounding caused an underscroll in RTL mode: -0.5 VS 0
| flushGrid(grid); | ||
|
|
||
| // Force reflow to workaround a Safari rendering issue | ||
| if (isDesktopSafari) { |
There was a problem hiding this comment.
Note: tested and this code added back in #3605 is still needed in both Safari 18 and 26 used by Playwright.
Description
The PR fixes a bug where the grid didn't take into account the scrollbar width when scrolling a cell into view horizontally.
Finding from #10464
Type of change