Skip to content

feat(annotate): instrument input actions and show subtitle overlay#39797

Merged
pavelfeldman merged 2 commits intomicrosoft:mainfrom
pavelfeldman:annotate-input-actions
Mar 22, 2026
Merged

feat(annotate): instrument input actions and show subtitle overlay#39797
pavelfeldman merged 2 commits intomicrosoft:mainfrom
pavelfeldman:annotate-input-actions

Conversation

@pavelfeldman
Copy link
Member

Summary

  • Add onBeforeInputAction instrumentation for raw keyboard/mouse/touchscreen via api* methods
  • Move annotation rendering into screencast listener with subtitle overlay
  • Populate metadata.box from element bounding rect
  • Change keyboard/mouse/touchscreen protocol methods from pausesBeforeAction to pausesBeforeInput
  • Simplify trace viewer pointer rendering

@pavelfeldman pavelfeldman force-pushed the annotate-input-actions branch from 8152196 to 589cb02 Compare March 21, 2026 00:33
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

for (const entry of this._renderedEntries) {
entry.box = entry.targetElement.getBoundingClientRect();
if (!entry.box && !entry.targetElement)
continue;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we still want to update the tooltip in this case.

Copy link
Member Author

Choose a reason for hiding this comment

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

Tooltip of what?

const title = renderTitleForCall(metadata);
const utility = await page.mainFrame()._utilityContext();

// Run this outside of the progress timer.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why? The progress timer is ticking anyway, so it still takes away from the action timeout.

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 never want this animation to interrupt, probably need to do something about progress there in general.


async apiUp(progress: Progress, options: { button?: types.MouseButton, clickCount?: number } = {}) {
progress.metadata.point = this.currentPoint();
await this._page.instrumentation.onBeforeInputAction(this._page, progress.metadata);
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like we now capture the same tracing snapshot twice - on onBeforeCall and onBeforeInputAction immediately after it.

const utility = await page.mainFrame()._utilityContext();

// Run this outside of the progress timer.
await utility.evaluate(async options => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's catch this one, or even better - the whole method?

const target = targetElements[0];
const targetBox = target?.getBoundingClientRect();
const targetCenter = target ? { x: targetBox.left + targetBox.width / 2, y: targetBox.top + targetBox.height / 2 } : null;
const isAligned = !targetCenter || Math.abs(targetCenter.x - pointX) > 10 || Math.abs(targetCenter.y - pointY) > 10;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const isAligned = !targetCenter || Math.abs(targetCenter.x - pointX) > 10 || Math.abs(targetCenter.y - pointY) > 10;
const isAligned = !targetCenter || (Math.abs(targetCenter.x - pointX) < 10 && Math.abs(targetCenter.y - pointY) < 10);

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@pavelfeldman pavelfeldman force-pushed the annotate-input-actions branch from 589cb02 to c9c52b2 Compare March 21, 2026 20:30
@github-actions
Copy link
Contributor

Test results for "tests 1"

3 flaky ⚠️ [chromium-library] › library/trace-viewer.spec.ts:1223 › should display language-specific locators `@chromium-ubuntu-22.04-node22`
⚠️ [firefox-library] › library/browsertype-connect.spec.ts:758 › launchServer › should upload a folder `@firefox-ubuntu-22.04-node20`
⚠️ [webkit-library] › library/browsercontext-add-cookies.spec.ts:426 › should allow unnamed cookies `@webkit-ubuntu-22.04-node20`

38818 passed, 845 skipped


Merge workflow run.

@github-actions
Copy link
Contributor

Test results for "MCP"

5458 passed, 340 skipped


Merge workflow run.

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