Skip to content

Replace platform with bowser for browser detection #1991

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

Merged
merged 2 commits into from
May 24, 2018

Conversation

rschamp
Copy link
Contributor

@rschamp rschamp commented May 6, 2018

The bowser package correctly detects Chrome Headless as Chrome, is already used by scratchr2, and is more actively maintained than platform.

@rschamp rschamp added this to the May 2018 milestone May 6, 2018
@rschamp rschamp requested a review from chrisgarrity May 6, 2018 20:00
Copy link
Contributor

@chrisgarrity chrisgarrity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks fine, but I tried it with my windows box at home and IE gets an object doesn't support this action error. The line it points at in lib.min.js is (function(module, exports, __webpack_require__) {

We'll need to look at this more tomorrow.

platform.name === 'Opera' ||
platform.name === 'Opera Mini' ||
platform.name === 'Silk') {
if (bowser.name === 'IE' ||

This comment was marked as abuse.

This comment was marked as abuse.

Ray Schamp added 2 commits May 15, 2018 09:54
bowser correctly detects Chrome Headless as Chrome, is already used by scratchr2, and is more actively maintained than platform.
Also, fix detection of IE so it works. Apparently bowser.name !== 'IE' on IE with bowser. bowser.msie works though.
@rschamp rschamp force-pushed the platform-bowser branch from 5b79d86 to c8182dd Compare May 15, 2018 14:22
@rschamp rschamp requested a review from chrisgarrity May 15, 2018 14:23
Copy link
Contributor

@chrisgarrity chrisgarrity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks cleaner, and it's working on my windows box at home (chrome, edge and IE)!

@rschamp rschamp merged commit b593f11 into scratchfoundation:develop May 24, 2018
@rschamp rschamp deleted the platform-bowser branch May 24, 2018 20:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants