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

d3@v6 with dc@v4 #1749

Closed
wants to merge 17 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
378 changes: 170 additions & 208 deletions package-lock.json

Large diffs are not rendered by default.

19 changes: 1 addition & 18 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,29 +25,12 @@
"url": "https://github.com/dc-js/dc.js.git"
},
"dependencies": {
"d3": "^5.15.1"
"d3": "^6.0.0"
},
"devDependencies": {
"compare-versions": "^3.6.0",
"crossfilter2": "^1.5.2",
"d3-array": "^2.4.0",
"d3-axis": "^1.0.12",
"d3-brush": "^1.1.5",
"d3-collection": "^1.0.7",
"d3-dispatch": "^1.0.6",
"d3-ease": "^1.0.6",
"d3-format": "^1.4.4",
"d3-geo": "^1.11.9",
"d3-hierarchy": "^1.1.9",
"d3-interpolate": "^1.4.0",
"d3-scale": "^3.2.1",
"d3-scale-chromatic": "^1.5.0",
"d3-selection": "^1.4.1",
"d3-shape": "^1.3.7",
"d3-time": "^1.1.0",
"d3-time-format": "^2.2.3",
"d3-timer": "^1.0.10",
"d3-zoom": "^1.8.3",
"eslint": "^6.8.0",
"eslint-plugin-import": "^2.20.2",
"eslint-plugin-node": "^10.0.0",
Expand Down
12 changes: 6 additions & 6 deletions spec/bar-chart-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ describe('dc.BarChart', () => {
});

it('should generate the y-axis domain dynamically', () => {
expect(nthYAxisText(0).text()).toMatch(/-10/);
expect(nthYAxisText(1).text()).toMatch(/-5/);
expect(nthYAxisText(0).text()).toMatch(/10/);
expect(nthYAxisText(1).text()).toMatch(/5/);
expect(nthYAxisText(2).text()).toBe('0');
});

Expand Down Expand Up @@ -583,7 +583,7 @@ describe('dc.BarChart', () => {
return d3.select(chart.selectAll('g.axis.y .tick text').nodes()[n]);
};

expect(nthText(0).text()).toBe('-20');
expect(nthText(0).text()).toBe('20');
expect(nthText(1).text()).toBe('0');
expect(nthText(2).text()).toBe('20');
});
Expand All @@ -609,9 +609,9 @@ describe('dc.BarChart', () => {
return d3.select(chart.selectAll('g.axis.y .tick text').nodes()[n]);
};

expect(nthText(0).text()).toBe('-30');
expect(nthText(1).text()).toBe('-20');
expect(nthText(2).text()).toBe('-10');
expect(nthText(0).text()).toBe('30');
expect(nthText(1).text()).toBe('20');
expect(nthText(2).text()).toBe('10');
expect(nthText(3).text()).toBe('0');
});
});
Expand Down
6 changes: 3 additions & 3 deletions spec/heatmap-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -225,10 +225,10 @@ describe('dc.heatmap', () => {

const reduceDimensionValues = function (dmsn) {
return dmsn.top(Infinity).reduce((p, d) => {
p.cols.add(d.colData);
p.rows.add(d.rowData);
p.cols.add(+d.colData);
p.rows.add(+d.rowData);
return p;
}, {cols: d3.set(), rows: d3.set()});
}, {cols: new Set(), rows: new Set()});
};

beforeEach(() => {
Expand Down
24 changes: 6 additions & 18 deletions spec/helpers/spec-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,16 +50,10 @@ const simulateChartBrushing = function (chart, domainSelection) {
// D3v4 needs scaled coordinates for the event
const scaledSelection = domainSelection.map(coord => chart.x()(coord));

d3.event = {
sourceEvent: true,
// an event with fields that dc cares about
chart._brushing({
selection: scaledSelection
};

try {
chart._brushing();
} finally {
d3.event = null;
}
});
};

// Simulate a dummy event - just enough for the handler to get fooled
Expand All @@ -70,14 +64,8 @@ const simulateChart2DBrushing = function (chart, domainSelection) {
return scale(coord);
}));

