Skip to content

Commit

Permalink
fix(bar): fix representation of radius for small data
Browse files Browse the repository at this point in the history 
clip surpassing shape area to mitigate visualization of value

Ref #3903
  • Loading branch information
netil committed Oct 22, 2024
1 parent 0060786 commit 609fe84
Show file tree
Hide file tree
Showing 3 changed files with 162 additions and 27 deletions.
4 changes: 4 additions & 0 deletions src/ChartInternal/data/IData.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
65 changes: 49 additions & 16 deletions src/ChartInternal/shape/bar.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<number | number[]>;
Expand Down Expand Up @@ -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);

Expand All @@ -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)
];
},
Expand All @@ -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);
Expand All @@ -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);

Expand All @@ -179,18 +180,24 @@ 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;
const barW = points[2][index] - points[0][index];

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}${
Expand All @@ -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`;
};
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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
Expand Down
120 changes: 109 additions & 11 deletions test/shape/bar-spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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"
]);
});

Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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() {
Expand All @@ -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() {
Expand All @@ -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 = {
Expand Down

0 comments on commit 609fe84

Please sign in to comment.