Skip to content

Commit 1c8017e

Browse files
committed
set url to gradient definition as an 'attribute', not in 'style'
- so that we don't have to fiddle with quotes inside of quotes in Snapshot.toSVG - clear corresponding 'style' setting to avoid potential conflicts - adapt toSVG tests
1 parent b32aac0 commit 1c8017e

File tree

3 files changed

+13
-24
lines changed

3 files changed

+13
-24
lines changed

Diff for: src/components/drawing/index.js

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

337-
sel.style(prop, 'url(#' + fullID + ')')
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)
338342
.style(prop + '-opacity', null);
339343
};
340344

Diff for: src/snapshot/tosvg.js

-16
Original file line numberDiff line numberDiff line change
@@ -116,22 +116,6 @@ 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-
135119
if(format === 'pdf' || format === 'eps') {
136120
// these formats make the extra line MathJax adds around symbols look super thick in some cases
137121
// it looks better if this is removed entirely.

Diff for: test/jasmine/tests/snapshot_test.js

+8-7
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, 6)).toBe('url(\"#', msg);
257+
expect((actual || '').substr(0, 5)).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(this.style.fill);
280+
checkURL(d3.select(this).attr('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].style.fill);
300+
checkURL(pointElements[i].getAttribute('fill'));
301301
}
302302

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

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

325326
var lineItems = svgDOM.getElementsByClassName('legendlines');
326-
checkURL(lineItems[1].firstChild.style.stroke, 'stroke gradient');
327+
checkURL(lineItems[1].firstChild.getAttribute('stroke'), 'stroke gradient');
327328
})
328329
.catch(failTest)
329330
.then(done);
@@ -340,7 +341,7 @@ describe('Plotly.Snapshot', function() {
340341
var fillItems = svgDOM.getElementsByClassName('cbfill');
341342
expect(fillItems.length).toBe(1, '# of colorbars');
342343
for(var i = 0; i < fillItems.length; i++) {
343-
checkURL(fillItems[i].style.fill, 'fill gradient ' + i);
344+
checkURL(fillItems[i].getAttribute('fill'), 'fill gradient ' + i);
344345
}
345346
})
346347
.catch(failTest)

0 commit comments

Comments
 (0)