From 567b323ed58701ec06cde183238b6460c0fa4d6e Mon Sep 17 00:00:00 2001 From: Jae Sung Park Date: Thu, 25 Jul 2024 17:12:14 +0900 Subject: [PATCH] feat(regions): Enhance regions rendering Improve dashed line rendering using css instead path command. Will render same way for nullish value. Fix #3830 Close #3790 --- src/ChartInternal/data/data.ts | 11 ++ src/ChartInternal/internals/scale.ts | 8 +- src/ChartInternal/shape/line.ts | 185 +++++++++++++++++++++------ src/config/Options/data/axis.ts | 8 +- test/internals/data-spec.ts | 136 ++++++++++++++------ test/internals/rergions-spec.ts | 8 +- 6 files changed, 270 insertions(+), 86 deletions(-) diff --git a/src/ChartInternal/data/data.ts b/src/ChartInternal/data/data.ts index 5b6c9eaf5..735811c9b 100644 --- a/src/ChartInternal/data/data.ts +++ b/src/ChartInternal/data/data.ts @@ -683,10 +683,21 @@ export default { (isObjectType(dataLabels) && notEmpty(dataLabels)); }, + /** + * Determine if has null value + * @param {Array} targets Data array to be evaluated + * @returns {boolean} + * @private + */ + hasNullDataValue(targets: IDataRow[]): boolean { + return targets.some(({value}) => value === null); + }, + /** * Get data index from the event coodinates * @param {Event} event Event object * @returns {number} + * @private */ getDataIndexFromEvent(event): number { const $$ = this; diff --git a/src/ChartInternal/internals/scale.ts b/src/ChartInternal/internals/scale.ts index 9d22b0e75..309742cde 100644 --- a/src/ChartInternal/internals/scale.ts +++ b/src/ChartInternal/internals/scale.ts @@ -15,12 +15,12 @@ import type {IDataRow, IGridData} from "../data/IData"; /** * Get scale * @param {string} [type='linear'] Scale type - * @param {number} [min] Min range - * @param {number} [max] Max range + * @param {number|Date} [min] Min range + * @param {number|Date} [max] Max range * @returns {d3.scaleLinear|d3.scaleTime} scale * @private */ -export function getScale(type = "linear", min = 0, max = 1): any { +export function getScale(type = "linear", min, max): any { const scale = ({ linear: d3ScaleLinear, log: d3ScaleSymlog, @@ -32,7 +32,7 @@ export function getScale(type = "linear", min = 0, max = 1): any { scale.type = type; /_?log/.test(type) && scale.clamp(true); - return scale.range([min, max]); + return scale.range([min ?? 0, max ?? 1]); } export default { diff --git a/src/ChartInternal/shape/line.ts b/src/ChartInternal/shape/line.ts index edb038f06..a165fbdd3 100644 --- a/src/ChartInternal/shape/line.ts +++ b/src/ChartInternal/shape/line.ts @@ -13,8 +13,72 @@ import { isValue, parseDate } from "../../module/util"; +import type {IDataRow} from "../data/IData"; import {getScale} from "../internals/scale"; +/** + * Get stroke dasharray style value + * @param {number} start Start position in path length + * @param {number} end End position in path length + * @param {Array} pattern Dash array pattern + * @param {boolean} isLastX Weather is last x tick + * @returns {object} Stroke dasharray style value and its length + * @private + */ +function getStrokeDashArray(start: number, end: number, pattern: [number, number], + isLastX = false): {dash: string, length: number} { + const dash = start ? [start, 0] : pattern; + + for (let i = start ? start : pattern.reduce((a, c) => a + c); i <= end;) { + pattern.forEach(v => { + if (i + v <= end) { + dash.push(v); + } + + i += v; + }); + } + + // make sure to have even length + dash.length % 2 !== 0 && dash.push(isLastX ? pattern[1] : 0); + + return { + dash: dash.join(" "), + length: dash.reduce((a, b) => a + b, 0) + }; +} + +/** + * Get regions data + * @param {Array} d Data object + * @param {object} _regions regions to be set + * @param {boolean} isTimeSeries whether is time series + * @returns {object} Regions data + * @private + */ +function getRegions(d, _regions, isTimeSeries) { + const $$ = this; + const regions: {start: number | string, end: number | string, style: string}[] = []; + const dasharray = "2 2"; // default value + + // Check start/end of regions + if (isDefined(_regions)) { + const getValue = (v: Date | any, def: number | Date): Date | any => ( + isUndefined(v) ? def : (isTimeSeries ? parseDate.call($$, v) : v) + ); + + for (let i = 0, reg; (reg = _regions[i]); i++) { + const start = getValue(reg.start, d[0].x); + const end = getValue(reg.end, d[d.length - 1].x); + const style = reg.style || {dasharray}; + + regions[i] = {start, end, style}; + } + } + + return regions; +} + export default { initLine(): void { const {$el} = this; @@ -211,34 +275,31 @@ export default { }; }, - lineWithRegions(d, x, y, _regions): string { + /** + * Set regions dasharray and get path + * @param {Array} d Data object + * @param {Function} x x scale function + * @param {Function} y y scale function + * @param {object} _regions regions to be set + * @returns {stirng} Path string + * @private + */ + lineWithRegions(d: IDataRow[], x, y, _regions): string { const $$ = this; const {config} = $$; const isRotated = config.axis_rotated; const isTimeSeries = $$.axis.isTimeSeries(); - const regions: any[] = []; const dasharray = "2 2"; // default value + const regions = getRegions.bind($$)(d, _regions, isTimeSeries); + + // when contains null data, can't apply style dashed + const hasNullDataValue = $$.hasNullDataValue(d); let xp; let yp; let diff; let diffx2; - // Check start/end of regions - if (isDefined(_regions)) { - const getValue = (v: Date | any, def: number): Date | any => ( - isUndefined(v) ? def : (isTimeSeries ? parseDate.call($$, v) : v) - ); - - for (let i = 0, reg; (reg = _regions[i]); i++) { - const start = getValue(reg.start, d[0].x); - const end = getValue(reg.end, d[d.length - 1].x); - const style = reg.style || {dasharray}; - - regions[i] = {start, end, style}; - } - } - // Set scales const xValue = isRotated ? dt => y(dt.value) : dt => x(dt.x); const yValue = isRotated ? dt => x(dt.x) : dt => y(dt.value); @@ -279,15 +340,12 @@ export default { yDiff = y0; } - const points = isRotated ? - [ - [yValue, xValue], - [yDiff, xDiff] - ] : - [ - [xValue, yValue], - [xDiff, yDiff] - ]; + const points = [ + [xValue, yValue], + [xDiff, yDiff] + ]; + + isRotated && points.forEach(v => v.reverse()); return generateM(points); }; @@ -296,6 +354,16 @@ export default { const axisType = {x: $$.axis.getAxisType("x"), y: $$.axis.getAxisType("y")}; let path = ""; + // clone the line path to be used to get length value + const target = $$.$el.line.filter(({id}) => id === d[0].id); + const tempNode = target.clone().style("display", "none"); + const getLength = (node, path) => node.attr("d", path).node().getTotalLength(); + const dashArray = { + dash: [], + lastLength: 0 + }; + let isLastX = false; + for (let i = 0, data; (data = d[i]); i++) { const prevData = d[i - 1]; const hasPrevData = prevData && isValue(prevData.value); @@ -316,24 +384,69 @@ export default { xp = getScale(axisType.x, prevData.x, data.x); yp = getScale(axisType.y, prevData.value, data.value); - const dx = x(data.x) - x(prevData.x); - const dy = y(data.value) - y(prevData.value); - const dd = Math.sqrt(Math.pow(dx, 2) + Math.pow(dy, 2)); + // when it contains null data, dash can't be applied with style + if (hasNullDataValue) { + const dx = x(data.x) - x(prevData.x); + const dy = y(data.value) - y(prevData.value); + const dd = Math.sqrt(Math.pow(dx, 2) + Math.pow(dy, 2)); - diff = style[0] / dd; // dash - diffx2 = diff * style[1]; // gap + diff = style[0] / dd; // dash + diffx2 = diff * style[1]; // gap - for (let j = diff; j <= 1; j += diffx2) { - path += sWithRegion(prevData, data, j, diff); + for (let j = diff; j <= 1; j += diffx2) { + path += sWithRegion(prevData, data, j, diff); - // to make sure correct line drawing - if (j + diffx2 >= 1) { - path += sWithRegion(prevData, data, 1, 0); + // to make sure correct line drawing + if (j + diffx2 >= 1) { + path += sWithRegion(prevData, data, 1, 0); + } + } + } else { + let points = []; + isLastX = data.x === d[d.length - 1].x; + + if (isTimeSeries) { + const x0 = +prevData.x; + const xv0 = new Date(x0); + const xv1 = new Date(x0 + (+data.x - x0)); + + points = [ + [x(xv0), y(yp(0))], // M + [x(xv1), y(yp(1))] // L + ]; + } else { + points = [ + [x(xp(0)), y(yp(0))], // M + [x(xp(1)), y(yp(1))] // L + ]; } + + isRotated && points.forEach(v => v.reverse()); + + const startLength = getLength(tempNode, path); + const endLength = getLength(tempNode, path += `L${points[1].join(",")}`); + + const strokeDashArray = getStrokeDashArray( + startLength - dashArray.lastLength, + endLength - dashArray.lastLength, + style, + isLastX + ); + + dashArray.lastLength += strokeDashArray.length; + dashArray.dash.push(strokeDashArray.dash); } } } + if (dashArray.dash.length) { + // if not last x tick, then should draw rest of path that is not drawed yet + !isLastX && dashArray.dash.push(getLength(tempNode, path)); + + tempNode.remove(); + target.attr("stroke-dasharray", dashArray.dash.join(" ")); + } + return path; }, diff --git a/src/config/Options/data/axis.ts b/src/config/Options/data/axis.ts index b4fef08c2..5f2a4286b 100644 --- a/src/config/Options/data/axis.ts +++ b/src/config/Options/data/axis.ts @@ -108,8 +108,12 @@ export default { * - The object type should be as: * - start {number}: Start data point number. If not set, the start will be the first data point. * - [end] {number}: End data point number. If not set, the end will be the last data point. - * - [style.dasharray="2 2"] {object}: The first number specifies a distance for the filled area, and the second a distance for the unfilled area. - * - **NOTE:** Currently this option supports only line chart and dashed style. If this option specified, the line will be dashed only in the regions. + * - [style.dasharray="2 2"] {string}: The first number specifies a distance for the filled area, and the second a distance for the unfilled area. + * - **NOTE:** + * - Supports only line type. + * - `start` and `end` values should be in the exact x value range. + * - Dashes will be applied using `stroke-dasharray` css property when data doesn't contain nullish value(or nullish value with `line.connectNull=true` set). + * - Dashes will be applied via path command when data contains nullish value. * @name data․regions * @memberof Options * @type {object} diff --git a/test/internals/data-spec.ts b/test/internals/data-spec.ts index 119ec8ed8..911b16ce0 100644 --- a/test/internals/data-spec.ts +++ b/test/internals/data-spec.ts @@ -759,19 +759,19 @@ describe("DATA", () => { args = { data: { columns: [ - ["data1", 30, 200, 200, 400, 150, 250] + ["data1", null, 100, 200, null, 100, 20, 30, null, null] ], regions: { data1: [ { - start: 1, + start: 0, end: 2, style: { dasharray: "5 3" } }, { - start: 3 + start: 5 } ] } @@ -787,33 +787,25 @@ describe("DATA", () => { expect(path.split("L").length).to.be.equal(expected.L); } - it("should be generating correct dashed path data", () => { - checkPathLengths({M: 118, L: 119}); - }); + const checkStyleLengths = (dataId, closeTo = 1) => { + const {line: {lines}} = chart.$; + const target = lines.filter(({id}) => id === dataId); + const strokeDashArray = target.attr("stroke-dasharray"); + const dashLength = strokeDashArray.split(" ").map(Number).reduce((a, c) => a + c); + const totalLength = target.node().getTotalLength(); + const lastDashLength = +strokeDashArray.match(/\d+(\.\d+)?$/)[0]; + const path = target.attr("d"); - it("set options for null data", () => { - args = { - data: { - columns: [ - ["data1", null, 100, 200, null, 100, 20, 30, null, null] - ], - regions: { - data1: [ - { - start: 0, - end: 2, - style: { - dasharray: "5 3" - } - }, - { - start: 5 - } - ] - } - } - }; - }); + expect(path.split("M").length).to.be.equal(2); + expect(path.split("L").length).to.be.equal(chart.data.values(dataId).filter(Boolean).length); + + + if(lastDashLength === totalLength) { + expect(dashLength).to.be.greaterThan(totalLength); + } else { + expect(totalLength).to.be.closeTo(dashLength, closeTo); + } + }; it("should be generating correct dashed path data", () => { checkPathLengths({M: 38, L: 37}); @@ -823,11 +815,11 @@ describe("DATA", () => { args.line = {connectNull: true}; }); - it("should be generating correct dashed path data", () => { - checkPathLengths({M: 37, L: 38}); + it("should be generating correct dashed style", () => { + checkStyleLengths("data1"); }); - it("set options", () => { + it("set options: category type axis", () => { args = { data: { columns: [ @@ -850,17 +842,81 @@ describe("DATA", () => { }); it("check regions for 'category' type axis", () => { - const d = chart.$.line.lines.attr("d").split("M").filter(Boolean); + checkStyleLengths("data1"); + }); - expect(d.length).to.be.greaterThan(50); + it("set options: axis.rotated=true", () => { + args.axis.rotated = true; + }); - // check x coordinate starts - d.slice(0,4).forEach(v => { - const x = parseInt(v, 10); + it("check regions for 'category' type axis on roated axis", () => { + checkStyleLengths("data1"); + }); - // should be between 70 ~ 80 - expect(x > 70 && x < 80).to.be.true; - }); + it("set options: timeseries type axis", () => { + args = { + data: { + x: "x", + columns: [ + ["x", "2023-08-25", "2023-08-26", "2023-08-27", "2023-08-29", , "2023-09-02"], + ["data1", 50, 20, 10, 30, 50], + ["data2", 23, 50, 30, 70, 25] + ], + regions: { + data1: [ + { + end: "2023-08-27", + style: { + dasharray: "5 2" + } + }, + { + start: "2023-08-29", + style: { + dasharray: "10 10" + } + }, + ], + data2: [ + { + end: "2023-08-26", + style: { + dasharray: "1 3" + } + }, + { + start: "2023-08-27", + end: "2023-08-29", + style: { + dasharray: "8 8" + } + } + ] + } + }, + axis: { + x: { + type: "timeseries", + tick: { + format: "%Y-%m-%d", + } + } + } + }; + }); + + it("check regions for 'timeseries' type axis", () => { + checkStyleLengths("data1", 4); + checkStyleLengths("data2"); + }); + + it("set options: axis.rotated=true", () => { + args.axis.rotated = true; + }); + + it("check regions for 'timeseries' type axis on roated axis", () => { + checkStyleLengths("data1", 4); + checkStyleLengths("data2"); }); }); diff --git a/test/internals/rergions-spec.ts b/test/internals/rergions-spec.ts index bb28c8396..ea2ef2ad7 100644 --- a/test/internals/rergions-spec.ts +++ b/test/internals/rergions-spec.ts @@ -186,8 +186,8 @@ describe("REGIONS", function() { data: { x: "x", columns: [ - ["x", "2023-08-25", "2023-08-26", "2023-08-27"], - ["data1", 50, 20, 10] + ["x", "2023-08-25", "2023-08-26", "2023-08-27", "2023-08-28", "2023-08-29"], + ["data1", 50, 20, 10, null, 20, 30] ], regions: { data1: [{ @@ -213,14 +213,14 @@ describe("REGIONS", function() { it("should regions applied for timeseries chart.", () => { const lCnt = chart.$.line.lines.attr("d").split("L").length; - expect(lCnt).to.be.above(30); + expect(lCnt).to.be.above(19); }); it("set options", () => { args = { data: { columns: [ - ["data1", 30, 200, 100, 400 , 150, 250, 30] + ["data1", null, 200, 100, 152, 150, 250, 30] ], type: "line", regions: {