Skip to content

Commit

Permalink
fix(subchart, zoom): Fix returning domain value
Browse files Browse the repository at this point in the history 
- Adjust .subchart()/.zoom() domain return value
- Update subchart.onbrush callback to be called only by dragging or API call

Fix #3347
  • Loading branch information
@netil
netil authored and netil committed Sep 5, 2023
1 parent ac9e229 commit 321510b
Show file tree
Hide file tree
Showing 11 changed files with 181 additions and 91 deletions.
47 changes: 29 additions & 18 deletions src/Chart/api/subchart.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import type {TDomain} from "../../ChartInternal/data/IData";

/**
* Select subchart by giving x domain range.
* - **鈩癸笍 NOTE:**
* - Due to the limitations of floating point precision, domain value may not be exact returning approximately values.
* @function subchart
* @instance
* @memberof Chart
Expand All @@ -18,31 +20,40 @@ import type {TDomain} from "../../ChartInternal/data/IData";
* chart.subchart([1, 2]);
*
* // Get the current subchart selection domain range
* // Domain value may not be exact returning approximately values.
* chart.subchart();
*/
// NOTE: declared funciton assigning to variable to prevent duplicated method generation in JSDoc.
const subchart = function<T = TDomain[]>(domainValue?: T): T | undefined {
const $$ = this.internal;
const {axis, brush, config, scale: {x, subX}} = $$;
let domain: any = domainValue;
const {axis, brush, config, scale: {x, subX}, state} = $$;
let domain;

if (config.subchart_show && Array.isArray(domain)) {
if (axis.isTimeSeries()) {
domain = domain.map(x => parseDate.bind($$)(x));
}
if (config.subchart_show) {
domain = domainValue;

if (Array.isArray(domain)) {
if (axis.isTimeSeries()) {
domain = domain.map(x => parseDate.bind($$)(x));
}

const isWithinRange = $$.withinRange(
domain,
$$.getZoomDomain("subX", true),
$$.getZoomDomain("subX")
);

const isWithinRange = $$.withinRange(
domain,
$$.getZoomDomain("subX", true),
$$.getZoomDomain("subX")
);

isWithinRange && brush.move(
brush.getSelection(),
domain.map(subX)
);
} else {
domain = x.orgDomain();
if (isWithinRange) {
state.domain = domain;

brush.move(
brush.getSelection(),
domain.map(subX)
);
}
} else {
domain = state.domain ?? x.orgDomain();
}
}

return domain as T;
Expand Down
110 changes: 62 additions & 48 deletions src/Chart/api/zoom.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,11 @@ import type {TDomainRange} from "../../ChartInternal/data/IData";

/**
* Zoom by giving x domain range.
* - **NOTE:**
* - **鈩癸笍 NOTE:**
* - For `wheel` type zoom, the minimum zoom range will be set as the given domain range. To get the initial state, [.unzoom()](#unzoom) should be called.
* - To be used [zoom.enabled](Options.html#.zoom) option should be set as `truthy`.
* - When x axis type is `category`, domain range should be specified as index numbers.
* - Due to the limitations of floating point precision, domain value may not be exact returning approximately values.
* @function zoom
* @instance
* @memberof Chart
Expand All @@ -20,70 +22,80 @@ import type {TDomainRange} from "../../ChartInternal/data/IData";
* // Zoom to specified domain range
* chart.zoom([10, 20]);
*
* // For timeseries, the domain value can be string, but the format should match with the 'data.xFormat' option.
* // For timeseries x axis, the domain value can be string, but the format should match with the 'data.xFormat' option.
* chart.zoom(["2021-02-03", "2021-02-08"]);
*
* // For category x axis, the domain value should be index number.
* chart.zoom([0, 3]);
*
* // Get the current zoomed domain range
* // Domain value may not be exact returning approximately values.
* chart.zoom();
*/
// NOTE: declared funciton assigning to variable to prevent duplicated method generation in JSDoc.
const zoom = function<T = TDomainRange>(domainValue?: T): T | undefined {
const $$ = this.internal;
const {$el, axis, config, org, scale} = $$;
const {$el, axis, config, org, scale, state} = $$;
const isRotated = config.axis_rotated;
const isCategorized = axis.isCategorized();
let domain: any = domainValue;

if (config.zoom_enabled && Array.isArray(domain)) {
if (axis.isTimeSeries()) {
domain = domain.map(x => parseDate.bind($$)(x));
}
let domain;

const isWithinRange = $$.withinRange(
domain,
$$.getZoomDomain("zoom", true),
$$.getZoomDomain("zoom")
);
if (config.zoom_enabled) {
domain = domainValue;

if (isWithinRange) {
if (isCategorized) {
domain = domain.map((v, i) => Number(v) + (i === 0 ? 0 : 1)) as T;
if (Array.isArray(domain)) {
if (axis.isTimeSeries()) {
domain = domain.map(x => parseDate.bind($$)(x));
}

// hide any possible tooltip show before the zoom
$$.api.tooltip.hide();

if (config.subchart_show) {
const x = scale.zoom || scale.x;

$$.brush.getSelection().call($$.brush.move, domain.map(x));
// resultDomain = domain;
} else {
// in case of 'config.zoom_rescale=true', use org.xScale
const x = isCategorized ? scale.x.orgScale() : (org.xScale || scale.x);

// Get transform from given domain value
// https://github.com/d3/d3-zoom/issues/57#issuecomment-246434951
const translate = [-x(domain[0]), 0];
const transform = d3ZoomIdentity
.scale(x.range()[1] / (
x(domain[1]) - x(domain[0])
))
.translate(
...(isRotated ? translate.reverse() : translate) as [number, number]
);

$el.eventRect
.call($$.zoom.transform, transform);
const isWithinRange = $$.withinRange(
domain,
$$.getZoomDomain("zoom", true),
$$.getZoomDomain("zoom")
);

if (isWithinRange) {
state.domain = domain;

if (isCategorized) {
domain = domain.map((v, i) => Number(v) + (i === 0 ? 0 : 1));
}

// hide any possible tooltip show before the zoom
$$.api.tooltip.hide();

if (config.subchart_show) {
const x = scale.zoom || scale.x;

$$.brush.getSelection().call($$.brush.move, domain.map(x));
// resultDomain = domain;
} else {
// in case of 'config.zoom_rescale=true', use org.xScale
const x = isCategorized ? scale.x.orgScale() : (org.xScale || scale.x);

// Get transform from given domain value
// https://github.com/d3/d3-zoom/issues/57#issuecomment-246434951
const translate = [-x(domain[0]), 0];
const transform = d3ZoomIdentity
.scale(x.range()[1] / (
x(domain[1]) - x(domain[0])
))
.translate(
...(isRotated ? translate.reverse() : translate) as [number, number]
);

$el.eventRect
.call($$.zoom.transform, transform);
}

$$.setZoomResetButton();
}

$$.setZoomResetButton();
} else {
domain = $$.zoom.getDomain();
}
} else {
domain = scale.zoom?.domain() ?? scale.x.orgDomain();
}

return domain;
return state.domain ?? domain;
};

extend(zoom, {
Expand Down Expand Up @@ -217,7 +229,7 @@ export default {
*/
unzoom(): void {
const $$ = this.internal;
const {config, $el: {eventRect, zoomResetBtn}} = $$;
const {config, $el: {eventRect, zoomResetBtn}, state} = $$;

if ($$.scale.zoom) {
config.subchart_show ?
Expand All @@ -231,6 +243,8 @@ export default {
if (d3ZoomTransform(eventRect.node()) !== d3ZoomIdentity) {
$$.zoom.transform(eventRect, d3ZoomIdentity);
}

state.domain = undefined;
}
}
};
38 changes: 28 additions & 10 deletions src/ChartInternal/interactions/subchart.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ export default {
*/
initBrush(): void {
const $$ = this;
const {config, scale, $el: {subchart}} = $$;
const {config, scale, $el: {subchart}, state} = $$;
const isRotated = config.axis_rotated;
let lastDomain;
let lastSelection;
let timeout;

// set the brush
Expand All @@ -42,14 +43,25 @@ export default {

// bind brush event
$$.brush.on("start brush end", event => {
const {selection, target, type} = event;
const {selection, sourceEvent, target, type} = event;

if (type === "start") {
$$.state.inputType === "touch" && $$.hideTooltip();
lastSelection = sourceEvent ? selection : null;
// sourceEvent && (state.domain = null);
}

// if (type === "brush") {
if (/(start|brush)/.test(type)) {
$$.redrawForBrush();
// when brush selection updates happens on one edge, update only chainging edge and
// is only for adjustment of given domain range to be used to return current domain range.
type === "brush" && sourceEvent && state.domain && lastSelection?.forEach((v, i) => {
if (v !== selection[i]) {
state.domain[i] = scale.x.orgDomain()[i];
}
});

$$.redrawForBrush(type !== "start");
}

if (type === "end") {
Expand Down Expand Up @@ -320,23 +332,28 @@ export default {
$$.redrawCircle(cx, cy, withTransition, undefined, true);
}

!state.rendered && initRange && $$.brush.move(
$$.brush.getSelection(),
initRange.map($$.scale.x)
);
if (!state.rendered && initRange) {
state.domain = initRange;

$$.brush.move(
$$.brush.getSelection(),
initRange.map($$.scale.x)
);
}
}
}
},

/**
* Redraw the brush.
* @param {boolean} [callCallbck=true] Call 'onbrush' callback or not.
* @private
*/
redrawForBrush() {
redrawForBrush(callCallbck = true): void {
const $$ = this;
const {config: {
subchart_onbrush: onBrush, zoom_rescale: withY
}, scale} = $$;
}, scale, state} = $$;

$$.redraw({
withTransition: false,
Expand All @@ -346,7 +363,8 @@ export default {
withDimension: false
});

onBrush.bind($$.api)(scale.x.orgDomain());
callCallbck && state.rendered &&
onBrush.bind($$.api)(state.domain ?? scale.x.orgDomain());
},

/**
Expand Down
16 changes: 13 additions & 3 deletions src/ChartInternal/interactions/zoom.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export default {
* @private
*/
// @ts-ignore
zoom.getDomain = (): number|Date[] => {
zoom.getDomain = (): (number | Date)[] => {
const domain = scale[scale.zoom ? "zoom" : "subX"].domain();
const isCategorized = $$.axis.isCategorized();

Expand All @@ -135,6 +135,7 @@ export default {

return domain;
};

$$.zoom = zoom;
},

Expand Down Expand Up @@ -175,6 +176,7 @@ export default {

if (event.sourceEvent) {
state.zooming = true;
state.domain = undefined;
}

const isMousemove = sourceEvent?.type === "mousemove";
Expand Down Expand Up @@ -206,7 +208,11 @@ export default {
$$.state.cancelClick = isMousemove;

// do not call event cb when is .unzoom() is called
!isUnZoom && callFn(config.zoom_onzoom, $$.api, $$.zoom.getDomain());
!isUnZoom && callFn(
config.zoom_onzoom,
$$.api,
$$.state.domain ?? $$.zoom.getDomain()
);
},

/**
Expand Down Expand Up @@ -239,7 +245,11 @@ export default {
state.zooming = false;

// do not call event cb when is .unzoom() is called
!isUnZoom && (e || state.dragging) && callFn(config.zoom_onzoomend, $$.api, $$.zoom.getDomain());
!isUnZoom && (e || state.dragging) && callFn(
config.zoom_onzoomend,
$$.api,
$$.state.domain ?? $$.zoom.getDomain()
);
},

/**
Expand Down
3 changes: 3 additions & 0 deletions src/config/Store/State.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ export default class State {
cssRule: {},

current: {
// current domain value. Assigned when is zoom is called
domain: undefined,

// chart whole dimension
width: 0,
height: 0,
Expand Down
Loading

0 comments on commit 321510b

Please sign in to comment.