Skip to content

Commit

Permalink
fix(axis): Fix x axis autorotate option applies
Browse files Browse the repository at this point in the history
Fix side-effect of #3433, where removed x axis tick overflow
computation

Fix #3499
  • Loading branch information
netil authored Nov 6, 2023
1 parent a8ebdef commit e45eaf7
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 33 deletions.
1 change: 1 addition & 0 deletions demo/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
<script src="https://unpkg.com/@stackblitz/sdk/bundles/sdk.umd.js"></script>
<script>
var path = ["../dist/", "../../gh-pages/release/latest/dist/"];
// var path = ["https://naver.github.io/billboard.js/release/3.9.0/dist/"];

!function() {
var cssFileName = localStorage.cssThemeName;
Expand Down
11 changes: 7 additions & 4 deletions src/ChartInternal/Axis/Axis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -587,12 +587,13 @@ class Axis {
const $$ = this.owner;
const {config, state: {current}, $el: {svg, chart}} = $$;
const currentTickMax = current.maxTickSize[id];
const configPrefix = `axis_${id}`;
const max = {
width: 0,
height: 0
};

if (withoutRecompute || !config[`axis_${id}_show`] || (
if (withoutRecompute || !config[`${configPrefix}_show`] || (
currentTickMax.width > 0 && $$.filterTargetsToShow().length === 0
)) {
return currentTickMax;
Expand Down Expand Up @@ -624,8 +625,9 @@ class Axis {
}

const axis = this.getAxis(id, scale, false, false, true);
const tickCount = config[`axis_${id}_tick_count`];
const tickValues = config[`axis_${id}_tick_values`];
const tickRotate = config[`${configPrefix}_tick_rotate`];
const tickCount = config[`${configPrefix}_tick_count`];
const tickValues = config[`${configPrefix}_tick_values`];

// Make to generate the final tick text to be rendered
// https://github.com/naver/billboard.js/issues/920
Expand All @@ -651,6 +653,7 @@ class Axis {
axis.create(dummy);

dummy.selectAll("text")
.attr("transform", isNumber(tickRotate) ? `rotate(${tickRotate})` : null)
.each(function(d, i) {
const {width, height} = this.getBoundingClientRect();

Expand Down Expand Up @@ -683,7 +686,7 @@ class Axis {

if ((axis.isCategorized() || axis.isTimeSeries()) &&
config.axis_x_tick_fit &&
!config.axis_x_tick_culling &&
(!config.axis_x_tick_culling || isEmpty(config.axis_x_tick_culling)) &&
!config.axis_x_tick_multiline &&
positiveRotation
) {
Expand Down
21 changes: 4 additions & 17 deletions src/ChartInternal/internals/size.axis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ export default {
getHorizontalAxisHeight(id: AxisType): number {
const $$ = this;
const {config, state} = $$;
const {current, rotatedPadding, isLegendRight, isLegendInset} = state;
const {rotatedPadding, isLegendRight, isLegendInset} = state;
const isRotated = config.axis_rotated;
const isFitPadding = config.padding?.mode === "fit";
const isInner = config[`axis_${id}_inner`];
Expand Down Expand Up @@ -70,21 +70,8 @@ export default {
}

const maxtickSize = $$.axis.getMaxTickSize(id);
const rotate = $$.getAxisTickRotate(id);

// Calculate x/y axis height when tick rotated
if (
((id === "x" && !isRotated) || (/y2?/.test(id) && isRotated)) && rotate
) {
h += maxtickSize.width *
Math.cos(Math.PI * (90 - Math.abs(rotate)) / 180);

if (!config.axis_x_tick_multiline && current.height) {
if (h > current.height / 2) {
h = current.height / 2;
}
}
} else if (maxtickSize.height > defaultHeight && config.legend_show) {

if (maxtickSize.height > defaultHeight) {
h += maxtickSize.height - defaultHeight;
}

Expand Down Expand Up @@ -140,10 +127,10 @@ export default {
}

if ($el.svg &&
config.axis_x_tick_autorotate &&
config.axis_x_tick_fit &&
!config.axis_x_tick_multiline &&
!config.axis_x_tick_culling &&
config.axis_x_tick_autorotate &&
allowedXAxisTypes
) {
rotate = $$.needToRotateXAxisTickTexts() ?
Expand Down
13 changes: 9 additions & 4 deletions src/ChartInternal/internals/size.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,10 +169,11 @@ export default {
* Get padding by the direction.
* @param {string} type "top" | "bottom" | "left" | "right"
* @param {boolean} [withoutRecompute=false] If set true, do not recompute the padding value.
* @param {boolean} [withXAxisTickTextOverflow=false] If set true, calculate x axis tick text overflow.
* @returns {number} padding value
* @private
*/
getCurrentPaddingByDirection(type: "top" | "bottom" | "left" | "right", withoutRecompute = false): number {
getCurrentPaddingByDirection(type: "top" | "bottom" | "left" | "right", withoutRecompute = false, withXAxisTickTextOverflow = false): number {
const $$ = this;
const {config, $el, state: {hasAxis}} = $$;
const isRotated = config.axis_rotated;
Expand Down Expand Up @@ -219,6 +220,8 @@ export default {
padding += isRotated ? (
!isFitPadding && isUndefined(paddingOption) ? 10 : 2
) : !isAxisShow || isAxisInner ? (isFitPadding ? 2 : 1) : 0;

padding += withXAxisTickTextOverflow ? $$.axis.getXAxisTickTextY2Overflow(defaultPadding) : 0;
} else if (type === "left" && isRotated && isUndefined(paddingOption)) {
padding = !config.axis_x_show ?
1 : (isFitPadding ? axisSize : Math.max(axisSize, 40));
Expand All @@ -238,10 +241,12 @@ export default {
return padding + (axisSize * axesLen) - gap;
},

getCurrentPadding(): {top: number, bottom: number, left: number, right: number} {
getCurrentPadding(withXAxisTickTextOverflow = false): {
top: number, bottom: number, left: number, right: number
} {
const $$ = this;
const [top, bottom, left, right] = ["top", "bottom", "left", "right"]
.map(v => $$.getCurrentPaddingByDirection(v));
.map(v => $$.getCurrentPaddingByDirection(v, null, withXAxisTickTextOverflow));

return {top, bottom, left, right};
},
Expand Down Expand Up @@ -314,7 +319,7 @@ export default {
const gaugeHeight = $$.hasType("gauge") && config.arc_needle_show &&
!config.gauge_fullCircle && !config.gauge_label_show ? 10 : 0;

const padding = $$.getCurrentPadding();
const padding = $$.getCurrentPadding(true);

// for main
state.margin = !isNonAxis && isRotated ? {
Expand Down
16 changes: 8 additions & 8 deletions test/internals/axis-spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1105,7 +1105,7 @@ describe("AXIS", function() {
const {$el: {axis: {x}}, state} = chart.internal;
const xBottom = x.node().getBoundingClientRect().bottom;

expect(xBottom).to.be.closeTo(state.current.height, 5);
expect(xBottom).to.be.below(state.current.height);
});

it("set option: legend.show=false", () => {
Expand Down Expand Up @@ -1227,7 +1227,7 @@ describe("AXIS", function() {
const height = internal.getHorizontalAxisHeight("x");

expect(box.height).to.be.above(50);
expect(height).to.be.above(68);
expect(height).to.be.above(67);
expect(height).to.be.below(80);
});
});
Expand Down Expand Up @@ -1272,7 +1272,7 @@ describe("AXIS", function() {
const height = internal.getHorizontalAxisHeight("x");

expect(box.height).to.be.above(50);
expect(height).to.be.above(68);
expect(height).to.be.above(67);
expect(height).to.be.below(80);
});
});
Expand All @@ -1289,7 +1289,7 @@ describe("AXIS", function() {

expect(xAxisTickRotate).to.be.equal(expectedXAxisTickRotate);
expect(xAxisBoundingClientRect.height).to.be.closeTo(expectedXAxisBoundingClientRect, 1);
expect(horizontalXAxisHeight).to.be.closeTo(expectedHorizontalXAxisHeight, 1);
expect(horizontalXAxisHeight).to.be.closeTo(expectedHorizontalXAxisHeight, 2);

const xAxisTickTextY2Overflow = chart.internal.axis.getXAxisTickTextY2Overflow(defaultPadding);

Expand Down Expand Up @@ -1349,7 +1349,7 @@ describe("AXIS", function() {
expect(tspan.attr("dx")).to.be.equal("0");
});

compare(0, 18.8125, 30, 0);
compare(0, 18.8125, 48, 0);
});

it("update args", () => {
Expand Down Expand Up @@ -1382,7 +1382,7 @@ describe("AXIS", function() {
it("should not resize x axis when all data hidden", () => {
chart.hide("data1");

compare(args.axis.x.tick.rotate, 6, 57, 71);
compare(args.axis.x.tick.rotate, 6, 55, 71);

chart.show("data1");
});
Expand Down Expand Up @@ -1503,7 +1503,7 @@ describe("AXIS", function() {
expect(tspan.attr("dx")).to.be.equal("0");
});

compare(0, 18.8125, 30, 0);
compare(0, 18.8125, 55, 0);
});

it("update args", () => {
Expand All @@ -1527,7 +1527,7 @@ describe("AXIS", function() {
it("should not resize x axis when all data hidden", () => {
chart.hide("Temperature");

compare(args.axis.x.tick.rotate, 6, 57, 108);
compare(args.axis.x.tick.rotate, 6, 55, 108);
});

it("should resize when show hidden data", () => {
Expand Down

0 comments on commit e45eaf7

Please sign in to comment.