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

replace prepend() calls since they aren't supported in IE11 #3491

Merged
merged 2 commits into from
Jan 29, 2019

Conversation

antoinerg
Copy link
Contributor

Fixes #3489

@antoinerg antoinerg added the bug something broken label Jan 28, 2019
@antoinerg
Copy link
Contributor Author

@etpinard The failing example provided by @wbrgss was tested to work under IE11 on Windows 10 by using the build from the current branch: https://plnkr.co/edit/SDQf3lmpKgltoTEoNYW8?p=preview

@@ -88,7 +88,9 @@ proto.update = function(graphInfo, buttons) {
}

if(fullLayout.modebar.orientation === 'v') {
this.element.prepend(logoGroup);
d3.select(this.element).insert(function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm. Does

d3.select(this.element).insert(logoGroup, ':first-child');

work also?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried that initially but it does not work:
screenshot_2019-01-28_17-07-49

Copy link
Contributor

Choose a reason for hiding this comment

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

Ha right, does

d3.select(this.element).insert(d3.select(logoGroup), ':first-child');

work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

d3.select(this.element).insert(d3.select(logoGroup), ':first-child');

Also fails with:
screenshot_2019-01-28_17-18-23

Would you prefer that I to turn the snippet from this PR into a one-liner?

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool. Thanks for checking!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Non- 🚫, but at that point the vanilla solution seems simpler:

pgNode.insertBefore(plotgroupBg.node(), pgNode.childNodes[0]);

newSvg.node().insertBefore(_glyphDefs.node().cloneNode(true),
newSvg.node().firstChild);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @alexcjohnson! I agree it is simpler. Done in commit 4b164a2

Updated demo https://plnkr.co/edit/3Rfo4svtr6PUDCiCRM3l?p=preview

@etpinard
Copy link
Contributor

Nicely done 💃 !

@etpinard
Copy link
Contributor

💃 (bis). Nice job!

@antoinerg antoinerg merged commit b1fd7d6 into master Jan 29, 2019
@antoinerg antoinerg deleted the 3489-v-modebar-ie11-pr branch January 29, 2019 02:09
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.

3 participants