-
Notifications
You must be signed in to change notification settings - Fork 124
enhancement: implement getContext method for device emulation #27
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: main
Are you sure you want to change the base?
enhancement: implement getContext method for device emulation #27
Conversation
| : LocalBrowserProvider; | ||
| private browserProviderType: T; | ||
| private actions: Array<AgentActionDefinition> = [...DEFAULT_ACTIONS]; | ||
| private clientType: "desktop" | "mobile"; |
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.
Can you take the types directly from the devices offered from the devices list ?
Something like private clientType?: keyof typeof devices;
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.
In the constructor too can you assign the clientType on the object instance ?
Like this.clientType = params.clientType
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.
Yes - I tried this, but it wouldn't give actual suggestions to user - reason being, playwright doesnt do export on Devices type. So, considering user perspective - asking user to select mobile / desktop - would make more sense since we're not able to provide suggestions.
|
|
||
| if (device === "mobile") { | ||
| const iPhone = devices["iPhone 12"]; | ||
| return await this.session.newContext({ |
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.
Pass in the entire device object, don't specify individual params here
this.session.newContext({...devices[device]})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.
I tried that - that wouldn't work because, there's this property where sharp is contradicting.
When we pass the entire object, it will throw an error like:
Error: Image to composite must have same dimensions or smaller
at Sharp.toBuffer (/home/tushar-rupani/missionctrl/HyperAgent/node_modules/sharp/lib/output.js:163:17)
at compositeScreenshot (/home/tushar-rupani/missionctrl/HyperAgent/src/agent/tools/agent.ts:38:6)
at runAgentTask (/home/tushar-rupani/missionctrl/HyperAgent/src/agent/tools/agent.ts:156:31)
at HyperAgent.executeTask (/home/tushar-rupani/missionctrl/HyperAgent/src/agent/index.ts:335:14)
at <anonymous> (/home/tushar-rupani/missionctrl/HyperAgent/src/agent/test.ts:13:18)
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.
The reason behind that is deviceScaleFactor
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.
And isMobile too.
| return this.session; | ||
| } | ||
|
|
||
| public async getContext( |
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.
Can you also add this method on the abstract base class that they inherit from ?
LocalBrowserProvider Configuration
This PR explains and have added the use-case on how to use the
clientTypesetting when configuring aHyperAgentinstance with theLocalBrowserProvider.Usage
Use the
clientTypeparameter to control whether the browser context should emulate a desktop or a mobile device.Notes
LocalProviderandHyperBrowserProvide.