Throws error in IE 11 (Windows 10)

[gfellerph]

I noticed that the library is not working in IE. I tracked down the cause and was briefly working on a solution but I'm a little bit stuck.

There seems to be a babel transpilation issue which makes use of the _toConsumableArray which in turn uses Array.from and that is not supported in IE. All this causes the image selection process to go into the catch routine.

After some googling, I tried to apply the fix with the transpile-runtime babel plugin as described here https://github.com/babel/babel/issues/934. This did not quite work out for me, but maybe I configured it wrong, I'm not familiar with rollup. I installed the babel-plugin-transform-runtime and added it as 'transform-runtime' to the .babelrc.

Another approach woud be to polyfill Array.from with this https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/from#Polyfill but it is a lot of additional code just for that tiny bit of functionality.

Additionaly, there are some issues with the demo page at https://medium-zoom.francoischalifour.com/ which can be fixed by introducing the Array.from polyfill into the demo.js and getting rid of the template string rendering the span element.

What do you think about IE11 support?

[francoischalifour]

As discussed in #26, I don't plan to support IE11. As far as I know, you'd need to polyfill Array.from() and Array.prototype.includes(). Including these two snippets of code before the medium-zoom import should be enough for your page to work.

If you do find the correct Babel settings without introducing any new dependencies (devDependencies are fine) and without degrading the dev experience, I might consider merging it.

[gfellerph]

Thanks for clarifying, I'll see if I can polyfill these two methods with transpile-runtime and I'll check if it does not hurt bundle size.

If you like, I'll add a browser support section to the readme, to avoid future misunderstandings (=

[francoischalifour]

Adding a browser support section to the README would be great! I should have done it already.

If we aim at supporting IE11, we could replace the 2 occurrences of Array.prototype.includes() by Array.prototype.indexOf() in the code (it's not a drastic change). We should however polyfill Array.prototype.from() or, in the "Browser support" section, specify that they need to include the polyfill for medium-zoom to work.

I don't think I'm in favor of polyfilling Array.prototype.from() in the library itself because it would increase the bundle size and be useful only for a minority of users.

The spirit of the library is to provide a great UX experience as well as the best DX experience (developer experience) as possible. With that in mind, we might introduce new features that are incompatible with IE11 (e.g. container feature with templates).

[jhildenbiddle]

I've create a pull request to address this issue, bringing support for IE10/11.

https://github.com/francoischalifour/medium-zoom/pull/35

Goals:

• Prevent errors in IE, which in turn prevent other JavaScript from executing
• Do so without requiring developers to include a separate polyfill

I understand IE10/11 support probably isn't a priority, but in its current state medium-zoom breaks apps in IE rather than failing gracefully. The pull request passes all tests and allows the demo/preview example to function properly in IE10/11.

Unfortunately, I discovered issues with the dropbox-paper-template and facebook-template examples after creating the pull-request. Specifically, IE appears to have an issue with the use of cloneNode(). Both of these examples render incorrectly in IE before any image is zoomed, so it's possible these issues are example-specific (I haven't looked into it). Before claiming support for IE10/11, these additional issues should be resolved.

[francoischalifour]

Hi @jhildenbiddle, thank you so much for taking the time to work on this issue!

As I mentioned in my previous message, template examples won't be able to work on IE < 11 because of its lack of template support. This is the main reason I didn't intend to support IE versions. I want to be able to ship new features without being slowed down by old browsers.

Thank you for your efforts though, I'll consider releasing an IE version based on your solution if the demand increases. It needs more testing on my part to be shipped though.

[jhildenbiddle]

Hi @francoischalifour.

Happy to help, and I certainly understand the hesitation with accepting a PR and claiming IE support without further testing.

Releasing a separate IE version seems like a lot of (arguably unnecessary) work, and as time marches on the need for this will only decrease. My goal is not so much to add support for older browsers but to prevent medium-zoom from breaking apps unnecessarily in IE, Edge 12/13, and older evergreen browsers.

Hopefully you won't mind me offering a few alternative paths forward:

1. Use feature detection to prevent medium-zoom from executing in unsupported browsers.

   For example, test for Array.from, Array.includes, and template support to determine if medium-zoom should execute. Changes to medium-zoom are minor, official browser support doesn't change, and apps and dependent libraries continue to work as-is with the added benefit of medium-zoom now failing gracefully in unsupported browsers. If developers polyfill their app, medium-zoom might work but it isn't officially supported.

2. Accept the PR to address critical errors and provide "basic" support for IE10-11/Edge12-13, then use feature detection to prevent breaking features (such as options.template) from executing in unsupported browsers.

   The benefit of this options is that developers interested in "basic" zoom functionality can now use medium-zoom out-of-the-box in previously unsupported browsers. This includes developers integrating medium-zoom directly as well as those using libraries that list medium-zoom as a dependency like docsify's zoom-image plugin which does not use medium-zoom's options.template feature. "Basic" support for IE10/11 provided by the PR can be seen in the Netlify deploy preview.

The reason I'm pushing for medium-zoom to fail gracefully is to address a use case where medium-zoom may be included by a third party. Specifically, I'm working on a customizable theme for docsify.js. Docsify provides a plugin architecture which I am leveraging to add theme-specific functionality. Docsify also offers a zoom-image plugin that is based on medium-zoom. When a user includes the zoom-image plugin, unsupported browsers break (because of medium-zoom) and JavaScript execution stops which breaks docsify and my theme's plugins.

My hope is to incorporate changes into medium-zoom to help everyone using the library. An alternate approach is to address the issues in docsify's zoom-image plugin (use a forked version of medium-zoom, add feature detection to the plugin, etc). Happy to go either way, but it seems like option 1 above is a win-win for everyone, no?

Whew. :)

If you've made it this far, thanks for taking the time. Much appreciated!

[francoischalifour]

Thanks for these perspectives, you definitely got a point.

I switched the discussion to the PR #35.