-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Conversation
views/jsconfig.handlebars
Outdated
@@ -1,8 +1,16 @@ | |||
{{!-- This file should be es5 only --}} | |||
var isIE = navigator.userAgent.toLowerCase().indexOf('msie') > -1 || | |||
!!navigator.userAgent.toLowerCase().match(/trident\/7\./); |
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.
Instead of a double negation on .match()
, we could possibly just use .test()
:
/trident\/7\./i.test(navigator.userAgent);
views/jsconfig.handlebars
Outdated
{{!-- This file should be es5 only --}} | ||
var isIE = navigator.userAgent.toLowerCase().indexOf('msie') > -1 || | ||
!!navigator.userAgent.toLowerCase().match(/trident\/7\./); | ||
var isUnsupportedPage = location.pathname.includes('/unsupported'); |
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.
I don't think we can use .includes()
here, per https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/includes#Browser_compatibility
May have to just use a cheap .indexOf('/unsupported') !== -1)
or similar.
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 is what happens when I start a PR at 4:30 on Friday
So the redirect works, in much the same way that #352 did, but it still has the same issue in that the page shows up like this due to l10n not firing. (As well as some CSS issues, but that should hopefully be easily fixable once we have some text) @dannycoates do you think you can get l10n working without es6? |
@dannycoates the console errors for you |
… doesn't support property or method 'from'
fixes #351