Skip to content

(webdriverio): fix scrollIntoView#11120

Merged
christian-bromann merged 5 commits intowebdriverio:mainfrom
erwinheitzman:fix-scrollintoview-command
Sep 9, 2023
Merged

(webdriverio): fix scrollIntoView#11120
christian-bromann merged 5 commits intowebdriverio:mainfrom
erwinheitzman:fix-scrollintoview-command

Conversation

@erwinheitzman
Copy link
Member

fix scrollIntoView by aligning it with it's native counterpart for both horizontal and vertical scrolling as well as scrolling multiple times. Changed scroll and scrollIntoView duration to 0

Proposed changes

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update

Checklist

  • I have read the CONTRIBUTING doc
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • I have added proper type definitions for new commands (if appropriate)

Reviewers: @webdriverio/project-committers

fix scrollIntoView by aligning it with it's native counterpart for both horizontal and vertical scrolling as well as scrolling multiple times. Changed scroll and scrollIntoView duration to 0
@christian-bromann
Copy link
Member

This is amazing 🎉 let's adjust the coverage treshold if it keeps failing for Windows.

Copy link
Member

@christian-bromann christian-bromann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🙌

@christian-bromann christian-bromann merged commit 2fa0411 into webdriverio:main Sep 9, 2023
@erwinheitzman erwinheitzman deleted the fix-scrollintoview-command branch September 22, 2023 13:22
try {
return await browser.action('wheel')
.scroll({ duration: 200, x: windowX, y: windowY, deltaX, deltaY, origin })
.scroll({ duration: 0, x: deltaX, deltaY, origin: this })

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, @christian-bromann, @erwinheitzman
Could you please tell me what's the reason for using x: deltaX here?
Why can't we just use deltaX, deltaY instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I made when changing this, it should be y: deltaY

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see. So it will be changed in future releases?
I just didn't want to create a bug for this minor thing. Scroll is working for me, but with warning and fallback to web api

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I can correct this, thing is it won't show up in the release notes because we use the issues to generate the release notes.

Regarding the warning, it falls back to the native web scrollIntoView when the webdriver approach throws an error so it would be interesting to find out why it's failing at that attempt and with what error.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it. Thanks

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.

3 participants