Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(bar): fix representation of radius for small data #3904

Merged
merged 1 commit into from
Oct 22, 2024
+162 −27
Merged

fix(bar): fix representation of radius for small data #3904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/ChartInternal/data/IData.ts
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading