Skip to content

Commit

Permalink
fix(option): Fix inconsistency of padding
Browse files Browse the repository at this point in the history
- Refactor getting padding values by direction
- Fix padding.bottom to follow from as other directions

Fix #3426
  • Loading branch information
netil authored and netil committed Sep 22, 2023
1 parent b91fd45 commit 694aa34
Show file tree
Hide file tree
Showing 15 changed files with 162 additions and 167 deletions.
4 changes: 2 additions & 2 deletions src/ChartInternal/Axis/Axis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ class Axis {

if (v === "x") {
res = clip.pathXAxis;
} else if (v === "y") { // && config.axis_y_inner) {
} else if (v === "y") { // || v === "y2") {
res = clip.pathYAxis;
}

Expand Down Expand Up @@ -670,7 +670,7 @@ class Axis {
!config.axis_x_tick_multiline &&
positiveRotation
) {
const widthWithoutCurrentPaddingLeft = state.current.width - $$.getCurrentPaddingLeft();
const widthWithoutCurrentPaddingLeft = state.current.width - $$.getCurrentPaddingByDirection("left");
const maxOverflow = this.getXAxisTickMaxOverflow(
xAxisTickRotate, widthWithoutCurrentPaddingLeft - defaultPadding
);
Expand Down
6 changes: 4 additions & 2 deletions src/ChartInternal/internals/clip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,24 +61,26 @@ export default {
const h = (isRotated ? (margin.top + height) + 10 : margin.bottom) + 20;

const x = isRotated ? -(1 + left) : -(left - 1);
const y = -15; // -Math.max(15, margin.top);
const w = isRotated ? margin.left + 20 : width + 10 + left;

node
.attr("x", x)
.attr("y", -2)
.attr("y", y)
.attr("width", w)
.attr("height", h);
},

/**
* Set y Axis clipPath dimension
* @param {d3Selecton} node clipPath <rect> selection
* @param {d3Selection} node clipPath <rect> selection
* @private
*/
setYAxisClipPath(node): void {
const $$ = this;
const {config, state: {margin, width, height}} = $$;
const isRotated = config.axis_rotated;

const left = Math.max(30, margin.left) - (isRotated ? 20 : 0);
const isInner = config.axis_y_inner;

Expand Down
8 changes: 4 additions & 4 deletions src/ChartInternal/internals/legend.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,11 @@ export default {

const insetLegendPosition = {
top: isLegendTop ?
$$.getCurrentPaddingTop() + config.legend_inset_y + 5.5 :
current.height - height - $$.getCurrentPaddingBottom() - config.legend_inset_y,
$$.getCurrentPaddingByDirection("top") + config.legend_inset_y + 5.5 :
current.height - height - $$.getCurrentPaddingByDirection("bottom") - config.legend_inset_y,
left: isLegendLeft ?
$$.getCurrentPaddingLeft() + config.legend_inset_x + 0.5 :
current.width - width - $$.getCurrentPaddingRight() - config.legend_inset_x + 0.5
$$.getCurrentPaddingByDirection("left") + config.legend_inset_x + 0.5 :
current.width - width - $$.getCurrentPaddingByDirection("right") - config.legend_inset_x + 0.5
};

$$.state.margin3 = {
Expand Down
2 changes: 1 addition & 1 deletion src/ChartInternal/internals/size.axis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ export default {
const $$ = this;
const {state: {axis, current}} = $$;
const xAxisLength = current.width -
$$.getCurrentPaddingLeft(false) - $$.getCurrentPaddingRight();
$$.getCurrentPaddingByDirection("left") - $$.getCurrentPaddingByDirection("right");
const tickCountWithPadding = axis.x.tickCount +
axis.x.padding.left + axis.x.padding.right;

Expand Down
212 changes: 91 additions & 121 deletions src/ChartInternal/internals/size.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/
import {document} from "../../module/browser";
import {$AXIS, $SUBCHART} from "../../config/classes";
import {isValue, ceil10, capitalize, isNumber, isEmpty} from "../../module/util";
import {ceil10, capitalize, isNumber, isEmpty, isUndefined} from "../../module/util";

export default {
/**
Expand Down Expand Up @@ -33,110 +33,6 @@ export default {
return h > 0 ? h : 320 / ($$.hasType("gauge") && !config.gauge_fullCircle ? 2 : 1);
},

getCurrentPaddingTop(): number {
const $$ = this;
const {config, state: {hasAxis}, $el} = $$;
const axesLen = hasAxis ? config.axis_y2_axes.length : 0;

let padding = isValue(config.padding_top) ?
config.padding_top : 0;

if ($el.title && $el.title.node()) {
padding += $$.getTitlePadding();
}

if (axesLen && config.axis_rotated) {
padding += $$.getHorizontalAxisHeight("y2") * axesLen;
}

return padding;
},

getCurrentPaddingBottom(): number {
const $$ = this;
const {config, state: {hasAxis}} = $$;
const axisId = config.axis_rotated ? "y" : "x";
const axesLen = hasAxis ? config[`axis_${axisId}_axes`].length : 0;
const padding = isValue(config.padding_bottom) ?
config.padding_bottom : 0;

return padding + (
axesLen ? $$.getHorizontalAxisHeight(axisId) * axesLen : 0
);
},

getCurrentPaddingLeft(withoutRecompute?: boolean): number {
const $$ = this;
const {config, state: {hasAxis}} = $$;
const isRotated = config.axis_rotated;
const isFitPadding = config.padding?.mode === "fit";
const axisId = isRotated ? "x" : "y";
const axesLen = hasAxis ? config[`axis_${axisId}_axes`].length : 0;
let axisWidth = hasAxis ? $$.getAxisWidthByAxisId(axisId, withoutRecompute) : 0;

if (!isFitPadding) {
axisWidth = ceil10(axisWidth);
}

let padding = config[`axis_${axisId}_inner`] || !config[`axis_${axisId}_show`] ? 0 : axisWidth;

if (isValue(config.padding_left)) {
padding = config.padding_left + (isFitPadding && isRotated ? axisWidth : 0);
} else if (hasAxis && isRotated) {
padding = !config.axis_x_show ?
1 : (isFitPadding ? axisWidth : Math.max(axisWidth, 40));
}

if (hasAxis && (isFitPadding || config[`axis_${axisId}_inner`]) && config[`axis_${axisId}_label`].text) {
padding += $$.axis.getAxisLabelPosition("y").isOuter ? 20 : 0;
}

return padding + (axisWidth * axesLen);
},

getCurrentPaddingRight(withXAxisTickTextOverflow = false): number {
const $$ = this;
const {config, state: {hasAxis}} = $$;
const isRotated = config.axis_rotated;
const isFitPadding = config.padding?.mode === "fit";
const defaultPadding = isFitPadding ? 2 : 10;
const legendWidthOnRight = $$.state.isLegendRight ? $$.getLegendWidth() + 20 : 0;
const axesLen = hasAxis ? config.axis_y2_axes.length : 0;
const axisLabelWidth = $$.axis?.getAxisLabelPosition("y2").isOuter ? 20 : 0;
const xAxisTickTextOverflow = withXAxisTickTextOverflow ?
$$.axis.getXAxisTickTextY2Overflow(defaultPadding) : 0;
let axisWidth = hasAxis && !config.axis_y2_inner ? $$.getAxisWidthByAxisId("y2") : 1;

if (!isFitPadding) {
axisWidth = ceil10(axisWidth);
}

let padding = isRotated ? 0 : Math.max(axisWidth + legendWidthOnRight, xAxisTickTextOverflow);

if (isValue(config.padding_right)) {
// padding = config.padding_right + (hasAxis ? 1 : 0); // 1 is needed not to hide tick line

padding = config.padding_right +
(isFitPadding && (isRotated || !config.axis_y2_show ? defaultPadding : padding)) +
(hasAxis && !isFitPadding ? 1 : 0); // 1 is needed not to hide tick line
} else if ($$.axis && isRotated) {
padding = defaultPadding + legendWidthOnRight;
} else if ($$.axis && (!config.axis_y2_show || config.axis_y2_inner)) {
padding = Math.max(
(isFitPadding && !config.axis_y2_show ? 2 : 1) + legendWidthOnRight + axisLabelWidth,
xAxisTickTextOverflow
);
}

if (hasAxis && !isRotated && isFitPadding &&
config.axis_y2_show && !config.axis_y2_inner && config.axis_y2_label.text
) {
padding += axisLabelWidth;
}

return padding + (axisWidth * axesLen);
},

/**
* Get the parent rect element's size
* @param {string} key property/attribute name
Expand Down Expand Up @@ -198,7 +94,7 @@ export default {
const chartRect = $el.chart.node().getBoundingClientRect();
const hasArc = $$.hasArcType();
const svgLeft = svgRect.right - chartRect.left -
(hasArc ? 0 : $$.getCurrentPaddingLeft(withoutRecompute));
(hasArc ? 0 : $$.getCurrentPaddingByDirection("left", withoutRecompute));

return svgLeft > 0 ? svgLeft : 0;
},
Expand Down Expand Up @@ -254,15 +150,85 @@ 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.
* @returns {number} padding value
* @private
*/
getCurrentPaddingByDirection(type: "top" | "bottom" | "left" | "right", withoutRecompute = false): number {
const $$ = this;
const {config, $el, state: {hasAxis}} = $$;
const isRotated = config.axis_rotated;
const isFitPadding = config.padding?.mode === "fit";
const paddingOption = isNumber(config[`padding_${type}`]) ? config[`padding_${type}`] : undefined;
const axisId = hasAxis ? {
top: isRotated ? "y2" : null,
bottom: isRotated ? "y" : "x",
left: isRotated ? "x" : "y",
right: isRotated ? null : "y2"
}[type] : null;

const isLeftRight = /^(left|right)$/.test(type);
const isAxisInner = axisId && config[`axis_${axisId}_inner`];
const isAxisShow = axisId && config[`axis_${axisId}_show`];
const axesLen = axisId ? config[`axis_${axisId}_axes`].length : 0;

let axisSize = axisId ? (
isLeftRight ?
$$.getAxisWidthByAxisId(axisId, withoutRecompute) :
$$.getHorizontalAxisHeight(axisId)
) : 0;
const defaultPadding = 20;
let gap = 0;

if (!isFitPadding && isLeftRight) {
axisSize = ceil10(axisSize);
}

let padding = hasAxis && isLeftRight && (
isAxisInner || (isUndefined(paddingOption) && !isAxisShow)
) ? 0 : (
isFitPadding ? (isAxisShow ? axisSize : 0) + (paddingOption ?? 0) : (
isUndefined(paddingOption) ? axisSize : paddingOption
)
);

if (isLeftRight && hasAxis) {
if (axisId && (isFitPadding || isAxisInner) && config[`axis_${axisId}_label`].text) {
padding += $$.axis.getAxisLabelPosition(axisId).isOuter ? defaultPadding : 0;
}

if (type === "right") {
padding += isRotated ? (
!isFitPadding && isUndefined(paddingOption) ? 10 : 2
) : !isAxisShow || isAxisInner ? (isFitPadding ? 2 : 1) : 0;
} else if (type === "left" && isRotated) {
padding = !config.axis_x_show ?
1 : (isFitPadding ? axisSize : Math.max(axisSize, 40));
}
} else {
if (type === "top") {
if ($el.title && $el.title.node()) {
padding += $$.getTitlePadding();
}

gap = isRotated && !isAxisInner ? axesLen : 0;
} else if (type === "bottom" && hasAxis && isRotated && !isAxisShow) {
padding += 1;
}
}

return padding + (axisSize * axesLen) - gap;
},

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

return {
top: $$.getCurrentPaddingTop(),
bottom: $$.getCurrentPaddingBottom(),
left: $$.getCurrentPaddingLeft(),
right: $$.getCurrentPaddingRight()
};
return {top, bottom, left, right};
},

/**
Expand Down Expand Up @@ -317,7 +283,11 @@ export default {
$$.updateXAxisTickClip();
}

const legendHeightForBottom = state.isLegendRight || state.isLegendInset ? 0 : currLegend.height;
const legendSize = {
right: config.legend_show && state.isLegendRight ? $$.getLegendWidth() + 20 : 0,
bottom: !config.legend_show || state.isLegendRight || state.isLegendInset ? 0 : currLegend.height
};

const xAxisHeight = isRotated || isNonAxis ? 0 : $$.getHorizontalAxisHeight("x");

const subchartXAxisHeight = config.subchart_axis_x_show && config.subchart_axis_x_tick_text_show ?
Expand All @@ -333,14 +303,14 @@ export default {

// for main
state.margin = !isNonAxis && isRotated ? {
top: $$.getHorizontalAxisHeight("y2") + padding.top,
right: isNonAxis ? 0 : $$.getCurrentPaddingRight(true),
bottom: $$.getHorizontalAxisHeight("y") + legendHeightForBottom + padding.bottom,
top: padding.top,
right: isNonAxis ? 0 : padding.right + legendSize.right,
bottom: legendSize.bottom + padding.bottom,
left: subchartHeight + (isNonAxis ? 0 : padding.left)
} : {
top: (isFitPadding ? 0 : 4) + padding.top, // for top tick text
right: isNonAxis ? 0 : $$.getCurrentPaddingRight(true),
bottom: gaugeHeight + xAxisHeight + subchartHeight + legendHeightForBottom + padding.bottom,
right: isNonAxis ? 0 : padding.right + legendSize.right,
bottom: gaugeHeight + subchartHeight + legendSize.bottom + padding.bottom,
left: isNonAxis ? 0 : padding.left
};

Expand All @@ -350,12 +320,12 @@ export default {
state.margin2 = isRotated ? {
top: state.margin.top,
right: NaN,
bottom: 20 + legendHeightForBottom,
bottom: 20 + legendSize.bottom,
left: $$.state.rotatedPadding.left
} : {
top: state.current.height - subchartHeight - legendHeightForBottom,
top: state.current.height - subchartHeight - legendSize.bottom,
right: NaN,
bottom: subchartXAxisHeight + legendHeightForBottom,
bottom: subchartXAxisHeight + legendSize.bottom,
left: state.margin.left
};

Expand Down
4 changes: 2 additions & 2 deletions src/ChartInternal/internals/tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,8 +311,8 @@ export default {
const hasTreemap = state.hasTreemap;
const isRotated = config.axis_rotated;
const svgLeft = $$.getSvgLeft(true);
let chartRight = svgLeft + current.width - $$.getCurrentPaddingRight();
const chartLeft = $$.getCurrentPaddingLeft(true);
let chartRight = svgLeft + current.width - $$.getCurrentPaddingByDirection("right");
const chartLeft = $$.getCurrentPaddingByDirection("left", true);
const size = 20;
let {x, y} = currPos;

Expand Down
2 changes: 1 addition & 1 deletion src/ChartInternal/internals/transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export default {
y = isRotated ? state.height + padding : 0;
} else if (target === "y2") {
x = isRotated ? 0 : state.width + padding;
y = isRotated && padding ? 1 - padding : 0;
y = isRotated ? 1 - padding : 0;
} else if (target === "subX") {
x = 0;
y = isRotated ? 0 : state.height2;
Expand Down
16 changes: 10 additions & 6 deletions src/Plugin/stanford/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,13 +121,15 @@ export default class Stanford extends Plugin {
$$.labelishData = d => d.values;
$$.opacityForCircle = () => 1;

const getCurrentPaddingRight = $$.getCurrentPaddingRight.bind($$);
const getCurrentPadding = $$.getCurrentPadding.bind($$);

$$.getCurrentPaddingRight = () => (
getCurrentPaddingRight() + (
this.colorScale ? this.colorScale.getColorScalePadding() : 0
)
);
$$.getCurrentPadding = () => {
const padding = getCurrentPadding();

padding.right += this.colorScale ? this.colorScale.getColorScalePadding() : 0;

return padding;
};
}

$init(): void {
Expand All @@ -144,6 +146,8 @@ export default class Stanford extends Plugin {
this.setStanfordTooltip();
this.colorScale.drawColorScale();

$$.right += this.colorScale ? this.colorScale.getColorScalePadding() : 0;

this.$redraw();
}

Expand Down
Loading

0 comments on commit 694aa34

Please sign in to comment.