-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
feat(screencast): add private recording APIs and basic test #3296
Conversation
@@ -58,6 +58,8 @@ export interface PageDelegate { | |||
canScreenshotOutsideViewport(): boolean; | |||
resetViewport(): Promise<void>; // Only called if canScreenshotOutsideViewport() returns false. | |||
setBackgroundColor(color?: { r: number; g: number; b: number; a: number; }): Promise<void>; | |||
startVideoRecording(options: types.VideoRecordingOptions): Promise<void>; |
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.
Let's move this to the browser context to cover popups, wdyt?
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.
That's the ultimate plan. This requires generating file names, events that video started etc. For now I'd like to have basic functionality to provide some test coverage. Context level API can be added on top of it. Do you think we won't need page level start/stop functionality?
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.
As long as it is on delegate only - lgtm!
I'd love to have a |
f77880f
to
d40c3d5
Compare
I actually had an argument against namespaces - they made rpc / other langs a bit harder to implement. So I would love to learn why you prefer it to make sure we make a balanced call |
135b74d
to
3bb284d
Compare
845456e
to
5b77b33
Compare
#1158