Skip to content

Commit

Permalink
Fix issue where gradient definitions could conflict with existing def…
Browse files Browse the repository at this point in the history
…initions
  • Loading branch information
jakezatecky committed Jul 21, 2016
1 parent c3dd161 commit ddb7171
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 15 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@
* [#19]: Add support for percentages in `chart.width` and `chart.height` (e.g. `'75%'`)
* [#38]: Split line break characters found in `label.format` into multiple lines

### Bug Fixes

* [#49]: Fix issue where gradient definitions could conflict with existing definitions

## v0.7.7 (July 15, 2016)

### New Features
Expand Down
30 changes: 25 additions & 5 deletions src/d3-funnel/Colorizer.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,20 @@ class Colorizer {
*/
constructor() {
this.hexExpression = /^#([0-9a-f]{3}|[0-9a-f]{6})$/i;

this.instanceId = null;
this.labelFill = null;

this.scale = null;
}

/**
* @param {string} instanceId
*
* @return {void}
*/
setInstanceId(instanceId) {
this.instanceId = instanceId;
}

/**
* @param {string} fill
*
Expand All @@ -34,15 +42,16 @@ class Colorizer {
* @param {Array} block
* @param {Number} index
* @param {string} type
* @param {string} instanceId
*
* @return {Object}
*/
getBlockFill(block, index, type) {
getBlockFill(block, index, type, instanceId) {
const raw = this.getBlockRawFill(block, index);

return {
raw,
actual: this.getBlockActualFill(raw, index, type),
actual: this.getBlockActualFill(raw, index, type, instanceId),
};
}

Expand Down Expand Up @@ -83,7 +92,18 @@ class Colorizer {
return raw;
}

return `url(#gradient-${index})`;
return `url(#${this.getGradientId(index)})`;
}

/**
* Return the gradient ID for the given index.
*
* @param {Number} index
*
* @return {string}
*/
getGradientId(index) {
return `${this.instanceId}-gradient-${index}`;
}

/**
Expand Down
15 changes: 7 additions & 8 deletions src/d3-funnel/D3Funnel.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ class D3Funnel {
this.labelFormatter = new LabelFormatter();
this.navigator = new Navigator();

this.id = null;
this.autoId = 0;

// Bind event handlers
Expand Down Expand Up @@ -115,6 +116,9 @@ class D3Funnel {

const settings = this.getSettings(options);

this.id = this.generateUniqueId();
this.colorizer.setInstanceId(this.id);

// Set labels
this.label = settings.label;
this.labelFormatter.setFormat(this.label.format);
Expand Down Expand Up @@ -371,12 +375,9 @@ class D3Funnel {
* @return {void}
*/
drawOntoDom() {
const id = this.generateUniqueId();

// Add the SVG
this.svg = d3.select(this.selector)
this.svg = d3.select(this.selector)// Add the SVG
.append('svg')
.attr('id', id)
.attr('id', this.id)
.attr('width', this.width)
.attr('height', this.height);

Expand Down Expand Up @@ -635,9 +636,7 @@ class D3Funnel {

// Create linear gradient
const gradient = defs.append('linearGradient')
.attr({
id: `gradient-${index}`,
});
.attr({ id: this.colorizer.getGradientId(index) });

// Define the gradient stops
const stops = [
Expand Down
4 changes: 2 additions & 2 deletions test/d3-funnel/D3Funnel.js
Original file line number Diff line number Diff line change
Expand Up @@ -694,9 +694,9 @@ describe('D3Funnel', () => {
// due to a Webkit bug in the current PhantomJS; workaround is
// to select the known ID of the linearGradient element
// https://bugs.webkit.org/show_bug.cgi?id=83438
assert.equal(1, d3.selectAll('#funnel defs #gradient-0')[0].length);
assert.equal(1, d3.selectAll('#funnel defs #d3-funnel-chart-0-gradient-0')[0].length);

assert.equal('url(#gradient-0)', d3.select('#funnel path').attr('fill'));
assert.equal('url(#d3-funnel-chart-0-gradient-0)', d3.select('#funnel path').attr('fill'));
});

it('should use solid fill when not set to \'gradient\'', () => {
Expand Down

0 comments on commit ddb7171

Please sign in to comment.