fix: replace `isDisplayed()` with viewport check in `autoScrollIntoView()` by mcollovati · Pull Request #2158 · vaadin/testbench

mcollovati · 2026-02-28T14:59:22Z

doubleClick(), click(x,y) and contextClick() threw MoveTargetOutOfBoundsException when the target element was inside a scrollable container (e.g. vaadin-grid) but scrolled out of the visible area. The root cause was that autoScrollIntoView() used wrappedElement.isDisplayed() which returns true for elements that are in the DOM but positioned outside the viewport.

Replace the check with isElementInViewport(), a JavaScript-based visibility test that verifies the element's bounding rect intersects the viewport and is not clipped by any scrollable ancestor. When the element is outside the viewport, call scrollIntoView({nearest}) to bring it into view before Selenium's Actions.moveToElement reads the element coordinates.

Also add a scrollIntoView(Map) public overload accepting scroll options.

Fixes #2156

…ew()` `doubleClick()`, `click(x,y)` and `contextClick()` threw `MoveTargetOutOfBoundsException` when the target element was inside a scrollable container (e.g. vaadin-grid) but scrolled out of the visible area. The root cause was that `autoScrollIntoView()` used `wrappedElement.isDisplayed()` which returns true for elements that are in the DOM but positioned outside the viewport. Replace the check with `isElementInViewport()`, a JavaScript-based visibility test that verifies the element's bounding rect intersects the viewport and is not clipped by any scrollable ancestor. When the element is outside the viewport, call `scrollIntoView({nearest})` to bring it into view before Selenium's `Actions.moveToElement` reads the element coordinates. Also add a `scrollIntoView(Map)` public overload accepting scroll options. Fixes #2156

mcollovati · 2026-03-02T13:49:46Z

Running flow-components tests locally, I got the following errors

[ERROR] Errors:
[ERROR]   ButtonIT>AbstractComponentIT.setup:30 » UnsupportedOperation Viewport couldn't be adjusted.
[ERROR]   TooltipDefaultsIT>AbstractComponentIT.setup:30 » UnsupportedOperation Viewport couldn't be adjusted.
[ERROR]   TooltipMarkdownIT>AbstractComponentIT.setup:30 » UnsupportedOperation Viewport couldn't be adjusted.

Moved the PR to draft for further investigation

mcollovati · 2026-03-03T10:40:19Z

Previously mentioned tests were just flaky. However, other tests are failing like DatePickerI18nIT.setI18n_i18nIsApplied or GridItemRefreshPageIT.updateAndRefreshItemsOnTheServerUsingDataCommunicator_withComponentRenderer.

Basically, the test cycle over elements that are currently rendered but not visible on screen without scrolling, and the use getText() to collect some data. The problem is that with the previous implementation the isDisplayed check returned true and the text was gathered without scrolling; however, with the viewport check the scroll happens, and this causes issues, for example, with infinite scroll since the data changes while the test is still cycling through the items.

mcollovati · 2026-03-03T10:48:44Z

So the current situation is that there are several methods calling autoScrollIntoView, but not all of them actually need the element to be in the viewport. For example, I think getText() and clear() just needs the element to be rendered, but doubleClick, contextMenu, click(x,y) (and perhaps sendKeys`) the element must be in the viewport.

I wonder if we should make autoScrollIntoView do different checks based on what the caller expects it to achieve. Something like autoScrollIntoView(boolean ensureInViewport): if ensureInViewport == false just check isDisplayed otherwise applu the new view port check.

Artur- · 2026-03-03T12:03:11Z

Why should autoScrollIntoView be invoked if you don't need the element to be in view? What would autoScrollIntoView(false) do - not scroll into view?

mcollovati · 2026-03-03T12:16:41Z

They both will do scrollIntoView, but based on different conditions. One only needs isDisplayed to be true, the other needs the element to be effectively visible and intractable like in the browser

