-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat(browser): add ElementHandle.isInViewport method #5243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
feat(browser): add ElementHandle.isInViewport method #5243
Conversation
Implements the isInViewport method and adds a corresponding test case. Resolves grafana#4423
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution, @Ashish050488 ❤️
We're trying to make it easy for users and match the Playwright API. This PR adds IsInViewPort to ElementHandle, but Playwright doesn't have a corresponding method. Fortunately, the Locator assertions do. Please move this method to the Locator type. Sorry for the inconvenience, as the issue was very old and not updated.
You should also fix the linter errors and tests. Thanks!
I was unable to run the full test suite on my Windows machine due to an existing issue (TestTmpDirCleanup failure). However, I successfully ran my new test (TestFrameElementIsInViewport) in isolation before discovering the test suite issue on Windows.
You can use the -skip flag for go test to ignore some of the flaky tests while running the whole test suite.
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove these extra lines.
| require.NoError(t, err) | ||
| require.NotNil(t, bottomElement) | ||
|
|
||
| // 1. First, check that the element is NOT in the viewport |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // 1. First, check that the element is NOT in the viewport | |
| // 1. First, check that the elements are NOT in the viewport |
| require.NoError(t, err) | ||
| require.False(t, inViewport, "the bottom element should not be in the viewport initially") | ||
|
|
||
| // Get the link at the top |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove this comment, as the code is clear.
| // It can take a moment for the scroll to complete, so we wait briefly. | ||
| time.Sleep(500 * time.Millisecond) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be flaky. Try using the isVisible method.
d35e805 to
4eaf78d
Compare
4eaf78d to
b52c4f3
Compare
Moves isInViewport method to Locator, delegates implementation to Frame, updates test case per review grafana#5243.
b52c4f3 to
4c8e513
Compare
| <<<<<<< HEAD | ||
| ======= | ||
| "strings" | ||
| "github.com/grafana/sobek" | ||
| >>>>>>> 8ea4cb8ad (refactor(browser): address PR feedback for isInViewport) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merge/Rebase artifact to cleanup 🙇🏻
|
@Ashish050488 We'll release k6 1.4.0 in two weeks. Please continue working on this PR if you want it merged into k6 1.4.0. Otherwise, we'll have to postpone merging your PR until k6 1.5.0. |
c3f583b to
88dcd4c
Compare
|
sorry for the delay i was stuck with something now i am active i did the changes asked for do provide feedback if changes required |
|
i am trying but i dont know why those test fail can you give me help a little like where am i going wrong i would really appreaciate if you teach me |
|
Hi @Ashish050488, you can run goimports to fix the "imported but not used" issue: |
Implements the isInViewport method and adds a corresponding test case. Resolves #4423
What?
This PR adds the
ElementHandle.isInViewport()method. This new method allows users to check if an element is currently visible within the browser's viewport by executing a script on the element.A new test,
TestFrameElementIsInViewport, has also been added toframe_test.goto validate this functionality.Why?
This feature was requested in issue #4423 to enable more robust testing for in-page navigations, specifically to confirm that an element has been scrolled into view. This adds functionality similar to what is available in other browser automation libraries.
Checklist
make check) and all pass.TestTmpDirCleanupfailure). However, I successfully ran my new test (TestFrameElementIsInViewport) in isolation before discovering the test suite issue on Windows.Checklist: Documentation (only for k6 maintainers and if relevant)
Please do not merge this PR until the following items are filled out.
Related PR(s)/Issue(s)