Skip to content

Commit

Permalink
fix(zoom): Correct zooming for category axis type (#441)
Browse files Browse the repository at this point in the history 
- Added internal 'getCustomizedScale()' to handle scale updates when zoom occurs and sum with the category offset
- Update default value for zoom events options(onzoomstart, onzoom, onzoomend) to undefined to avoid unnecessary call
- Reset zoom scaling when `.flush()` is called
- Moved 'emulateEvent' to util.js
- And some refactorings

Fix #177
Close #441
  • Loading branch information
@netil
netil authored Jun 7, 2018
1 parent 1158c71 commit c8a164a
Show file tree
Hide file tree
Showing 13 changed files with 481 additions and 383 deletions.
90 changes: 70 additions & 20 deletions spec/api/api.zoom-spec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,21 @@ describe("API zoom", function() {

chart.zoom(target);

const domain = chart.internal.zoomScale.domain();
const domain = chart.internal.zoomScale.domain().map(Math.round);

expect(Math.round(domain[0])).to.be.equal(target[0]);
expect(Math.round(domain[1])).to.be.equal(target[1]);
expect(domain[0]).to.be.equal(target[0]);
expect(domain[1]).to.be.equal(target[1]);
});

it("should be zoomed properly again", () => {
const target = [1, 4];

chart.zoom(target);

const domain = chart.internal.zoomScale.domain();
const domain = chart.internal.zoomScale.domain().map(Math.round);

expect(Math.round(domain[0])).to.be.equal(target[0]);
expect(Math.round(domain[1])).to.be.equal(target[1]);
expect(domain[0]).to.be.equal(target[0]);
expect(domain[1]).to.be.equal(target[1]);
});

it("should be zoomed and showing focus grid properly when target contained minus value", () => {
Expand All @@ -57,11 +57,11 @@ describe("API zoom", function() {
// If target contained minus value should not be null
expect(zoomScale).to.not.be.null;

const domain = chart.internal.zoomScale.domain();
const domain = chart.internal.zoomScale.domain().map(Math.round);

// domain value must be above than target
expect(Math.round(domain[0])).to.be.above(target[0]);
expect(Math.round(domain[1])).to.be.above(target[1]);
expect(domain[0]).to.be.above(target[0]);
expect(domain[1]).to.be.above(target[1]);
});
});

Expand Down Expand Up @@ -126,6 +126,52 @@ describe("API zoom", function() {
});
});

describe("zoom category type", () => {
before(() => {
chart = util.generate({
data: {
columns: [
["data1", 30, 200, 100, 400, 150, 250]
]
},
axis: {
x: {
type: "category"
}
},
zoom: {
enabled: true
}
});
});

it("should be zoomed properly", done => {
const target = [1,2];

const internal = chart.internal;

chart.zoom(target);

setTimeout(() => {
const rectlist = internal.main.selectAll(`.${CLASS.eventRect}`)
.filter((v, i) => target.indexOf(i) !== -1);
const rectSize = rectlist.attr("width");
const domain = internal.zoomScale.domain().map(Math.round);

expect(domain[0]).to.be.equal(target[0]);
expect(domain[1] - 1).to.be.equal(target[1]);

rectlist.each(function(d, i) {
const x = +d3.select(this).attr("x");

expect(x * i).to.be.closeTo(rectSize * i, 5);
});

done();
}, 1000);
});
});

