Skip to content

Commit

Permalink
fix(interaction): Correct rect element sizing
Browse files Browse the repository at this point in the history
- Update condition for rect resizing
- Some refactorings on ChartInternal.js

Fix #522
Close #523
  • Loading branch information
netil committed Aug 3, 2018
1 parent 19f5baf commit 6a09985
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 117 deletions.
28 changes: 13 additions & 15 deletions spec/api/api.zoom-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -190,25 +190,23 @@ describe("API zoom", function() {
});

it("should be zoomed properly", done => {
const target = [3, 5];
const internal = chart.internal;
const main = internal.main;
const rectlist = chart.$.main.selectAll(`.${CLASS.eventRect}`).nodes();
const rect = [];

const bars = main.select(`.${CLASS.chartBars}`).node();
const rects = main.select(`.${CLASS.eventRects}`).node();
const rectlist = main.selectAll(`.${CLASS.eventRect}`).nodes();
const orgWidth = bars.getBoundingClientRect().width;
const rectWidth = internal.getEventRectWidth();

chart.zoom(target);
// when
chart.zoom([3, 5]);

setTimeout(() => {
rectlist.forEach(v => {
expect(parseFloat(d3.select(v).attr("width"))).to.be.equal(rectWidth);
});
rectlist.forEach(function(el, i) {
const x = +el.getAttribute("x");
const width = +el.getAttribute("width");

if (i > 0) {
expect(rect[i - 1]).to.be.equal(x);
}

expect(bars.getBoundingClientRect().width/orgWidth).to.be.above(2.5);
expect(rects.getBoundingClientRect().width/orgWidth).to.be.above(2.5);
rect.push(x + width);
});

done();
}, 500)
Expand Down
61 changes: 45 additions & 16 deletions spec/interactions/interaction-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe("INTERACTION", () => {
const lefts = [69, 130, 198, 403];
const widths = [61, 68, 205, 197.5];

chart.internal.main.selectAll(`.${CLASS.eventRect}`).each(function(d, i) {
chart.$.main.selectAll(`.${CLASS.eventRect}`).each(function (d, i) {
const box = d3.select(this).node().getBoundingClientRect();

expect(box.left).to.be.closeTo(lefts[i], 10);
Expand All @@ -62,11 +62,11 @@ describe("INTERACTION", () => {
});

it("should have 1 event rects properly", () => {
const eventRects = chart.internal.main.selectAll(`.${CLASS.eventRect}`);
const eventRects = chart.$.main.selectAll(`.${CLASS.eventRect}`);

expect(eventRects.size()).to.be.equal(1);

eventRects.each(function() {
eventRects.each(function () {
const box = d3.select(this).node().getBoundingClientRect();

expect(box.left).to.be.closeTo(30.5, 10);
Expand All @@ -92,7 +92,7 @@ describe("INTERACTION", () => {
const lefts = [33.5, 185.5, 348, 497.5];
const widths = [152, 162.5, 149.5, 138.5];

chart.internal.main.selectAll(`.${CLASS.eventRect}`).each(function (d, i) {
chart.$.main.selectAll(`.${CLASS.eventRect}`).each(function (d, i) {
const box = d3.select(this).node().getBoundingClientRect();

expect(box.left).to.be.closeTo(lefts[i], 10);
Expand All @@ -115,19 +115,48 @@ describe("INTERACTION", () => {
});

it("should have 1 event rects properly", () => {
const eventRects = chart.internal.main.selectAll(`.${CLASS.eventRect}`);
const eventRects = chart.$.main.selectAll(`.${CLASS.eventRect}`);

expect(eventRects.size()).to.be.equal(1);

eventRects.each(function() {
eventRects.each(function () {
const box = d3.select(this).node().getBoundingClientRect();

expect(box.left).to.be.closeTo(30.5, 10);
expect(box.width).to.be.closeTo(608, 10);
});
});

describe("indexed", () => {
before(() => {
args = {
data: {
columns: [
["data", 10, 20, 30, 40, 50]
]
}
};
});

it("rect elements should be positioned without gaps", () => {
const rect = [];

chart.$.main.selectAll(`.${CLASS.eventRect}`).each(function(d, i) {
const x = +this.getAttribute("x");
const width = +this.getAttribute("width");

if (i > 0) {
expect(rect[i - 1]).to.be.equal(x);
}

rect.push(x + width);
});
});
});
});
});

describe("Different interactions", () => {
describe("check for data.onclick", () => {
let clicked = false;
let data;
Expand All @@ -153,7 +182,7 @@ describe("INTERACTION", () => {
});

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

Expand All @@ -173,7 +202,7 @@ describe("INTERACTION", () => {
});

it("check for data click for rectangle data point", () => {
const main = chart.internal.main;
const main = chart.$.main;
const rect = main.select(`.${CLASS.eventRect}.${CLASS.eventRect}`).node();
const circle = main.select(`.${CLASS.circles}-data1 rect`).node().getBBox();

Expand All @@ -196,7 +225,7 @@ describe("INTERACTION", () => {
});

it("check for data click for polygon data point", () => {
const main = chart.internal.main;
const main = chart.$.main;
const rect = main.select(`.${CLASS.eventRect}.${CLASS.eventRect}`).node();
const circle = main.select(`.${CLASS.circles}-data2 use`).node().getBBox();

Expand All @@ -218,7 +247,7 @@ describe("INTERACTION", () => {
});

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

Expand All @@ -238,7 +267,7 @@ describe("INTERACTION", () => {
});

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

Expand All @@ -258,7 +287,7 @@ describe("INTERACTION", () => {
});

it("check for data click for bubble", () => {
const main = chart.internal.main;
const main = chart.$.main;
const rect = main.select(`.${CLASS.eventRect}.${CLASS.eventRect}`).node();
const circle = main.select(`.${CLASS.circles}-data2 circle`).node().getBBox();
const delta = 50;
Expand All @@ -279,7 +308,7 @@ describe("INTERACTION", () => {
});

it("check for data click for bar", () => {
const main = chart.internal.main;
const main = chart.$.main;
const rect = main.select(`.${CLASS.eventRect}.${CLASS.eventRect}-0`).node();
const path = main.select(`.${CLASS.bars}-data1 path`).node().getBBox();

Expand All @@ -299,7 +328,7 @@ describe("INTERACTION", () => {
});

it("check for data click for pie", () => {
const main = chart.internal.main;
const main = chart.$.main;
const path = main.select(`.${CLASS.arcs}-data1 path`).node();
const box = path.getBBox();

Expand All @@ -319,7 +348,7 @@ describe("INTERACTION", () => {
});

it("check for data click for gauge", () => {
const main = chart.internal.main;
const main = chart.$.main;
const path = main.select(`.${CLASS.arcs}-data1 path`).node();
const box = path.getBBox();

Expand Down Expand Up @@ -358,7 +387,7 @@ describe("INTERACTION", () => {
});

it("check for data click for multiple xs", () => {
const main = chart.internal.main;
const main = chart.$.main;
const rect = main.select(`.${CLASS.eventRects}.${CLASS.eventRectsMultiple} rect`).node();
const circle = main.select(`.${CLASS.circles}.${CLASS.circles}-data1 circle`).node().getBBox();

Expand Down
8 changes: 4 additions & 4 deletions src/interactions/interaction.js
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,10 @@ extend(ChartInternal.prototype, {
let rectW;
let rectX;

if (($$.isCustomX() || $$.isTimeSeries()) && !$$.isCategorized()) {
if ($$.isCategorized()) {
rectW = $$.getEventRectWidth();
rectX = d => xScale(d.x) - (rectW / 2);
} else {
// update index for x that is used by prevX and nextX
$$.updateXs();

Expand Down Expand Up @@ -249,9 +252,6 @@ extend(ChartInternal.prototype, {

return (xScale(thisX) + xScale(prevX)) / 2;
};
} else {
rectW = $$.getEventRectWidth();
rectX = d => xScale(d.x) - (rectW / 2);
}

x = isRotated ? 0 : rectX;
Expand Down
Loading

0 comments on commit 6a09985

Please sign in to comment.