From 609fe844a801dc8109dcf8a3ffc6f4a5019ff02a Mon Sep 17 00:00:00 2001 From: netil Date: Tue, 22 Oct 2024 17:30:38 +0900 Subject: [PATCH] fix(bar): fix representation of radius for small data clip surpassing shape area to mitigate visualization of value Ref #3903 --- src/ChartInternal/data/IData.ts | 4 ++ src/ChartInternal/shape/bar.ts | 65 ++++++++++++----- test/shape/bar-spec.ts | 120 +++++++++++++++++++++++++++++--- 3 files changed, 162 insertions(+), 27 deletions(-) diff --git a/src/ChartInternal/data/IData.ts b/src/ChartInternal/data/IData.ts index ba173a51e..b84ea459a 100644 --- a/src/ChartInternal/data/IData.ts +++ b/src/ChartInternal/data/IData.ts @@ -50,6 +50,10 @@ export interface IArcData { value: number | null; } +export interface IBarData extends IDataRow { + clipPath?: string | null; +} + export interface IGridData { axis?: string; text: string; diff --git a/src/ChartInternal/shape/bar.ts b/src/ChartInternal/shape/bar.ts index 634ff84a2..60aaba1e6 100644 --- a/src/ChartInternal/shape/bar.ts +++ b/src/ChartInternal/shape/bar.ts @@ -5,7 +5,7 @@ import type {DataRow} from "../../../types/types"; import {$BAR, $COMMON} from "../../config/classes"; import {getRandom, isNumber} from "../../module/util"; -import type {IDataRow} from "../data/IData"; +import type {IBarData} from "../data/IData"; import type {IOffset} from "./shape"; type BarTypeDataRow = DataRow; @@ -106,7 +106,7 @@ export default { * @returns {string} Color string * @private */ - updateBarColor(d: IDataRow): string { + updateBarColor(d: IBarData): string { const $$ = this; const fn = $$.getStylePropValue($$.color); @@ -129,6 +129,7 @@ export default { $$.$T(bar, withTransition, getRandom()) .attr("d", d => (isNumber(d.value) || $$.isBarRangeType(d)) && drawFn(d)) .style("fill", $$.updateBarColor.bind($$)) + .style("clip-path", d => d.clipPath) .style("opacity", null) ]; }, @@ -153,7 +154,7 @@ export default { * @returns {Function} * @private */ - generateDrawBar(barIndices, isSub?: boolean): (d: IDataRow, i: number) => string { + generateDrawBar(barIndices, isSub?: boolean): (d: IBarData, i: number) => string { const $$ = this; const {config} = $$; const getPoints = $$.generateGetBarPoints(barIndices, isSub); @@ -166,7 +167,7 @@ export default { isNumber(barRadiusRatio) ? w => w * barRadiusRatio : null ); - return (d: IDataRow, i: number) => { + return (d: IBarData, i: number) => { // 4 points that make a bar const points = getPoints(d, i); @@ -179,10 +180,16 @@ export default { const isNegative = (!isInverted && isUnderZero) || (isInverted && !isUnderZero); const pathRadius = ["", ""]; - let radius = 0; - const isGrouped = $$.isGrouped(d.id); const isRadiusData = getRadius && isGrouped ? $$.isStackingRadiusData(d) : false; + const init = [ + points[0][indexX], + points[0][indexY] + ]; + let radius = 0; + + // initialize as null to not set attribute if isn't needed + d.clipPath = null; if (getRadius) { const index = isRotated ? indexY : indexX; @@ -190,7 +197,7 @@ export default { radius = !isGrouped || isRadiusData ? getRadius(barW) : 0; - const arc = `a${radius},${radius} ${isNegative ? `1 0 0` : `0 0 1`} `; + const arc = `a${radius} ${radius} ${isNegative ? `1 0 0` : `0 0 1`} `; pathRadius[+!isRotated] = `${arc}${radius},${radius}`; pathRadius[+isRotated] = `${arc}${ @@ -200,15 +207,41 @@ export default { isNegative && pathRadius.reverse(); } + const pos = isRotated ? + points[1][indexX] + (isNegative ? radius : -radius) : + points[1][indexY] + (isNegative ? -radius : radius); + + // Apply clip-path in case of radius angle surpass the bar shape + // https://github.com/naver/billboard.js/issues/3903 + if (radius) { + let clipPath = ""; + + if (isRotated) { + if (isNegative && init[0] < pos) { + clipPath = `0 ${pos - init[0]}px 0 0`; + } else if (!isNegative && init[0] > pos) { + clipPath = `0 0 0 ${init[0] - pos}px`; + } + } else { + if (isNegative && init[1] > pos) { + clipPath = `${init[1] - pos}px 0 0 0`; + } else if (!isNegative && init[1] < pos) { + clipPath = `0 0 ${pos - init[1]}px 0`; + } + } + + d.clipPath = `inset(${clipPath})`; + } + // path string data shouldn't be containing new line chars // https://github.com/naver/billboard.js/issues/530 const path = isRotated ? - `H${points[1][indexX] + (isNegative ? radius : -radius)} ${pathRadius[0]}V${ - points[2][indexY] - radius - } ${pathRadius[1]}H${points[3][indexX]}` : - `V${points[1][indexY] + (isNegative ? -radius : radius)} ${pathRadius[0]}H${ - points[2][indexX] - radius - } ${pathRadius[1]}V${points[3][indexY]}`; + `H${pos} ${pathRadius[0]}V${points[2][indexY] - radius} ${pathRadius[1]}H${ + points[3][indexX] + }` : + `V${pos} ${pathRadius[0]}H${points[2][indexX] - radius} ${pathRadius[1]}V${ + points[3][indexY] + }`; return `M${points[0][indexX]},${points[0][indexY]}${path}z`; }; @@ -219,7 +252,7 @@ export default { * @param {object} d Data row * @returns {boolean} */ - isStackingRadiusData(d: IDataRow): boolean { + isStackingRadiusData(d: IBarData): boolean { const $$ = this; const {$el, config, data, state} = $$; const {id, index, value} = d; @@ -264,7 +297,7 @@ export default { * @private */ generateGetBarPoints(barIndices, - isSub?: boolean): (d: IDataRow, i: number) => [number, number][] { + isSub?: boolean): (d: IBarData, i: number) => [number, number][] { const $$ = this; const {config} = $$; const axis = isSub ? $$.axis.subX : $$.axis.x; @@ -275,7 +308,7 @@ export default { const barOffset = $$.getShapeOffset($$.isBarType, barIndices, !!isSub); const yScale = $$.getYScaleById.bind($$); - return (d: IDataRow, i: number) => { + return (d: IBarData, i: number) => { const {id} = d; const y0 = yScale.call($$, id, isSub)($$.getShapeYMin(id)); const offset = barOffset(d, i) || y0; // offset is for stacked bar chart diff --git a/test/shape/bar-spec.ts b/test/shape/bar-spec.ts index bfc8adb88..66b8f683b 100644 --- a/test/shape/bar-spec.ts +++ b/test/shape/bar-spec.ts @@ -665,8 +665,8 @@ describe("SHAPE BAR", () => { it("check the bar radius", () => { const path = [ - "M228.91666666666669,331.55555555555554V380.5833333333333 a10,10 1 0 0 10,10H233.91666666666669 a10,10 1 0 0 10,-10V331.55555555555554z", - "M246.91666666666669,331.55555555555554V223.5 a10,10 0 0 1 10,-10H251.91666666666669 a10,10 0 0 1 10,10V331.55555555555554z" + "M228.91666666666669,331.55555555555554V380.5833333333333 a10 10 1 0 0 10,10H233.91666666666669 a10 10 1 0 0 10,-10V331.55555555555554z", + "M246.91666666666669,331.55555555555554V223.5 a10 10 0 0 1 10,-10H251.91666666666669 a10 10 0 0 1 10,10V331.55555555555554z" ]; checkRadius(path); @@ -678,8 +678,8 @@ describe("SHAPE BAR", () => { it("check the rotated axis bar radius", () => { checkRadius([ - "M131.11111111111111,161.16666666666669H59.166666666666664 a10,10 1 0 0 -10,10V166.16666666666669 a10,10 1 0 0 10,10H131.11111111111111z", - "M131.11111111111111,179.16666666666669H285 a10,10 0 0 1 10,10V184.16666666666669 a10,10 0 0 1 -10,10H131.11111111111111z" + "M131.11111111111111,161.16666666666669H59.166666666666664 a10 10 1 0 0 -10,10V166.16666666666669 a10 10 1 0 0 10,10H131.11111111111111z", + "M131.11111111111111,179.16666666666669H285 a10 10 0 0 1 10,10V184.16666666666669 a10 10 0 0 1 -10,10H131.11111111111111z" ]); }); @@ -690,8 +690,8 @@ describe("SHAPE BAR", () => { it("check the axis bar radius in ratio", () => { const path = [ - "M228.91666666666669,331.55555555555554V383.0833333333333 a7.5,7.5 1 0 0 7.5,7.5H236.41666666666669 a7.5,7.5 1 0 0 7.5,-7.5V331.55555555555554z", - "M246.91666666666669,331.55555555555554V221 a7.5,7.5 0 0 1 7.5,-7.5H254.41666666666669 a7.5,7.5 0 0 1 7.5,7.5V331.55555555555554z" + "M228.91666666666669,331.55555555555554V383.0833333333333 a7.5 7.5 1 0 0 7.5,7.5H236.41666666666669 a7.5 7.5 1 0 0 7.5,-7.5V331.55555555555554z", + "M246.91666666666669,331.55555555555554V221 a7.5 7.5 0 0 1 7.5,-7.5H254.41666666666669 a7.5 7.5 0 0 1 7.5,7.5V331.55555555555554z" ]; checkRadius(path); @@ -957,7 +957,7 @@ describe("SHAPE BAR", () => { ]; chart.$.bar.bars.each(function(d) { - const hasRadius = !/a0,0/.test(this.getAttribute("d")); + const hasRadius = !/a0 0/.test(this.getAttribute("d")); if (hasRadius) { const found = expected[d.index].some(v => v.id === d.id && v.value === d.value); @@ -1026,8 +1026,8 @@ describe("SHAPE BAR", () => { it("radius should be rendered correclty on rotated axis", () => { const expected = [ - 'M295,85.80000000000001H477.23333333333323 a63.599999999999994,63.599999999999994 0 0 1 63.599999999999994,63.599999999999994V149.4 a63.599999999999994,63.599999999999994 0 0 1 -63.599999999999994,63.599999999999994H295z', - 'M295,213H112.76666666666665 a63.599999999999994,63.599999999999994 1 0 0 -63.599999999999994,63.599999999999994V276.6 a63.599999999999994,63.599999999999994 1 0 0 63.599999999999994,63.599999999999994H295z' + 'M295,85.80000000000001H477.23333333333323 a63.599999999999994 63.599999999999994 0 0 1 63.599999999999994,63.599999999999994V149.4 a63.599999999999994 63.599999999999994 0 0 1 -63.599999999999994,63.599999999999994H295z', + 'M295,213H112.76666666666665 a63.599999999999994 63.599999999999994 1 0 0 -63.599999999999994,63.599999999999994V276.6 a63.599999999999994 63.599999999999994 1 0 0 63.599999999999994,63.599999999999994H295z' ]; chart.$.bar.bars.each(function() { @@ -1043,8 +1043,8 @@ describe("SHAPE BAR", () => { it("radius should be rendered correclty on rotated & inverted axis", () => { const expected = [ - 'M295,85.80000000000001H112.76666666666668 a63.599999999999994,63.599999999999994 1 0 0 -63.599999999999994,63.599999999999994V149.4 a63.599999999999994,63.599999999999994 1 0 0 63.599999999999994,63.599999999999994H295z', - 'M295,213H477.23333333333323 a63.599999999999994,63.599999999999994 0 0 1 63.599999999999994,63.599999999999994V276.6 a63.599999999999994,63.599999999999994 0 0 1 -63.599999999999994,63.599999999999994H295z' + 'M295,85.80000000000001H112.76666666666668 a63.599999999999994 63.599999999999994 1 0 0 -63.599999999999994,63.599999999999994V149.4 a63.599999999999994 63.599999999999994 1 0 0 63.599999999999994,63.599999999999994H295z', + 'M295,213H477.23333333333323 a63.599999999999994 63.599999999999994 0 0 1 63.599999999999994,63.599999999999994V276.6 a63.599999999999994 63.599999999999994 0 0 1 -63.599999999999994,63.599999999999994H295z' ]; chart.$.bar.bars.each(function() { @@ -1053,6 +1053,104 @@ describe("SHAPE BAR", () => { }); }); + describe("bar radius surpassing condition", () => { + beforeAll(() => { + args = { + data: { + type: "bar", + columns:[ + ["data", -10289158423, -204482173, 3075954443] + ] + }, + axis: { + y: { + "min":-50000000000, + "max":50000000000, + "tick":{ + "show":false, + "outer":false, + "text":{ + "position":{ + "x":-20 + } + }, + "stepSize": 25000000000 + },"padding":0 + } + }, + bar: { + radius: 2, + width: 10, + padding: 2 + } + }; + }); + + it("should negative value set clip-path", () => { + chart.$.bar.bars.each(function(d, i) { + if (i === 1) { + const d = this.getAttribute("d"); + const value = [d.match(/,([^V]*)/)[1], d.match(/V([^\s]*)/)[1]].reduce((a, c) => +a - +c); + + expect(+this.style.clipPath.match(/\(([^px]*)/)[1]).to.be.closeTo(value, 1); + } else { + expect(this.style.clipPath).to.be.equal(""); + } + }); + }); + + it("set options: set positive data value", () => { + args.data.columns[0][2] = 204482173; + }); + + it("should positive value set clip-path", () => { + chart.$.bar.bars.each(function(d, i) { + if (i === 1) { + const d = this.getAttribute("d"); + const value = [d.match(/,([^V]*)/)[1], d.match(/V([^\s]*)/)[1]].reduce((a, c) => +c - +a); + + expect(+this.style.clipPath.match(/\s([^px]*)px\)$/)[1]).to.be.closeTo(value, 1); + } else { + expect(this.style.clipPath).to.be.equal(""); + } + }); + }); + + it("set options: axis.rotated=true", () => { + args.axis.rotated = true; + }); + + it("should positive value set clip-path for rotated axis", () => { + chart.$.bar.bars.each(function(d, i) { + if (i === 1) { + const d = this.getAttribute("d"); + const value = [d.match(/M([^,]*)/)[1], d.match(/H([^\s]*)/)[1]].reduce((a, c) => +a - +c); + + expect(+this.style.clipPath.match(/\s([^px]*)px\)$/)[1]).to.be.closeTo(value, 1); + } else { + expect(this.style.clipPath).to.be.equal(""); + } + }); + }); + + it("set options: set negative data value", () => { + args.data.columns[0][2] = -204482173; + }); + + it("should negative value set clip-path for rotated axis", () => { + chart.$.bar.bars.each(function(d, i) { + if (i === 1) { + const d = this.getAttribute("d"); + const value = [d.match(/M([^,]*)/)[1], d.match(/H([^\s]*)/)[1]].reduce((a, c) => +c - +a); + + expect(+this.style.clipPath.match(/px\s([^px]*)/)[1]).to.be.closeTo(value, 1); + } else { + expect(this.style.clipPath).to.be.equal(""); + } + }); + }); + }); + describe("bar linear gradient", () => { beforeAll(() => { args = {