d3.event = {
sourceEvent: true,
// an event with fields that dc cares about
chart._brushing({
selection: scaledSelection
};

try {
chart._brushing();
} finally {
d3.event = null;
}
});
};
8 changes: 4 additions & 4 deletions spec/line-chart-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -562,7 +562,7 @@ describe('dc.lineChart', () => {
return d3.select(chart.selectAll('g.axis.y .tick text').nodes()[n]);
};

expect(nthText(0).text()).toBe('-20');
expect(nthText(0).text()).toBe('20');
expect(nthText(1).text()).toBe('0');
expect(nthText(2).text()).toBe('20');
});
Expand Down Expand Up @@ -598,9 +598,9 @@ describe('dc.lineChart', () => {
return d3.select(chart.selectAll('g.axis.y .tick text').nodes()[n]);
};

expect(nthText(0).text()).toBe('-30');
expect(nthText(1).text()).toBe('-20');
expect(nthText(2).text()).toBe('-10');
expect(nthText(0).text()).toBe('30');
expect(nthText(1).text()).toBe('20');
expect(nthText(2).text()).toBe('10');
expect(nthText(3).text()).toBe('0');
});
});
Expand Down
4 changes: 2 additions & 2 deletions spec/row-chart-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -460,7 +460,7 @@ describe('dc.rowChart', () => {
}

itShouldBehaveLikeARowChartWithGroup(positiveGroupHolder, 5, ['0', '5', '10']);
itShouldBehaveLikeARowChartWithGroup(negativeGroupHolder, 5, ['-10', '-5', '0']);
itShouldBehaveLikeARowChartWithGroup(mixedGroupHolder, 5, ['-5', '0', '5']);
itShouldBehaveLikeARowChartWithGroup(negativeGroupHolder, 5, ['10', '5', '0']);
itShouldBehaveLikeARowChartWithGroup(mixedGroupHolder, 5, ['5', '0', '5']);
itShouldBehaveLikeARowChartWithGroup(largerGroupHolder, 7);
});
3 changes: 2 additions & 1 deletion src/base/bubble-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { scaleLinear } from 'd3-scale';
import {ColorMixin} from './color-mixin';
import {transition} from '../core/core';
import {events} from '../core/events';
import {adaptHandler} from '../core/d3compat';

