Skip to content

fix: do not use getCoordinates().inViewPort() to scroll into view#2161

Merged
caalador merged 1 commit intomainfrom
chore/do-not-use-inViewPort
Mar 6, 2026
Merged

fix: do not use getCoordinates().inViewPort() to scroll into view#2161
caalador merged 1 commit intomainfrom
chore/do-not-use-inViewPort

Conversation

@mcollovati
Copy link
Contributor

Calling getCoordinates().inViewPort() seems to always scroll even if the element is already visible and interactable. This might cause issue in existing tests. This change partially reverts the previous fix to just use scrollIntoView after viewport check.

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 requested a review from caalador March 5, 2026 15:03
@caalador caalador merged commit 083d0c9 into main Mar 6, 2026
10 checks passed
@caalador caalador deleted the chore/do-not-use-inViewPort branch March 6, 2026 05:31
mcollovati added a commit that referenced this pull request Mar 6, 2026
)

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 added 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.
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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants