Skip to content

Commit 0557bb1

Browse files
committed
Revert "set url to gradient definition as an 'attribute', not in 'style'"
- see #2914 (comment) for more info. This reverts commit 1c8017e.
1 parent 1c8017e commit 0557bb1

File tree

3 files changed

+24
-13
lines changed

3 files changed

+24
-13
lines changed

src/components/drawing/index.js

+1-5
Original file line numberDiff line numberDiff line change
@@ -334,11 +334,7 @@ drawing.gradient = function(sel, gd, gradientID, type, colorscale, prop) {
334334
});
335335
});
336336

337-
// Set url to gradient definition as an 'attribute', so that
338-
// we don't have to fiddle with quotes inside of quotes in Snapshot.toSVG.
339-
// Clear corresponding 'style' setting to avoid potential conflicts.
340-
sel.attr(prop, 'url(#' + fullID + ')')
341-
.style(prop, null)
337+
sel.style(prop, 'url(#' + fullID + ')')
342338
.style(prop + '-opacity', null);
343339
};
344340

src/snapshot/tosvg.js

+16
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,22 @@ module.exports = function toSVG(gd, format, scale) {
116116
}
117117
});
118118

119+
svg.selectAll('.point, .scatterpts, .legendfill>path, .legendlines>path, .cbfill').each(function() {
120+
var pt = d3.select(this);
121+
122+
// similar to font family styles above,
123+
// we must remove " after the SVG DOM has been serialized
124+
var fill = this.style.fill;
125+
if(fill && fill.indexOf('url(') !== -1) {
126+
pt.style('fill', fill.replace(DOUBLEQUOTE_REGEX, DUMMY_SUB));
127+
}
128+
129+
var stroke = this.style.stroke;
130+
if(stroke && stroke.indexOf('url(') !== -1) {
131+
pt.style('stroke', stroke.replace(DOUBLEQUOTE_REGEX, DUMMY_SUB));
132+
}
133+
});
134+
119135
if(format === 'pdf' || format === 'eps') {
120136
// these formats make the extra line MathJax adds around symbols look super thick in some cases
121137
// it looks better if this is removed entirely.

test/jasmine/tests/snapshot_test.js

+7-8
Original file line numberDiff line numberDiff line change
@@ -254,7 +254,7 @@ describe('Plotly.Snapshot', function() {
254254
describe('should handle quoted style properties', function() {
255255
function checkURL(actual, msg) {
256256
// which is enough tot check that toSVG did its job right
257-
expect((actual || '').substr(0, 5)).toBe('url(#', msg);
257+
expect((actual || '').substr(0, 6)).toBe('url(\"#', msg);
258258
}
259259

260260
it('- marker-gradient case', function(done) {
@@ -277,7 +277,7 @@ describe('Plotly.Snapshot', function() {
277277
});
278278

279279
d3.selectAll('.point,.scatterpts').each(function() {
280-
checkURL(d3.select(this).attr('fill'));
280+
checkURL(this.style.fill);
281281
});
282282

283283
return Plotly.Snapshot.toSVG(gd);
@@ -297,12 +297,12 @@ describe('Plotly.Snapshot', function() {
297297
expect(pointElements.length).toEqual(3);
298298

299299
for(i = 0; i < pointElements.length; i++) {
300-
checkURL(pointElements[i].getAttribute('fill'));
300+
checkURL(pointElements[i].style.fill);
301301
}
302302

303303
var legendPointElements = svgDOM.getElementsByClassName('scatterpts');
304304
expect(legendPointElements.length).toEqual(1);
305-
checkURL(legendPointElements[0].getAttribute('fill'));
305+
checkURL(legendPointElements[0].style.fill);
306306
})
307307
.catch(failTest)
308308
.then(done);
@@ -319,12 +319,11 @@ describe('Plotly.Snapshot', function() {
319319

320320
var fillItems = svgDOM.getElementsByClassName('legendfill');
321321
for(var i = 0; i < fillItemIndices.length; i++) {
322-
var path = fillItems[fillItemIndices[i]].firstChild;
323-
checkURL(path.getAttribute('fill'), 'fill gradient ' + i);
322+
checkURL(fillItems[fillItemIndices[i]].firstChild.style.fill, 'fill gradient ' + i);
324323
}
325324

326325
var lineItems = svgDOM.getElementsByClassName('legendlines');
327-
checkURL(lineItems[1].firstChild.getAttribute('stroke'), 'stroke gradient');
326+
checkURL(lineItems[1].firstChild.style.stroke, 'stroke gradient');
328327
})
329328
.catch(failTest)
330329
.then(done);
@@ -341,7 +340,7 @@ describe('Plotly.Snapshot', function() {
341340
var fillItems = svgDOM.getElementsByClassName('cbfill');
342341
expect(fillItems.length).toBe(1, '# of colorbars');
343342
for(var i = 0; i < fillItems.length; i++) {
344-
checkURL(fillItems[i].getAttribute('fill'), 'fill gradient ' + i);
343+
checkURL(fillItems[i].style.fill, 'fill gradient ' + i);
345344
}
346345
})
347346
.catch(failTest)

0 commit comments

Comments
 (0)