/**
* This Mixin provides reusable functionalities for any chart that needs to visualize data using bubbles.
Expand Down Expand Up @@ -149,7 +150,7 @@ export const BubbleMixin = Base => class extends ColorMixin(Base) {
label = bubbleGEnter.append('text')
.attr('text-anchor', 'middle')
.attr('dy', '.3em')
.on('click', d => this.onClick(d));
.on('click', adaptHandler(d => this.onClick(d)));
}

label
Expand Down
81 changes: 47 additions & 34 deletions src/base/coordinate-grid-mixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ import {scaleBand, scaleLinear, scaleOrdinal} from 'd3-scale';
import {axisBottom, axisLeft, axisRight} from 'd3-axis';
import {zoom, zoomIdentity} from 'd3-zoom';
import {brushX} from 'd3-brush';
import {event} from 'd3-selection';

import {ColorMixin} from './color-mixin';
import {MarginMixin} from './margin-mixin';
import {optionalTransition, transition} from '../core/core';
import {units} from '../core/units';
import {constants} from '../core/constants';
import {utils} from '../core/utils';
import {adaptHandler} from '../core/d3compat';
import {logger} from '../core/logger';
import {filters} from '../core/filters';
import {events} from '../core/events';
Expand Down Expand Up @@ -67,6 +67,7 @@ export class CoordinateGridMixin extends ColorMixin(MarginMixin) {
this._brushOn = true;
this._parentBrushOn = false;
this._round = undefined;
this._ignoreBrushEvents = false; // ignore when carrying out programmatic brush operations

this._renderHorizontalGridLine = false;
this._renderVerticalGridLine = false;
Expand All @@ -77,9 +78,10 @@ export class CoordinateGridMixin extends ColorMixin(MarginMixin) {
this._zoomScale = [1, Infinity];
this._zoomOutRestrict = true;

this._zoom = zoom().on('zoom', () => this._onZoom());
this._zoom = zoom().on('zoom', adaptHandler((d, evt) => this._onZoom(evt)));
this._nullZoom = zoom().on('zoom', null);
this._hasBeenMouseZoomable = false;
this._ignoreZoomEvents = false; // ignore when carrying out programmatic zoom operations

this._rangeChart = undefined;
this._focusChart = undefined;
Expand Down Expand Up @@ -932,7 +934,7 @@ export class CoordinateGridMixin extends ColorMixin(MarginMixin) {

renderBrush (g, doTransition) {
if (this._brushOn) {
this._brush.on('start brush end', () => this._brushing());
this._brush.on('start brush end', adaptHandler((d, evt) => this._brushing(evt)));

// To retrieve selection we need self._gBrush
this._gBrush = g.append('g')
Expand Down Expand Up @@ -972,22 +974,12 @@ export class CoordinateGridMixin extends ColorMixin(MarginMixin) {
return !brushSelection || brushSelection[1] <= brushSelection[0];
}

_brushing () {
// Avoids infinite recursion (mutual recursion between range and focus operations)
// Source Event will be null when brush.move is called programmatically (see below as well).
if (!event.sourceEvent) {
_brushing (evt) {
if (this._ignoreBrushEvents) {
return;
}

// Ignore event if recursive event - i.e. not directly generated by user action (like mouse/touch etc.)
// In this case we are more worried about this handler causing brush move programmatically which will
// cause this handler to be invoked again with a new d3.event (and current event set as sourceEvent)
// This check avoids recursive calls
if (event.sourceEvent.type && ['start', 'brush', 'end'].indexOf(event.sourceEvent.type) !== -1) {
return;
}

let brushSelection = event.selection;
let brushSelection = evt.selection;
if (brushSelection) {
brushSelection = brushSelection.map(this.x().invert);
}
Expand All @@ -1009,9 +1001,22 @@ export class CoordinateGridMixin extends ColorMixin(MarginMixin) {
this.redrawGroup();
}

_withoutBrushEvents (closure) {
const oldValue = this._ignoreBrushEvents;
this._ignoreBrushEvents = true;

try {
closure();
} finally {
this._ignoreBrushEvents = oldValue;
}
}

setBrushExtents (doTransition) {
// Set boundaries of the brush, must set it before applying to self._gBrush
this._brush.extent([[0, 0], [this.effectiveWidth(), this.effectiveHeight()]]);
this._withoutBrushEvents(() => {
// Set boundaries of the brush, must set it before applying to self._gBrush
this._brush.extent([[0, 0], [this.effectiveWidth(), this.effectiveHeight()]]);
});

this._gBrush
.call(this._brush);
Expand All @@ -1024,8 +1029,10 @@ export class CoordinateGridMixin extends ColorMixin(MarginMixin) {
}

if (!brushSelection) {
this._gBrush
.call(this._brush.move, null);
this._withoutBrushEvents(() => {
this._gBrush
.call(this._brush.move, null);
})

this._gBrush.selectAll(`path.${CUSTOM_BRUSH_HANDLE_CLASS}`)
.attr('display', 'none');
Expand All @@ -1035,8 +1042,10 @@ export class CoordinateGridMixin extends ColorMixin(MarginMixin) {
const gBrush =
optionalTransition(doTransition, this.transitionDuration(), this.transitionDelay())(this._gBrush);

gBrush
.call(this._brush.move, scaledSelection);
this._withoutBrushEvents(() => {
gBrush
.call(this._brush.move, scaledSelection);
});

gBrush.selectAll(`path.${CUSTOM_BRUSH_HANDLE_CLASS}`)
.attr('display', null)
Expand Down Expand Up @@ -1235,26 +1244,30 @@ export class CoordinateGridMixin extends ColorMixin(MarginMixin) {
// If we changing zoom status (for example by calling focus), tell D3 zoom about it
_updateD3zoomTransform () {
if (this._zoom) {
this._zoom.transform(this.root(), this._domainToZoomTransform(this.x().domain(), this._xOriginalDomain, this._origX));
this._withoutZoomEvents(() => {
this._zoom.transform(this.root(), this._domainToZoomTransform(this.x().domain(), this._xOriginalDomain, this._origX));
});
}
}

_onZoom () {
// Avoids infinite recursion (mutual recursion between range and focus operations)
// Source Event will be null when zoom is called programmatically (see below as well).
if (!event.sourceEvent) {
return;
_withoutZoomEvents (closure) {
const oldValue = this._ignoreZoomEvents;
this._ignoreZoomEvents = true;

try {
closure();
} finally {
this._ignoreZoomEvents = oldValue;
}
}

// Ignore event if recursive event - i.e. not directly generated by user action (like mouse/touch etc.)
// In this case we are more worried about this handler causing zoom programmatically which will
// cause this handler to be invoked again with a new d3.event (and current event set as sourceEvent)
// This check avoids recursive calls
if (event.sourceEvent.type && ['start', 'zoom', 'end'].indexOf(event.sourceEvent.type) !== -1) {
_onZoom (evt) {
// ignore zoom events if it was caused by a programmatic change
if (this._ignoreZoomEvents) {
return;
}

const newDomain = event.transform.rescaleX(this._origX).domain();
const newDomain = evt.transform.rescaleX(this._origX).domain();
this.focus(newDomain, false);
}

Expand Down
5 changes: 3 additions & 2 deletions src/charts/bar-chart.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {transition} from '../core/core';
import {constants} from '../core/constants';
import {logger} from '../core/logger';
import {pluck, utils} from '../core/utils';
import {adaptHandler} from '../core/d3compat';

const MIN_BAR_WIDTH = 1;
const DEFAULT_GAP_BETWEEN_BARS = 2;
Expand Down Expand Up @@ -144,7 +145,7 @@ export class BarChart extends StackMixin {
.merge(labels);

if (this.isOrdinal()) {
labelsEnterUpdate.on('click', d => this.onClick(d));
labelsEnterUpdate.on('click', adaptHandler(d => this.onClick(d)));
labelsEnterUpdate.attr('cursor', 'pointer');
}

Expand Down Expand Up @@ -188,7 +189,7 @@ export class BarChart extends StackMixin {
}

if (this.isOrdinal()) {
barsEnterUpdate.on('click', d => this.onClick(d));
barsEnterUpdate.on('click', adaptHandler(d => this.onClick(d)));
}

transition(barsEnterUpdate, this.transitionDuration(), this.transitionDelay())
Expand Down
5 changes: 3 additions & 2 deletions src/charts/box-plot.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {CoordinateGridMixin} from '../base/coordinate-grid-mixin';
import {transition} from '../core/core';
import {units} from '../core/units';
import {utils} from '../core/utils';
import {adaptHandler} from '../core/d3compat';

// Returns a function to compute the interquartile range.
function defaultWhiskersIQR (k) {
Expand Down Expand Up @@ -196,10 +197,10 @@ export class BoxPlot extends CoordinateGridMixin {
.attr('class', 'box')
.attr('transform', (d, i) => this._boxTransform(d, i))
.call(this._box)
.on('click', d => {
.on('click', adaptHandler(d => {
this.filter(this.keyAccessor()(d));
this.redrawGroup();
});
}));
return boxesGEnter.merge(boxesG);
}

Expand Down
3 changes: 2 additions & 1 deletion src/charts/bubble-chart.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {BubbleMixin} from '../base/bubble-mixin';
import {CoordinateGridMixin} from '../base/coordinate-grid-mixin';
import {transition} from '../core/core';
import {adaptHandler} from '../core/d3compat';

/**
* A concrete implementation of a general purpose bubble chart that allows data visualization using the
Expand Down Expand Up @@ -73,7 +74,7 @@ export class BubbleChart extends BubbleMixin(CoordinateGridMixin) {
.attr('class', this.BUBBLE_NODE_CLASS)
.attr('transform', d => this._bubbleLocator(d))
.append('circle').attr('class', (d, i) => `${this.BUBBLE_CLASS} _${i}`)
.on('click', d => this.onClick(d))
.on('click', adaptHandler(d => this.onClick(d)))
.attr('fill', this.getColor)
.attr('r', 0);

Expand Down
Loading