-
Notifications
You must be signed in to change notification settings - Fork 88
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(browser): add support for the new headless mode in BrowserConfig #208
feat(browser): add support for the new headless mode in BrowserConfig #208
Conversation
src/browser.rs
Outdated
#[derive(Debug, Clone)] | ||
enum HeadlessMode { | ||
False, | ||
True, | ||
New, | ||
} |
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 we document these?
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.
Hi @mattsse sure and it's now added. Sorry for not having included any description from the start.
f95b254
to
dda6fff
Compare
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.
last nits
/// The "headful" mode. | ||
False, | ||
/// The old headless mode (default). | ||
True, |
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.
this should be #[default]
src/browser.rs
Outdated
pub fn with_head(mut self) -> Self { | ||
self.headless = false; | ||
self.headless = HeadlessMode::False; | ||
self | ||
} | ||
|
||
pub fn new_headless_mode(mut self) -> Self { | ||
self.headless = HeadlessMode::New; | ||
self | ||
} |
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.
we want a new function that provides the HeadlessMode
as argument
src/browser.rs
Outdated
@@ -559,11 +559,21 @@ async fn ws_url_from_output( | |||
} | |||
} | |||
|
|||
#[derive(Debug, Clone)] |
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.
#[derive(Debug, Clone)] | |
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] |
Hi @mattsse , thanks a lot for the recommendations! I have the request changes applied accordingly. |
resolves #183 (kind of)