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.

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(axis): Fix x axis autorotate option applies #3508

Merged
merged 1 commit into from
Nov 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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