Skip to content

Commit 91b5dca

Browse files
netilnetil
andauthored
fix(bar): fix representation of radius for small data
clip surpassing shape area to mitigate visualization of value Fix #3903 Co-authored-by: netil <[email protected]>
1 parent 0060786 commit 91b5dca

File tree

3 files changed

+162
-27
lines changed

3 files changed

+162
-27
lines changed

src/ChartInternal/data/IData.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,10 @@ export interface IArcData {
5050
value: number | null;
5151
}
5252

53+
export interface IBarData extends IDataRow {
54+
clipPath?: string | null;
55+
}
56+
5357
export interface IGridData {
5458
axis?: string;
5559
text: string;

src/ChartInternal/shape/bar.ts

Lines changed: 49 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import type {DataRow} from "../../../types/types";
66
import {$BAR, $COMMON} from "../../config/classes";
77
import {getRandom, isNumber} from "../../module/util";
8-
import type {IDataRow} from "../data/IData";
8+
import type {IBarData} from "../data/IData";
99
import type {IOffset} from "./shape";
1010

1111
type BarTypeDataRow = DataRow<number | number[]>;
@@ -106,7 +106,7 @@ export default {
106106
* @returns {string} Color string
107107
* @private
108108
*/
109-
updateBarColor(d: IDataRow): string {
109+
updateBarColor(d: IBarData): string {
110110
const $$ = this;
111111
const fn = $$.getStylePropValue($$.color);
112112

@@ -129,6 +129,7 @@ export default {
129129
$$.$T(bar, withTransition, getRandom())
130130
.attr("d", d => (isNumber(d.value) || $$.isBarRangeType(d)) && drawFn(d))
131131
.style("fill", $$.updateBarColor.bind($$))
132+
.style("clip-path", d => d.clipPath)
132133
.style("opacity", null)
133134
];
134135
},
@@ -153,7 +154,7 @@ export default {
153154
* @returns {Function}
154155
* @private
155156
*/
156-
generateDrawBar(barIndices, isSub?: boolean): (d: IDataRow, i: number) => string {
157+
generateDrawBar(barIndices, isSub?: boolean): (d: IBarData, i: number) => string {
157158
const $$ = this;
158159
const {config} = $$;
159160
const getPoints = $$.generateGetBarPoints(barIndices, isSub);
@@ -166,7 +167,7 @@ export default {
166167
isNumber(barRadiusRatio) ? w => w * barRadiusRatio : null
167168
);
168169

169-
return (d: IDataRow, i: number) => {
170+
return (d: IBarData, i: number) => {
170171
// 4 points that make a bar
171172
const points = getPoints(d, i);
172173

@@ -179,18 +180,24 @@ export default {
179180
const isNegative = (!isInverted && isUnderZero) || (isInverted && !isUnderZero);
180181

181182
const pathRadius = ["", ""];
182-
let radius = 0;
183-
184183
const isGrouped = $$.isGrouped(d.id);
185184
const isRadiusData = getRadius && isGrouped ? $$.isStackingRadiusData(d) : false;
185+
const init = [
186+
points[0][indexX],
187+
points[0][indexY]
188+
];
189+
let radius = 0;
190+
191+
// initialize as null to not set attribute if isn't needed
192+
d.clipPath = null;
186193

187194
if (getRadius) {
188195
const index = isRotated ? indexY : indexX;
189196
const barW = points[2][index] - points[0][index];
190197

191198
radius = !isGrouped || isRadiusData ? getRadius(barW) : 0;
192199

193-
const arc = `a${radius},${radius} ${isNegative ? `1 0 0` : `0 0 1`} `;
200+
const arc = `a${radius} ${radius} ${isNegative ? `1 0 0` : `0 0 1`} `;
194201

195202
pathRadius[+!isRotated] = `${arc}${radius},${radius}`;
196203
pathRadius[+isRotated] = `${arc}${
@@ -200,15 +207,41 @@ export default {
200207
isNegative && pathRadius.reverse();
201208
}
202209

210+
const pos = isRotated ?
211+
points[1][indexX] + (isNegative ? radius : -radius) :
212+
points[1][indexY] + (isNegative ? -radius : radius);
213+
214+
// Apply clip-path in case of radius angle surpass the bar shape
215+
// https://github.com/naver/billboard.js/issues/3903
216+
if (radius) {
217+
let clipPath = "";
218+
219+
if (isRotated) {
220+
if (isNegative && init[0] < pos) {
221+
clipPath = `0 ${pos - init[0]}px 0 0`;
222+
} else if (!isNegative && init[0] > pos) {
223+
clipPath = `0 0 0 ${init[0] - pos}px`;
224+
}
225+
} else {
226+
if (isNegative && init[1] > pos) {
227+
clipPath = `${init[1] - pos}px 0 0 0`;
228+
} else if (!isNegative && init[1] < pos) {
229+
clipPath = `0 0 ${pos - init[1]}px 0`;
230+
}
231+
}
232+
233+
d.clipPath = `inset(${clipPath})`;
234+
}
235+
203236
// path string data shouldn't be containing new line chars
204237
// https://github.com/naver/billboard.js/issues/530
205238
const path = isRotated ?
206-
`H${points[1][indexX] + (isNegative ? radius : -radius)} ${pathRadius[0]}V${
207-
points[2][indexY] - radius
208-
} ${pathRadius[1]}H${points[3][indexX]}` :
209-
`V${points[1][indexY] + (isNegative ? -radius : radius)} ${pathRadius[0]}H${
210-
points[2][indexX] - radius
211-
} ${pathRadius[1]}V${points[3][indexY]}`;
239+
`H${pos} ${pathRadius[0]}V${points[2][indexY] - radius} ${pathRadius[1]}H${
240+
points[3][indexX]
241+
}` :
242+
`V${pos} ${pathRadius[0]}H${points[2][indexX] - radius} ${pathRadius[1]}V${
243+
points[3][indexY]
244+
}`;
212245

213246
return `M${points[0][indexX]},${points[0][indexY]}${path}z`;
214247
};
@@ -219,7 +252,7 @@ export default {
219252
* @param {object} d Data row
220253
* @returns {boolean}
221254
*/
222-
isStackingRadiusData(d: IDataRow): boolean {
255+
isStackingRadiusData(d: IBarData): boolean {
223256
const $$ = this;
224257
const {$el, config, data, state} = $$;
225258
const {id, index, value} = d;
@@ -264,7 +297,7 @@ export default {
264297
* @private
265298
*/
266299
generateGetBarPoints(barIndices,
267-
isSub?: boolean): (d: IDataRow, i: number) => [number, number][] {
300+
isSub?: boolean): (d: IBarData, i: number) => [number, number][] {
268301
const $$ = this;
269302
const {config} = $$;
270303
const axis = isSub ? $$.axis.subX : $$.axis.x;
@@ -275,7 +308,7 @@ export default {
275308
const barOffset = $$.getShapeOffset($$.isBarType, barIndices, !!isSub);
276309
const yScale = $$.getYScaleById.bind($$);
277310

278-
return (d: IDataRow, i: number) => {
311+
return (d: IBarData, i: number) => {
279312
const {id} = d;
280313
const y0 = yScale.call($$, id, isSub)($$.getShapeYMin(id));
281314
const offset = barOffset(d, i) || y0; // offset is for stacked bar chart

test/shape/bar-spec.ts

Lines changed: 109 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -665,8 +665,8 @@ describe("SHAPE BAR", () => {
665665

666666
it("check the bar radius", () => {
667667
const path = [
668-
"M228.91666666666669,331.55555555555554V380.5833333333333 a10,10 1 0 0 10,10H233.91666666666669 a10,10 1 0 0 10,-10V331.55555555555554z",
669-
"M246.91666666666669,331.55555555555554V223.5 a10,10 0 0 1 10,-10H251.91666666666669 a10,10 0 0 1 10,10V331.55555555555554z"
668+
"M228.91666666666669,331.55555555555554V380.5833333333333 a10 10 1 0 0 10,10H233.91666666666669 a10 10 1 0 0 10,-10V331.55555555555554z",
669+
"M246.91666666666669,331.55555555555554V223.5 a10 10 0 0 1 10,-10H251.91666666666669 a10 10 0 0 1 10,10V331.55555555555554z"
670670
];
671671

672672
checkRadius(path);
@@ -678,8 +678,8 @@ describe("SHAPE BAR", () => {
678678

679679
it("check the rotated axis bar radius", () => {
680680
checkRadius([
681-
"M131.11111111111111,161.16666666666669H59.166666666666664 a10,10 1 0 0 -10,10V166.16666666666669 a10,10 1 0 0 10,10H131.11111111111111z",
682-
"M131.11111111111111,179.16666666666669H285 a10,10 0 0 1 10,10V184.16666666666669 a10,10 0 0 1 -10,10H131.11111111111111z"
681+
"M131.11111111111111,161.16666666666669H59.166666666666664 a10 10 1 0 0 -10,10V166.16666666666669 a10 10 1 0 0 10,10H131.11111111111111z",
682+
"M131.11111111111111,179.16666666666669H285 a10 10 0 0 1 10,10V184.16666666666669 a10 10 0 0 1 -10,10H131.11111111111111z"
683683
]);
684684
});
685685

@@ -690,8 +690,8 @@ describe("SHAPE BAR", () => {
690690

691691
it("check the axis bar radius in ratio", () => {
692692
const path = [
693-
"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",
694-
"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"
693+
"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",
694+
"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"
695695
];
696696

697697
checkRadius(path);
@@ -957,7 +957,7 @@ describe("SHAPE BAR", () => {
957957
];
958958

959959
chart.$.bar.bars.each(function(d) {
960-
const hasRadius = !/a0,0/.test(this.getAttribute("d"));
960+
const hasRadius = !/a0 0/.test(this.getAttribute("d"));
961961

962962
if (hasRadius) {
963963
const found = expected[d.index].some(v => v.id === d.id && v.value === d.value);
@@ -1026,8 +1026,8 @@ describe("SHAPE BAR", () => {
10261026

10271027
it("radius should be rendered correclty on rotated axis", () => {
10281028
const expected = [
1029-
'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',
1030-
'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'
1029+
'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',
1030+
'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'
10311031
];
10321032

10331033
chart.$.bar.bars.each(function() {
@@ -1043,8 +1043,8 @@ describe("SHAPE BAR", () => {
10431043

10441044
it("radius should be rendered correclty on rotated & inverted axis", () => {
10451045
const expected = [
1046-
'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',
1047-
'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'
1046+
'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',
1047+
'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'
10481048
];
10491049

10501050
chart.$.bar.bars.each(function() {
@@ -1053,6 +1053,104 @@ describe("SHAPE BAR", () => {
10531053
});
10541054
});
10551055

1056+
describe("bar radius surpassing condition", () => {
1057+
beforeAll(() => {
1058+
args = {
1059+
data: {
1060+
type: "bar",
1061+
columns:[
1062+
["data", -10289158423, -204482173, 3075954443]
1063+
]
1064+
},
1065+
axis: {
1066+
y: {
1067+
"min":-50000000000,
1068+
"max":50000000000,
1069+
"tick":{
1070+
"show":false,
1071+
"outer":false,
1072+
"text":{
1073+
"position":{
1074+
"x":-20
1075+
}
1076+
},
1077+
"stepSize": 25000000000
1078+
},"padding":0
1079+
}
1080+
},
1081+
bar: {
1082+
radius: 2,
1083+
width: 10,
1084+
padding: 2
1085+
}
1086+
};
1087+
});
1088+
1089+
it("should negative value set clip-path", () => {
1090+
chart.$.bar.bars.each(function(d, i) {
1091+
if (i === 1) {
1092+
const d = this.getAttribute("d");
1093+
const value = [d.match(/,([^V]*)/)[1], d.match(/V([^\s]*)/)[1]].reduce((a, c) => +a - +c);
1094+
1095+
expect(+this.style.clipPath.match(/\(([^px]*)/)[1]).to.be.closeTo(value, 1);
1096+
} else {
1097+
expect(this.style.clipPath).to.be.equal("");
1098+
}
1099+
});
1100+
});
1101+
1102+
it("set options: set positive data value", () => {
1103+
args.data.columns[0][2] = 204482173;
1104+
});
1105+
1106+
it("should positive value set clip-path", () => {
1107+
chart.$.bar.bars.each(function(d, i) {
1108+
if (i === 1) {
1109+
const d = this.getAttribute("d");
1110+
const value = [d.match(/,([^V]*)/)[1], d.match(/V([^\s]*)/)[1]].reduce((a, c) => +c - +a);
1111+
1112+
expect(+this.style.clipPath.match(/\s([^px]*)px\)$/)[1]).to.be.closeTo(value, 1);
1113+
} else {
1114+
expect(this.style.clipPath).to.be.equal("");
1115+
}
1116+
});
1117+
});
1118+
1119+
it("set options: axis.rotated=true", () => {
1120+
args.axis.rotated = true;
1121+
});
1122+
1123+
it("should positive value set clip-path for rotated axis", () => {
1124+
chart.$.bar.bars.each(function(d, i) {
1125+
if (i === 1) {
1126+
const d = this.getAttribute("d");
1127+
const value = [d.match(/M([^,]*)/)[1], d.match(/H([^\s]*)/)[1]].reduce((a, c) => +a - +c);
1128+
1129+
expect(+this.style.clipPath.match(/\s([^px]*)px\)$/)[1]).to.be.closeTo(value, 1);
1130+
} else {
1131+
expect(this.style.clipPath).to.be.equal("");
1132+
}
1133+
});
1134+
});
1135+
1136+
it("set options: set negative data value", () => {
1137+
args.data.columns[0][2] = -204482173;
1138+
});
1139+
1140+
it("should negative value set clip-path for rotated axis", () => {
1141+
chart.$.bar.bars.each(function(d, i) {
1142+
if (i === 1) {
1143+
const d = this.getAttribute("d");
1144+
const value = [d.match(/M([^,]*)/)[1], d.match(/H([^\s]*)/)[1]].reduce((a, c) => +c - +a);
1145+
1146+
expect(+this.style.clipPath.match(/px\s([^px]*)/)[1]).to.be.closeTo(value, 1);
1147+
} else {
1148+
expect(this.style.clipPath).to.be.equal("");
1149+
}
1150+
});
1151+
});
1152+
});
1153+
10561154
describe("bar linear gradient", () => {
10571155
beforeAll(() => {
10581156
args = {

0 commit comments

Comments
 (0)