Switching to a SVG presentation attribute to assist Microsoft browsers #1723
Conversation
…s that do not properly support CSS styles
| @@ -444,11 +444,11 @@ module.exports = function(svg, styledData, layout, callbacks) { | |||
| .append('g') | |||
| .classed('sankeyLinks', true) | |||
| .style('fill', 'none') | |||
| .style('transform', linksTransform); | |||
There was a problem hiding this comment.
Good catch.
Would anyone be interested in making @alexcjohnson 's strict-d3 wrapper throw an error on selection.style('transform', <>)?
There was a problem hiding this comment.
Good idea! I'm working on it now in a separate PR.
There was a problem hiding this comment.
We need to check if the styled element is an SVG element or not. (I'm planning to just check the first element in the selection with sel.node() as selecting both HTML and SVG elements in the same selection is not commonly done.) A linting problem arises:
/Users/robert/repos/plotly/plotly.js/test/image/strict-d3.js
42:38 error 'SVGElement' is not defined no-undef
Interestingly the linter doesn't balk on a similar usage.
Have we got an established workaround, or should I just lint-define the SVGElement locally?
There was a problem hiding this comment.
I don't think we have an established workaround, either lint defining it or var SVGElement = window.SVGElement would be fine by me.
There was a problem hiding this comment.
var SVGElement = window.SVGElement
Yep, that's the way to go here 👍
|
The tweening effect when switching between horizontal and vertical layout was a bit like a circus attraction, and interestingly, the switch to attr made it noticeably worse, crossing into creepy territory. I'm reworking a chunk of code so that links and nodes don't "fly" on separate arcs but they remain joint instead. It also stops the node rectangle shape morphing too. As I think this PR stands on its legs though (fixing the error) it'll be a separate PR based on this one. |
Interesting indeed... I certainly wouldn't have expected that to make a difference. Personally I don't see animating an orientation change as being particularly useful to viewers (did we have an explicit requirement to do this?), so I'd be at least as happy just snapping to the new layout - particularly if it's going to take a noticeable effort to fix this. |
|
This looks to be all set yes? Particularly since #1729 is merged so it's tested... |
There is still insufficient support for CSS styles on SVG elements in IE: plotly/plotly.R#1023 (comment)