Skip to content

fix: replace isDisplayed() with viewport check in autoScrollIntoView() (CP: 9.6)#2164

Merged
mcollovati merged 2 commits into9.6from
cherry/cherrypick-2158-to-9.6
Mar 6, 2026
Merged

fix: replace isDisplayed() with viewport check in autoScrollIntoView() (CP: 9.6)#2164
mcollovati merged 2 commits into9.6from
cherry/cherrypick-2158-to-9.6

Conversation

@mcollovati
Copy link
Contributor

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.

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

Fixes #2156

… 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
)

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.
@mcollovati mcollovati enabled auto-merge (squash) March 6, 2026 08:50
@github-actions
Copy link

github-actions bot commented Mar 6, 2026

Test Results

  156 files  +13    156 suites  +13   22m 7s ⏱️ - 7m 14s
1 142 tests +89  1 136 ✅ +100  6 💤 +3  0 ❌ ±0 
1 512 runs  +89  1 506 ✅ +100  6 💤 +3  0 ❌ ±0 

Results for commit 5a0152c. ± Comparison against base commit 8832dd3.

♻️ This comment has been updated with latest results.

@mcollovati mcollovati merged commit 69941d8 into 9.6 Mar 6, 2026
5 of 6 checks passed
@mcollovati mcollovati deleted the cherry/cherrypick-2158-to-9.6 branch March 6, 2026 09:37
vaadin-bot pushed a commit that referenced this pull request Mar 6, 2026
…) (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.
mshabarov pushed a commit that referenced this pull request Mar 9, 2026
…) (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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants