-
Notifications
You must be signed in to change notification settings - Fork 904
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: fallback to default browser if Chrome is not found #612
feat: fallback to default browser if Chrome is not found #612
Conversation
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.
Flow is not happy
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.
Does the debugger actually work in other browsers?
packages/cli/src/commands/server/middleware/getDevToolsMiddleware.js
Outdated
Show resolved
Hide resolved
Co-Authored-By: Michał Pierzchała <thymikee@gmail.com>
'https://www.google.com/chrome/', | ||
)}`, | ||
); | ||
//fallback to default browser |
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.
nit: space here before the inline command. I also think we can rid of that comment and rename the function to something like: launchDefaultBrowser
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.
Done. @thymikee
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.
Looks good to me
* feat: fallback to default browser if chrome not found * refactor: make flow happy * refactor: fix launchDebugger path Co-Authored-By: Michał Pierzchała <thymikee@gmail.com> * refactor: rename launchBrowser to launchDefaultBrowser
Summary:
It can be annoying for users (like me) who do not have Google Chrome installed to enable 'Remote Debugging' only to receive an error. This rectifies that by falling back to the default installed browser making the whole experience more fluid.
Test Plan:
Tested on a sample app by enabling 'Remote Debugging'.