-
Notifications
You must be signed in to change notification settings - Fork 325
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
Observe perform candidates #426
Conversation
🦋 Changeset detectedLatest commit: 9776827 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…M on a11y context
… observe_perform_candidates
…dates # Conflicts: # lib/StagehandPage.ts # lib/handlers/observeHandler.ts
… observe_perform_candidates
need to check if _waitForSettledDom is completely necessary in public performPlaywrightMethod |
just missing adding changeset |
await newOpenedTab.close(); | ||
await stagehandPage.goto(newOpenedTab.url()); | ||
await stagehandPage.waitForLoadState("domcontentloaded"); | ||
// await stagehandPage._waitForSettledDom(domSettleTimeoutMs); |
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.
note: figure out how to get around this -- maybe we make waitForSettledDom
a function that takes in a page?
@@ -76,65 +78,22 @@ export class StagehandObserveHandler { | |||
}, | |||
}); | |||
|
|||
let outputString: string; |
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.
checking my understanding: we're removing this because we're not cross-referencing DOM with a11y tree, and now we're basically just removing a11y tree if the user insists on only observing visible elements
why
Observe got a major upgrade. Now it will return a suggested playwright method with any necessary arguments for the generated candidate elements. It also includes a significant speedup when using a11y tree processing for context.
what changed
The output of observe calls used to be in the format:
Now, observe returns the following:
On top of that, the a11y tree context parsing / candidate generation got a major speedup by skipping processAllDom.
With a11y trees, the context might include elements present in the DOM but behind some effect in the UI. To give users control on whether to include these in the context or not, we provide the flag
onlyVisible
. In essence, setting it to true will default to the previous observe method, and false (default) will use the new features described in this PR.Sample usage:
test plan