Skip to content

Commit

Permalink
fix(zoom): Correct not to re-scale while zoom is disabled (#273)
Browse files Browse the repository at this point in the history
- Fix the x Axis re-scaling
- Reinforce zoom APIs and options description
- Updated to return when param is not given for .zoom.max(), .zoom.min() and .zoom.range()

Fix #272
Close #273
  • Loading branch information
netil authored Feb 6, 2018
1 parent 874d6c0 commit 502d6fd
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 34 deletions.
59 changes: 55 additions & 4 deletions spec/api/api.zoom-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/
/* eslint-disable */
import util from "../assets/util";
import CLASS from "../../src/config/classes";

describe("API zoom", function() {
let chart;
Expand Down Expand Up @@ -144,9 +145,9 @@ describe("API zoom", function() {

it("should be zoomed properly", done => {
const target = [3, 5];
const bars = d3.select(".bb-chart-bars").node();
const rects = d3.select(".bb-event-rects").node();
const rectlist = d3.selectAll(".bb-event-rect").nodes();
const bars = d3.select(`.${CLASS.chartBars}`).node();
const rects = d3.select(`.${CLASS.eventRects}`).node();
const rectlist = d3.selectAll(`.${CLASS.eventRect}`).nodes();
const orgWidth = bars.getBoundingClientRect().width;
const rectWidth = chart.internal.getEventRectWidth();

Expand Down Expand Up @@ -197,5 +198,55 @@ describe("API zoom", function() {
});
});

// @TODO zoom.enable / max/min/range
describe("zoom.enable()", () => {
chart = util.generate({
data: {
columns: [
["data1", 30, 200, 100, 400, 150, 250]
]
},
zoom: {
enabled: true
}
});

it("should be disabled & enabled zoom", () => {
const main = chart.internal.main;
const domain = [1, 2];

// when disable zoom
chart.zoom.enable(false);

const selector = `.${CLASS.eventRect}-1`;
const xValue = +main.select(selector).attr("x");
const tickTransform = [];

main.selectAll(`.${CLASS.axisX} .tick`).each(function() {
tickTransform.push(this.getAttribute("transform"));
});

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

expect(+main.select(selector).attr("x")).to.be.equal(xValue);

// check x Axis to not be zoomed
main.selectAll(`.${CLASS.axisX} .tick`).each(function(i) {
expect(this.getAttribute("transform")).to.be.equal(tickTransform[i]);
});

// when enable zoom
chart.zoom.enable(true);

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

expect(+main.select(selector).attr("x")).to.below(xValue);
});
});

// @TODO zoom.max/min/range
});
4 changes: 2 additions & 2 deletions spec/interactions/zoom-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ describe("ZOOM", function() {
}
},
zoom: {
enable: true
enabled: true
},
subchart: {
show: true
Expand Down Expand Up @@ -83,7 +83,7 @@ describe("ZOOM", function() {
]
},
zoom: {
enable: true
enabled: true
}
};
});
Expand Down
63 changes: 35 additions & 28 deletions src/api/api.zoom.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,15 @@ import {
zoomIdentity as d3ZoomIdentity,
} from "d3";
import Chart from "../internals/Chart";
import {isDefined, extend} from "../internals/util";
import {isDefined, isObject, extend} from "../internals/util";

/**
* Zoom by giving x domain.
* @method zoom
* @instance
* @memberOf Chart
* @param {Array} domainValue If domain is given, the chart will be zoomed to the given domain. If no argument is given, the current zoomed domain will be returned.
* @return {Array} domain value in array
* @example
* // Zoom to specified domain
* chart.zoom([10, 20]);
Expand All @@ -25,11 +26,12 @@ import {isDefined, extend} from "../internals/util";
*/
const zoom = function(domainValue) {
const $$ = this.internal;
const isTimeSeries = $$.isTimeSeries();
let domain = domainValue;
let resultDomain;

if (domain) {
if ($$.isTimeSeries()) {
if ($$.config.zoom_enabled && domain) {
if (isTimeSeries) {
domain = domain.map(x => $$.parseDate(x));
}

Expand All @@ -41,12 +43,12 @@ const zoom = function(domainValue) {
} else {
const orgDomain = $$.x.orgDomain();
const k = (orgDomain[1] - orgDomain[0]) / (domain[1] - domain[0]);
const tx = $$.isTimeSeries() ?
const tx = isTimeSeries ?
(0 - k * $$.x(domain[0].getTime())) : domain[0] - k * $$.x(domain[0]);

$$.zoom.updateTransformScale(d3ZoomIdentity
.translate(tx, 0)
.scale(k));
$$.zoom.updateTransformScale(
d3ZoomIdentity.translate(tx, 0).scale(k)
);

resultDomain = $$.zoomScale.domain();
}
Expand All @@ -71,65 +73,72 @@ extend(zoom, {
* @method zoom․enable
* @instance
* @memberOf Chart
* @param {Boolean} enabled If enabled is true, the feature of zooming will be enabled. If false is given, it will be disabled.
* @param {Boolean} enabled If enabled is true, the feature of zooming will be enabled. If false is given, it will be disabled.<br>When set to false, the current zooming status will be reset.
* @example
* // Enable zooming
* chart.zoom.enable(true);
*
* // Disable zooming
* chart.zoom.enable(false);
*/
enable: function(enabled) {
enable: function(enabled = false) {
const $$ = this.internal;

$$.config.zoom_enabled = enabled;
$$.updateAndRedraw();
},

/**
* Set zoom max
* Set or get x Axis maximum zoom range value
* @method zoom․max
* @instance
* @memberOf Chart
* @param {Number} max
* @param {Number} [max] maximum value to set for zoom
* @return {Number} zoom max value
* @example
* // Set maximum range value
* chart.zoom.max(20);
*/
max: function(max) {
const $$ = this.internal;
const config = $$.config;

if (max === 0 || max) {
config.zoom_x_max = d3Max([$$.orgXDomain[1], max]);
} else {
return config.zoom_x_max;
}

return undefined;
return config.zoom_x_max;
},

/**
* Set zoom min
* Set or get x Axis minimum zoom range value
* @method zoom․min
* @instance
* @memberOf Chart
* @param {Number} min
* @param {Number} [min] minimum value tp set for zoom
* @return {Number} zoom min value
* @example
* // Set minimum range value
* chart.zoom.min(-1);
*/
min: function(min) {
const $$ = this.internal;
const config = $$.config;

if (min === 0 || min) {
config.zoom_x_min = d3Min([$$.orgXDomain[0], min]);
} else {
return config.zoom_x_min;
}

return undefined;
return config.zoom_x_min;
},

/**
* Set zoom range
* @method zoom․range
* @instance
* @memberOf Chart
* @param {Object} range
* @return {Object} range
* @param {Object} [range]
* @return {Object} zoom range value
* {
* min: 0,
* max: 100
Expand All @@ -143,17 +152,15 @@ extend(zoom, {
range: function(range) {
const domain = this.domain;

if (arguments.length) {
if (isObject(range)) {
isDefined(range.max) && domain.max(range.max);
isDefined(range.min) && domain.min(range.min);
} else {
return {
max: domain.max(),
min: domain.min()
};
}

return undefined;
return {
max: domain.max(),
min: domain.min()
};
}
});

Expand Down
6 changes: 6 additions & 0 deletions src/config/Options.js
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ export default class Options {
* @property {Boolean} [zoom.rescale=false] Enable to rescale after zooming.<br>
* If true set, y domain will be updated according to the zoomed region.
* @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>
Expand All @@ -142,6 +144,10 @@ export default class Options {
* enabled: true,
* rescale: true,
* extent: [1, 100] // enable more zooming
* x: {
* min: -1, // set min range
* max: 10 // set max range
* },
* onzoom: function(domain) { ... },
* onzoomstart: function(event) { ... },
* onzoomend: function(domain) { ... }
Expand Down

0 comments on commit 502d6fd

Please sign in to comment.