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 modebar <style> update in IE11 #3187

Merged
merged 2 commits into from
Oct 30, 2018
Merged

Fix modebar <style> update in IE11 #3187

merged 2 commits into from
Oct 30, 2018

Conversation

etpinard
Copy link
Contributor

This PR is a fixup of #3068 for IE11, which hopefully will be enough to close #3186. It would be nice to add a test before merging this in.

cc @plotly/plotly_js

- that override DOM methods to catch browser-dependent
  (i.e. IE) bugs
'use strict';

(function(arr) {
arr.forEach(function(item) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Taken from @alexcjohnson 's #3186 (comment), this gave

image

before the previous commit.

I chose to throw instead of logging to avoid situations like the one solved in #3168.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: we could also add unpolyfill.js to our image tests (like we do for strict-d3).

I chose not to as throwing errors in our jasmine test should be enough to catch these bugs and debugging the old nw.js image server is a pain.

@antoinerg
Copy link
Contributor

antoinerg commented Oct 30, 2018

Beautiful! Thanks @etpinard for fixing my mistake so quickly!

💃 when the tests pass!

I love the new test. This mistake won't be happening again 😄 !

@etpinard
Copy link
Contributor Author

Merging.

@etpinard etpinard merged commit ec0ddd8 into master Oct 30, 2018
@etpinard etpinard deleted the custom-modebar-ie11-fix branch October 30, 2018 20:33
@antoinerg antoinerg self-assigned this Jan 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

plots in plotly.js tutorial docs not displaying in Internet Explorer 11
2 participants