-
Notifications
You must be signed in to change notification settings - Fork 576
chrome-launcher + spelling fixes + docs #120
chrome-launcher + spelling fixes + docs #120
Conversation
@@ -21,11 +21,11 @@ import { | |||
export default class LocalRuntime { |
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.
These are mostly spelling fixes
@@ -39,7 +55,7 @@ export default class LocalChrome implements Chrome { | |||
fitWindow: false, // as we cannot resize the window, `fitWindow: false` is needed in order for the viewport to be resizable | |||
} | |||
|
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 was a bug waiting for us 😄
@@ -53,7 +53,7 @@ export async function wait(timeout: number): Promise<void> { | |||
export async function nodeExists(client: Client, selector: string): Promise<boolean> { |
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.
Small change here for brevity (also it didn't work?)
Not sure how we're booting chrome in aws... might need to alter that script now that chromeless will try and boot chrome automatically |
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.
@joelgriffith Nice!
Could you add launchChrome: false
in the run handler so that Chromeless doesn't try to spawn Chrome? Headless Chrome is already running by the time we call the LocalChrome constructor.
Oh! And don't forget to update the CHANGELOG—this enhancement probably counts as a notable change.
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.
@joelgriffith looks good to me (other than the merge conflicts again 👀 )
@timsuchanek can you take a look too, so that we have 2 sign-offs?
I get |
Makes it so consumers can just have chrome launch automatically without starting it themselves (though they can still launch manually if they prefer).