describe("zoom bar chart", () => {
before(() => {
chart = util.generate({
Expand All @@ -145,18 +191,22 @@ describe("API zoom", function() {

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

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 = chart.internal.getEventRectWidth();
const rectWidth = internal.getEventRectWidth();

chart.zoom(target);

setTimeout(() => {
rectlist.forEach(v => {
expect(parseFloat(d3.select(v).attr("width"))).to.be.equal(rectWidth);
});

expect(bars.getBoundingClientRect().width/orgWidth).to.be.above(2.5);
expect(rects.getBoundingClientRect().width/orgWidth).to.be.above(2.5);

Expand Down Expand Up @@ -184,10 +234,10 @@ describe("API zoom", function() {

chart.zoom(target);

domain = chart.internal.zoomScale.domain();
domain = chart.internal.zoomScale.domain().map(Math.round);

expect(Math.round(domain[0])).to.be.equal(target[0]);
expect(Math.round(domain[1])).to.be.equal(target[1]);
expect(domain[0]).to.be.equal(target[0]);
expect(domain[1]).to.be.equal(target[1]);

chart.unzoom();

Expand Down Expand Up @@ -226,8 +276,8 @@ describe("API zoom", function() {
});

// check the returned domain value
chart.zoom(domain).forEach((v, i) => {
expect(Math.round(v)).to.not.equal(domain[i]);
chart.zoom(domain).map(Math.round).forEach((v, i) => {
expect(v).to.not.equal(domain[i]);
});

expect(+main.select(selector).attr("x")).to.be.equal(xValue);
Expand All @@ -240,8 +290,8 @@ describe("API zoom", function() {
// when enable zoom
chart.zoom.enable(true);

chart.zoom(domain).forEach((v, i) => {
expect(Math.round(v)).to.equal(domain[i]);
chart.zoom(domain).map(Math.round).forEach((v, i) => {
expect(v).to.equal(domain[i]);
});

expect(+main.select(selector).attr("x")).to.below(xValue);
Expand Down
3 changes: 3 additions & 0 deletions src/api/api.chart.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ extend(Chart.prototype, {
* chart.flush();
*/
flush() {
// reset possible zoom scale
this.internal.zoomScale = null;

this.internal.updateAndRedraw({
withLegend: true,
withTransition: false,
Expand Down
6 changes: 3 additions & 3 deletions src/api/api.zoom.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
} from "d3-array";
import {zoomIdentity as d3ZoomIdentity} from "d3-zoom";
import Chart from "../internals/Chart";
import {isDefined, isObject, extend} from "../internals/util";
import {isDefined, isObject, isFunction, extend} from "../internals/util";

/**
* Zoom by giving x domain.
Expand Down Expand Up @@ -44,7 +44,7 @@ const zoom = function(domainValue) {
const orgDomain = $$.x.orgDomain();
const k = (orgDomain[1] - orgDomain[0]) / (domain[1] - domain[0]);
const tx = isTimeSeries ?
(0 - k * $$.x(domain[0].getTime())) : domain[0] - k * $$.x(domain[0]);
(0 - k * $$.x(domain[0].getTime())) : domain[0] - k * ($$.x(domain[0]) - $$.xAxis.tickOffset());

$$.zoom.updateTransformScale(
d3ZoomIdentity.translate(tx, 0).scale(k)
Expand All @@ -59,7 +59,7 @@ const zoom = function(domainValue) {
withDimension: false
});

$$.config.zoom_onzoom.call(this, $$.x.orgDomain());
isFunction($$.config.zoom_onzoom) && $$.config.zoom_onzoom.call(this, $$.x.orgDomain());
} else {
resultDomain = ($$.zoomScale || $$.x).domain();
}
Expand Down
12 changes: 8 additions & 4 deletions src/axis/bb.axis.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,10 +121,8 @@ export default function(params = {}) {
function copyScale() {
const newScale = scale.copy();

if (params.isCategory || !newScale.domain().length) {
const domain = scale.domain();

newScale.domain([domain[0], domain[1] - 1]);
if (!newScale.domain().length) {
newScale.domain(scale.domain());
}

return newScale;
Expand Down Expand Up @@ -464,6 +462,12 @@ export default function(params = {}) {
return axis;
};

/**
* Return tick's offset value.
* The value will be set for 'category' axis type.
* @return {number}
* @private
*/
axis.tickOffset = () => tickOffset;

/**
Expand Down
16 changes: 8 additions & 8 deletions src/config/Options.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,11 @@ export default class Options {
* @property {Array} [zoom.extent=[1, 10]] Change zoom extent.
* @property {Number} [zoom.x.min] Set x Axis minimum zoom range
* @property {Number} [zoom.x.max] Set x Axis maximum zoom range
* @property {Function} [zoom.onzoom=function(){}] Set callback that is called when the chart is zooming.<br>
* Specified function receives the zoomed domain.
* @property {Function} [zoom.onzoomstart=function(){}] Set callback that is called when zooming starts.<br>
* @property {Function} [zoom.onzoomstart=undefined] Set callback that is called when zooming starts.<br>
* Specified function receives the zoom event.
* @property {Function} [zoom.onzoomend=function(){}] Set callback that is called when zooming ends.<br>
* @property {Function} [zoom.onzoom=undefined] Set callback that is called when the chart is zooming.<br>
* Specified function receives the zoomed domain.
* @property {Function} [zoom.onzoomend=undefined] Set callback that is called when zooming ends.<br>
* Specified function receives the zoomed domain.
* @example
* zoom: {
Expand All @@ -148,18 +148,18 @@ export default class Options {
* min: -1, // set min range
* max: 10 // set max range
* },
* onzoom: function(domain) { ... },
* onzoomstart: function(event) { ... },
* onzoom: function(domain) { ... },
* onzoomend: function(domain) { ... }
* }
*/
zoom_enabled: false,
zoom_extent: undefined,
zoom_privileged: false,
zoom_rescale: false,
zoom_onzoom: () => {},
zoom_onzoomstart: () => {},
zoom_onzoomend: () => {},
zoom_onzoom: undefined,
zoom_onzoomstart: undefined,
zoom_onzoomend: undefined,
zoom_x_min: undefined,
zoom_x_max: undefined,

Expand Down
Loading

0 comments on commit c8a164a

Please sign in to comment.