Skip to content
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

Fix switch theme button for IE11 #89

Closed
kylejrp opened this issue May 30, 2019 · 6 comments
Closed

Fix switch theme button for IE11 #89

kylejrp opened this issue May 30, 2019 · 6 comments
Labels
bug Something isn't working

Comments

@kylejrp
Copy link
Collaborator

kylejrp commented May 30, 2019

I haven't nailed down the source of this problem, but the "Switch Theme" button on the demo page has been broken on IE11 for a while.

This change was not introduced by #85. However, #85 may have added more compatibility issues on top of it.

We could try tracing down the original source of the problem, or we could just update the build tools to automatically support IE11 and other browsers.

@kylejrp kylejrp added the bug Something isn't working label May 30, 2019
@kimulaco
Copy link
Contributor

@kylejrp Thank you for making the issue!
Since I corresponded to #85 , I would like to correspond to this issue as well.

@kognise
Copy link
Owner

kognise commented May 27, 2020

I'm on macOS right now - is there a way for me to test this? If not would someone with ie11 mind jumping on this issue?

@kylejrp
Copy link
Collaborator Author

kylejrp commented May 27, 2020

If I recall correctly it has to due with IE11 not supporting the arrow function syntax.

You can manually convert arrow functions to the older syntax to fix it.

We should be looking to transpile the JavaScript on the demo page for IE11 compatibility similar to how our CSS build targets IE11 for CSS compatibility

I can test it this evening if nobody else can jump on it before then

@kylejrp
Copy link
Collaborator Author

kylejrp commented May 27, 2020

This wouldn't be strictly required for a 2.0 launch now that I think about it - it's just a bug in the demo site. Feel free to remove it from the milestone if you agree.

@kognise kognise removed this from the 2.0 Launch milestone May 27, 2020
@kognise kognise mentioned this issue May 27, 2020
7 tasks
@jonaskuske
Copy link
Collaborator

Yeah, we need to transpile script.js and load URLSearchParams and Promise polyfills if the browser doesn't support them natively. I can take care of that, along with a general refactor/cleanup :)

@jonaskuske
Copy link
Collaborator

This is fixed in 2.0 (#194) – script.js is transpiled now and I've made sure that we only use APIs that are available in IE, e.g. child.parentElement.removeChild(child) vs. just child.remove(). Also, a polyfill for the async clipboard API and Promise is loaded if necessary.

Only (minor) issue that remains is that our Lint rules (which require us to use ES6 arrow functions) apply to inline <script> in the HTML, but the code there is not transpiled, so we need to use standard functions there for IE compat and manually disable the ESLint errors. But I think that's okay, it only affects the Analytics snippet and not much more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants