Skip to content

Fix ScrollIntoView - correct position for the focus#10224

Merged
christian-bromann merged 5 commits intowebdriverio:mainfrom
lacell75:fixScroll
Apr 20, 2023
Merged

Fix ScrollIntoView - correct position for the focus#10224
christian-bromann merged 5 commits intowebdriverio:mainfrom
lacell75:fixScroll

Conversation

@lacell75
Copy link
Contributor

Proposed changes

In case where there are several possible scrolls in the page(example: a page with a left menu), the scroll doesn't work on the other parts of the page because the focus by default is positioned at x:0 , y:0.
To fix it, I just added the origin element in argument.

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)

Further comments

Reviewers: @webdriverio/project-committers

@christian-bromann
Copy link
Member

@lacell75 thanks for raising the PR

The solution seems reasonable even though I am not sure if it has implications on other use cases. However the test seems to legitimately fail: e2e/wdio/headless/test.e2e.ts:91:36

@lacell75
Copy link
Contributor Author

lacell75 commented Apr 18, 2023 via email

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Apr 19, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

@lacell75
Copy link
Contributor Author

@christian-bromann PR ready to review

@christian-bromann
Copy link
Member

PR ready to review

@lacell75 did you see my comment? I wonder if we can optimize the e2e test a bit

@lacell75
Copy link
Contributor Author

PR ready to review

@lacell75 did you see my comment? I wonder if we can optimize the e2e test a bit

Sorry, I had not seen your comment. Improvement done

@lacell75
Copy link
Contributor Author

lacell75 commented Apr 20, 2023

I think everything is ok now

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.

Great work, amazing change 👍

@christian-bromann christian-bromann added the PR: Polish 💅 PRs that contain improvements on existing features label Apr 20, 2023
@christian-bromann christian-bromann merged commit 8c2d65c into webdriverio:main Apr 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: Polish 💅 PRs that contain improvements on existing features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants