Skip to content

Fix NPD on click API call #4845

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

Merged
merged 3 commits into from
Jun 16, 2025
Merged

Fix NPD on click API call #4845

merged 3 commits into from
Jun 16, 2025

Conversation

ankur22
Copy link
Contributor

@ankur22 ankur22 commented Jun 11, 2025

What?

We're swapping out the use of working with cdppage.GetLayoutMetrics to working with a manual workaround that retrieves the current frame's width and height from the utility context.

Why?

This should avoid NPDs that we sometimes see. More context can be found here in Playwright's PR for a similar issue: microsoft/playwright#2669.

Checklist

  • I have performed a self-review of my code.
  • I have commented on my code, particularly in hard-to-understand areas.
  • I have added tests for my changes.
  • I have run linter and tests locally (make check) and all pass.

Checklist: Documentation (only for k6 maintainers and if relevant)

Please do not merge this PR until the following items are filled out.

  • I have added the correct milestone and labels to the PR.
  • I have updated the release notes: b783fd2
  • I have updated or added an issue to the k6-documentation: grafana/k6-docs#NUMBER if applicable
  • I have updated or added an issue to the TypeScript definitions: grafana/k6-DefinitelyTyped#NUMBER if applicable

Related PR(s)/Issue(s)

Issue: #4123
Comment: #4123 (comment)

@ankur22 ankur22 requested a review from a team as a code owner June 11, 2025 12:15
@ankur22 ankur22 requested review from inancgumus and joanlopez and removed request for a team June 11, 2025 12:15
@ankur22 ankur22 force-pushed the fix/npd-on-click branch from 345b865 to 5b7d491 Compare June 11, 2025 12:19
Copy link
Contributor

@inancgumus inancgumus left a comment

Choose a reason for hiding this comment

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

LGTM. But with one suggestion.

inancgumus
inancgumus previously approved these changes Jun 11, 2025
joanlopez
joanlopez previously approved these changes Jun 13, 2025
@joanlopez joanlopez added this to the v1.1.0 milestone Jun 13, 2025
@ankur22 ankur22 dismissed stale reviews from joanlopez and inancgumus via 93af43c June 16, 2025 12:46
ankur22 added 3 commits June 16, 2025 13:48
This works with the utility context which might help avoid the NPDs we
see when working with the current approach -- cdppage.GetLayoutMetrics.
@ankur22 ankur22 force-pushed the fix/npd-on-click branch from 93af43c to 38ae96e Compare June 16, 2025 12:48
@ankur22 ankur22 requested review from inancgumus and joanlopez June 16, 2025 12:48
@ankur22 ankur22 merged commit 5460663 into master Jun 16, 2025
38 of 39 checks passed
@ankur22 ankur22 deleted the fix/npd-on-click branch June 16, 2025 15:20
ankur22 added a commit that referenced this pull request Jun 17, 2025
joanlopez added a commit that referenced this pull request Jun 25, 2025
* Base release notes for v1.1.0

* Add release notes for dependabot PRs

* Add #4809 and #4831 into the v1.1.0 release note

* Add #4845 to the v1.1.0 release notes

* Add example for locator.count

* Remove await on expect in count example

* Add section for nth, first and count in v1.1.0...

... release notes.

* Update examples to work with latest test library

This also updates the nth, first and last example.

* Add some PRs from the beginning of the cycle into release notes

* Add external PRs, among others, to release notes

* Clean up stuff from release notes template

* Add remaining PRs into release notes

* Add #4862 to the v1.1.0 release note

* Apply suggestions from code review

Co-authored-by: Mihail Stoykov <312246+mstoykov@users.noreply.github.com>

* Add #4864 to v1.1.0 release notes

* Remove #4862 from v1.1.0

* Apply suggestions from code review

Co-authored-by: İnanç Gümüş <inanc.gumus@grafana.com>

* web/crypto updates

* Update release notes/v1.1.0.md

Co-authored-by: Mihail Stoykov <312246+mstoykov@users.noreply.github.com>

* docs: document v1.1.0 forward roadmap

* Refine experimental modules notes

---------

Co-authored-by: ankur22 <ankur.agarwal@grafana.com>
Co-authored-by: Mihail Stoykov <312246+mstoykov@users.noreply.github.com>
Co-authored-by: İnanç Gümüş <inanc.gumus@grafana.com>
Co-authored-by: oleiade <theo@crevon.me>
Co-authored-by: Ivan <2103732+codebien@users.noreply.github.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.

4 participants