…lement` Rename `autoScrollIntoView()` to `ensureVisible()` and `autoScrollIntoViewport()` to `ensureInteractable()` to better communicate intent rather than mechanism.

mcollovati · 2026-03-04T07:57:46Z

Tested the latest changes with flow-components and validation passed.
I'm going to test also with Flow

mcollovati · 2026-03-04T10:15:39Z

Flow validation also passed locally

… or viewport based on the use case (#2158) * fix: replace `isDisplayed()` with viewport check in `autoScrollIntoView()` `doubleClick()`, `click(x,y)` and `contextClick()` threw `MoveTargetOutOfBoundsException` when the target element was inside a scrollable container (e.g. vaadin-grid) but scrolled out of the visible area. The root cause was that `autoScrollIntoView()` used `wrappedElement.isDisplayed()` which returns true for elements that are in the DOM but positioned outside the viewport. Replace the check with `isElementInViewport()`, a JavaScript-based visibility test that verifies the element's bounding rect intersects the viewport and is not clipped by any scrollable ancestor. When the element is outside the viewport, call `scrollIntoView({nearest})` to bring it into view before Selenium's `Actions.moveToElement` reads the element coordinates. Also add a `scrollIntoView(Map)` public overload accepting scroll options. Fixes #2156 * refactor: rename scroll helpers to goal-oriented names in `TestBenchElement` Rename `autoScrollIntoView()` to `ensureVisible()` and `autoScrollIntoViewport()` to `ensureInteractable()` to better communicate intent rather than mechanism. * use Selenium API to scroll into viewport

…) (CP: 9.6) (#2164) * fix: refactor `autoScrollIntoView()` feature to check `isDisplayed()` or viewport based on the use case (#2158) * fix: replace `isDisplayed()` with viewport check in `autoScrollIntoView()` `doubleClick()`, `click(x,y)` and `contextClick()` threw `MoveTargetOutOfBoundsException` when the target element was inside a scrollable container (e.g. vaadin-grid) but scrolled out of the visible area. The root cause was that `autoScrollIntoView()` used `wrappedElement.isDisplayed()` which returns true for elements that are in the DOM but positioned outside the viewport. Replace the check with `isElementInViewport()`, a JavaScript-based visibility test that verifies the element's bounding rect intersects the viewport and is not clipped by any scrollable ancestor. When the element is outside the viewport, call `scrollIntoView({nearest})` to bring it into view before Selenium's `Actions.moveToElement` reads the element coordinates. Also add a `scrollIntoView(Map)` public overload accepting scroll options. Fixes #2156 * refactor: rename scroll helpers to goal-oriented names in `TestBenchElement` Rename `autoScrollIntoView()` to `ensureVisible()` and `autoScrollIntoViewport()` to `ensureInteractable()` to better communicate intent rather than mechanism. * use Selenium API to scroll into viewport * fix: do not use getCoordinates().inViewPort() to scroll into view (#2161) Calling getCoordinates().inViewPort() seems to always scroll even if the element is alredy visibile and interactable. This migth cause issue in existing tests. This change partially reverts the previous fix to just use scrollIntoView after viewport check.

…) (CP: 9.6) (#2164) (#2166) * fix: refactor `autoScrollIntoView()` feature to check `isDisplayed()` or viewport based on the use case (#2158) * fix: replace `isDisplayed()` with viewport check in `autoScrollIntoView()` `doubleClick()`, `click(x,y)` and `contextClick()` threw `MoveTargetOutOfBoundsException` when the target element was inside a scrollable container (e.g. vaadin-grid) but scrolled out of the visible area. The root cause was that `autoScrollIntoView()` used `wrappedElement.isDisplayed()` which returns true for elements that are in the DOM but positioned outside the viewport. Replace the check with `isElementInViewport()`, a JavaScript-based visibility test that verifies the element's bounding rect intersects the viewport and is not clipped by any scrollable ancestor. When the element is outside the viewport, call `scrollIntoView({nearest})` to bring it into view before Selenium's `Actions.moveToElement` reads the element coordinates. Also add a `scrollIntoView(Map)` public overload accepting scroll options. Fixes #2156 * refactor: rename scroll helpers to goal-oriented names in `TestBenchElement` Rename `autoScrollIntoView()` to `ensureVisible()` and `autoScrollIntoViewport()` to `ensureInteractable()` to better communicate intent rather than mechanism. * use Selenium API to scroll into viewport * fix: do not use getCoordinates().inViewPort() to scroll into view (#2161) Calling getCoordinates().inViewPort() seems to always scroll even if the element is alredy visibile and interactable. This migth cause issue in existing tests. This change partially reverts the previous fix to just use scrollIntoView after viewport check. Co-authored-by: Marco Collovati <marco@vaadin.com>

caalador approved these changes Mar 2, 2026

View reviewed changes

mcollovati marked this pull request as draft March 2, 2026 13:48

mcollovati mentioned this pull request Mar 3, 2026

test: add a test for clicks on non visible grid columns vaadin/flow-components#8587

Merged

mcollovati added 2 commits March 3, 2026 15:09

refactor: rename scroll helpers to goal-oriented names in `TestBenchE…

7cb210d

…lement` Rename `autoScrollIntoView()` to `ensureVisible()` and `autoScrollIntoViewport()` to `ensureInteractable()` to better communicate intent rather than mechanism.

use Selenium API to scroll into viewport

580c3a1

mcollovati marked this pull request as ready for review March 4, 2026 10:15

mcollovati requested a review from caalador March 4, 2026 10:15

caalador approved these changes Mar 4, 2026

View reviewed changes

mcollovati merged commit fbaf6b9 into main Mar 5, 2026
10 checks passed

mcollovati deleted the issues/2156-doubleclick_viewport branch March 5, 2026 09:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: replace `isDisplayed()` with viewport check in `autoScrollIntoView()`#2158

fix: replace `isDisplayed()` with viewport check in `autoScrollIntoView()`#2158
mcollovati merged 3 commits intomainfrom
issues/2156-doubleclick_viewport

mcollovati commented Feb 28, 2026

Uh oh!

mcollovati commented Mar 2, 2026

Uh oh!

mcollovati commented Mar 3, 2026

Uh oh!

mcollovati commented Mar 3, 2026

Uh oh!

Artur- commented Mar 3, 2026

Uh oh!

mcollovati commented Mar 3, 2026

Uh oh!

mcollovati commented Mar 4, 2026

Uh oh!

mcollovati commented Mar 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

mcollovati commented Feb 28, 2026

Uh oh!

mcollovati commented Mar 2, 2026

Uh oh!

mcollovati commented Mar 3, 2026

Uh oh!

mcollovati commented Mar 3, 2026

Uh oh!

Artur- commented Mar 3, 2026

Uh oh!

mcollovati commented Mar 3, 2026

Uh oh!

mcollovati commented Mar 4, 2026

Uh oh!

mcollovati commented Mar 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants