Skip to content

Commit

Permalink
fix(zoom): Correct not firing data.onclick callback
Browse files Browse the repository at this point in the history 
Split data click handlers as separate function and make to use
those for drag zoom

Fix #583
Close #584
  • Loading branch information
@netil
netil authored Sep 10, 2018
1 parent cd0d109 commit 7368446
Show file tree
Hide file tree
Showing 3 changed files with 151 additions and 108 deletions.
147 changes: 83 additions & 64 deletions spec/interactions/zoom-spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -134,87 +134,93 @@ describe("ZOOM", function() {
});

describe("zoom type drag", () => {
before(() => {
args = {
size: {
width: 300,
height: 250
},
data: {
columns: [
["data1", 30, 200, 100, 400, 3150, 250],
["data2", 50, 20, 10, 40, 15, 6025]
]
},
zoom: {
enabled: {
type: "drag"
}
}
};
});
let clickedData;

before(() => {
args = {
size: {
width: 300,
height: 250
},
data: {
columns: [
["data1", 30, 200, 100, 400, 3150, 250],
["data2", 50, 20, 10, 40, 15, 6025]
],
onclick: d => {
clickedData = d;
}

},
zoom: {
enabled: {
type: "drag"
}
}
};
});

it("check for data zoom", () => {
const main = chart.$.main;
const xValue = +main.select(`.${CLASS.eventRect}-2`).attr("x");
it("check for data zoom", () => {
const main = chart.$.main;
const xValue = +main.select(`.${CLASS.eventRect}-2`).attr("x");

// when
chart.zoom([0, 3]); // zoom in
// when
chart.zoom([0, 3]); // zoom in

expect(+main.select(`.${CLASS.eventRect}-2`).attr("x")).to.be.above(xValue);
});
expect(+main.select(`.${CLASS.eventRect}-2`).attr("x")).to.be.above(xValue);
});

it("check for x axis resize after zoom", () => {
const main = chart.$.main;
const rx = /H(\d+)/;
it("check for x axis resize after zoom", () => {
const main = chart.$.main;
const rx = /H(\d+)/;

const domain = main.select(`.${CLASS.axisX} > .domain`);
const pathValue = +domain.attr("d").match(rx)[1];
const domain = main.select(`.${CLASS.axisX} > .domain`);
const pathValue = +domain.attr("d").match(rx)[1];

chart.zoom([0, 4]);
chart.resize({ width: 400 });
chart.zoom([0, 4]);
chart.resize({width: 400});

expect(+domain.attr("d").match(rx)[1]).to.be.above(pathValue);
});
expect(+domain.attr("d").match(rx)[1]).to.be.above(pathValue);
});

it("check for x axis resize after zoom in/out", () => {
const main = chart.$.main;
const rx = /H(\d+)/;
it("check for x axis resize after zoom in/out", () => {
const main = chart.$.main;
const rx = /H(\d+)/;

const domain = main.select(`.${CLASS.axisX} > .domain`);
const pathValue = +domain.attr("d").match(rx)[1];
const domain = main.select(`.${CLASS.axisX} > .domain`);
const pathValue = +domain.attr("d").match(rx)[1];

chart.zoom([0, 4]); // zoom in
chart.zoom([0, 6]); // zoom out
chart.zoom([0, 4]); // zoom in
chart.zoom([0, 6]); // zoom out

expect(+domain.attr("d").match(rx)[1]).to.be.equal(pathValue);
expect(+domain.attr("d").match(rx)[1]).to.be.equal(pathValue);

// resize
chart.resize({ width: 400 });
// resize
chart.resize({width: 400});

// check if chart react on resize
expect(+domain.attr("d").match(rx)[1]).to.be.above(pathValue);
});
// check if chart react on resize
expect(+domain.attr("d").match(rx)[1]).to.be.above(pathValue);
});

it("check for the reset zoom button", () => {
// when
chart.zoom([0, 4]);
it("check for the reset zoom button", () => {
// when
chart.zoom([0, 4]);

const resetBtn = chart.$.chart.select(`.${CLASS.buttonZoomReset}`);
const resetBtn = chart.$.chart.select(`.${CLASS.buttonZoomReset}`);

expect(resetBtn.empty()).to.be.false;
expect(resetBtn.empty()).to.be.false;

// when button is clicked
resetBtn.node().click();
// when button is clicked
resetBtn.node().click();

expect(resetBtn.style("display")).to.be.equal("none");
});
expect(resetBtn.style("display")).to.be.equal("none");
});

it("set options zoom.resetButton.text='test'", () => {
args.zoom.resetButton = {
text: "test"
};
});
it("set options zoom.resetButton.text='test'", () => {
args.zoom.resetButton = {
text: "test"
};
});

it("check for the custom reset zoom button text", () => {
// when
Expand All @@ -231,7 +237,7 @@ describe("ZOOM", function() {
});

it("check for the y axis rescale", () => {
const axisY = chart.$.main.select(`.${CLASS.axisY}`);
const axisY = chart.$.main.select(`.${CLASS.axisY}`);

// when
chart.zoom([0, 2]);
Expand All @@ -241,12 +247,25 @@ describe("ZOOM", function() {
expect(+tick.textContent).to.be.equal(200);

// when
chart.zoom([2,4]);
chart.zoom([2, 4]);

tick = axisY.selectAll(".tick").nodes().pop();

expect(+tick.textContent).to.be.equal(3000);
});

it("check for data.onclick", () => {
const main = chart.$.main;
const rect = main.select(`.${CLASS.eventRect}.${CLASS.eventRect}-0`).node();
const circle = chart.$.line.circles.node().getBBox();

util.fireEvent(rect, "click", {
clientX: circle.x,
clientY: circle.y
}, chart);

expect(clickedData).to.not.be.undefined;
});
});

describe("zoom on regions", () => {
Expand Down
96 changes: 54 additions & 42 deletions src/interactions/interaction.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -440,22 +440,8 @@ extend(ChartInternal.prototype, {
const rect = eventRectEnter.append("rect")
.attr("class", $$.classEvent.bind($$))
.style("cursor", config.data_selection_enabled && config.data_selection_grouped ? "pointer" : null)
.on("click", d => {
if ($$.hasArcType() || !$$.toggleShape || $$.cancelClick) {
$$.cancelClick && ($$.cancelClick = false);

return;
}

const index = d.index;

$$.main.selectAll(`.${CLASS.shape}-${index}`)
.each(function(d2) {
if (config.data_selection_grouped || $$.isWithinShape(this, d2)) {
$$.toggleShape(this, d2, index);
$$.config.data_onclick.call($$.api, d2, this);
}
});
.on("click", function(d) {
$$.clickHandlerForSingleX.bind(this)(d, $$);
})
.call($$.getDraggableSelection());

Expand Down Expand Up @@ -507,6 +493,27 @@ extend(ChartInternal.prototype, {
return rect;
},

clickHandlerForSingleX(d, ctx) {
const $$ = ctx;
const config = $$.config;

if ($$.hasArcType() || !$$.toggleShape || $$.cancelClick) {
$$.cancelClick && ($$.cancelClick = false);

return;
}

const index = d.index;

$$.main.selectAll(`.${CLASS.shape}-${index}`)
.each(function(d2) {
if (config.data_selection_grouped || $$.isWithinShape(this, d2)) {
$$.toggleShape(this, d2, index);
config.data_onclick.call($$.api, d2, this);
}
});
},

/**
* Create an eventRect,
* Register touch and drag events.
Expand All @@ -516,7 +523,6 @@ extend(ChartInternal.prototype, {
*/
generateEventRectsForMultipleXs(eventRectEnter) {
const $$ = this;
const config = $$.config;

const rect = eventRectEnter
.append("rect")
Expand All @@ -526,30 +532,7 @@ extend(ChartInternal.prototype, {
.attr("height", $$.height)
.attr("class", CLASS.eventRect)
.on("click", function() {
const targetsToShow = $$.filterTargetsToShow($$.data.targets);

if ($$.hasArcType(targetsToShow)) {
return;
}

const mouse = d3Mouse(this);
const closest = $$.findClosestFromTargets(targetsToShow, mouse);

if (!closest) {
return;
}

// select if selection enabled
if ($$.isBarType(closest.id) || $$.dist(closest, mouse) < config.point_sensitivity) {
$$.main.selectAll(`.${CLASS.shapes}${$$.getTargetSelectorSuffix(closest.id)}`)
.selectAll(`.${CLASS.shape}-${closest.index}`)
.each(function() {
if (config.data_selection_grouped || $$.isWithinShape(this, closest)) {
$$.toggleShape(this, closest, closest.index);
$$.config.data_onclick.call($$.api, closest, this);
}
});
}
$$.clickHandlerForMultipleXS.bind(this)($$);
})
.call($$.getDraggableSelection());

Expand All @@ -571,6 +554,35 @@ extend(ChartInternal.prototype, {
return rect;
},

clickHandlerForMultipleXS(ctx) {
const $$ = ctx;
const config = $$.config;
const targetsToShow = $$.filterTargetsToShow($$.data.targets);

if ($$.hasArcType(targetsToShow)) {
return;
}

const mouse = d3Mouse(this);
const closest = $$.findClosestFromTargets(targetsToShow, mouse);

if (!closest) {
return;
}

// select if selection enabled
if ($$.isBarType(closest.id) || $$.dist(closest, mouse) < config.point_sensitivity) {
$$.main.selectAll(`.${CLASS.shapes}${$$.getTargetSelectorSuffix(closest.id)}`)
.selectAll(`.${CLASS.shape}-${closest.index}`)
.each(function() {
if (config.data_selection_grouped || $$.isWithinShape(this, closest)) {
$$.toggleShape(this, closest, closest.index);
config.data_onclick.call($$.api, closest, this);
}
});
}
},

/**
* Dispatch a mouse event.
* @private
Expand All @@ -595,6 +607,6 @@ extend(ChartInternal.prototype, {
clientY: y
};

emulateEvent[/^mouse/.test(type) ? "mouse" : "touch"](eventRect, type, params);
emulateEvent[/^(mouse|click)/.test(type) ? "mouse" : "touch"](eventRect, type, params);
}
});
16 changes: 14 additions & 2 deletions src/interactions/zoom.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import {
} from "d3-array";
import {
mouse as d3Mouse,
event as d3Event
event as d3Event,
select as d3Select
} from "d3-selection";
import {drag as d3Drag} from "d3-drag";
import {zoom as d3Zoom} from "d3-zoom";
Expand Down Expand Up @@ -243,6 +244,7 @@ extend(ChartInternal.prototype, {
let zoomRect = null;

$$.zoomBehaviour = d3Drag()
.clickDistance(4)
.on("start", function() {
$$.setDragStatus(true);

Expand Down Expand Up @@ -270,7 +272,7 @@ extend(ChartInternal.prototype, {
.attr("x", Math.min(start, end))
.attr("width", Math.abs(end - start));
})
.on("end", () => {
.on("end", function(d) {
const scale = $$.zoomScale || $$.x;

$$.setDragStatus(false);
Expand All @@ -291,6 +293,16 @@ extend(ChartInternal.prototype, {
if (start !== end) {
$$.api.zoom([start, end].map(v => scale.invert(v)));
$$.onZoomEnd();
} else {
if ($$.isMultipleX()) {
$$.clickHandlerForMultipleXS.bind(this)($$);
} else {
const event = d3Event.sourceEvent || d3Event;
const [x, y] = "clientX" in event ? [event.clientX, event.clientY] : [event.x, event.y];
const target = document.elementFromPoint(x, y);

$$.clickHandlerForSingleX.bind(target)(d3Select(target).datum(), $$);
}
}
});
},
Expand Down

0 comments on commit 7368446

Please sign in to comment.