Skip to content

Commit

Permalink
refactor(xy): axis title and related simplifications (#1324)
Browse files Browse the repository at this point in the history
* safer text measurement, no need for manual destroy, no need for class
* switched away from the svg measurer in tests and removed both DOM based measurers
* [].sort predicate compareByValueAsc equipped to deal with duplicates
* simplified and split getSimplePadding
* simpler computeAxesSizes, renderBars etc.
* removal of remaining shadowing (ctx) => cases
* unified horizontal and vertical titles, due to much shared code
* unified global and panel titles, due to much shared code
* removed parallel redundant switch/case structures etc. from values.bar.ts
* replaced two other versions of clamp with clamp
* removed the git commit and husky
* minor: forEach where a for is essentially that, etc.
  • Loading branch information
monfera authored Aug 25, 2021
1 parent 52e919e commit 4b43650
Show file tree
Hide file tree
Showing 37 changed files with 562 additions and 1,165 deletions.
5 changes: 0 additions & 5 deletions .huskyrc.js

This file was deleted.

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,6 @@
"eslint-plugin-react-hooks": "^4.2.0",
"eslint-plugin-unicorn": "^25.0.1",
"html-webpack-plugin": "^4.5.2",
"husky": "^4.3.6",
"jest": "^26.6.3",
"jest-canvas-mock": "^2.3.1",
"jest-extended": "^0.11.5",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { Box, TextMeasure } from '../../../../common/text_utils';
import { ScaleContinuous } from '../../../../scales';
import { ScaleType } from '../../../../scales/constants';
import { SettingsSpec } from '../../../../specs';
import { CanvasTextBBoxCalculator } from '../../../../utils/bbox/canvas_text_bbox_calculator';
import { withTextMeasure } from '../../../../utils/bbox/canvas_text_bbox_calculator';
import { snapDateToESInterval } from '../../../../utils/chrono/elasticsearch';
import { clamp, range } from '../../../../utils/common';
import { Dimensions } from '../../../../utils/dimensions';
Expand Down Expand Up @@ -56,21 +56,16 @@ function getValuesInRange(
/**
* Resolves the maximum number of ticks based on the chart width and sample label based on formatter config.
*/
function getTicks(chartWidth: number, xAxisLabelConfig: Config['xAxisLabel']): number {
const bboxCompute = new CanvasTextBBoxCalculator();
const labelSample = xAxisLabelConfig.formatter(Date.now());
const { width } = bboxCompute.compute(
labelSample,
xAxisLabelConfig.padding,
xAxisLabelConfig.fontSize,
xAxisLabelConfig.fontFamily,
);
bboxCompute.destroy();
const maxTicks = Math.floor(chartWidth / width);
// Dividing by 2 is a temp fix to make sure {@link ScaleContinuous} won't produce
// to many ticks creating nice rounded tick values
// TODO add support for limiting the number of tick in {@link ScaleContinuous}
return maxTicks / 2;
function getTicks(chartWidth: number, { formatter, padding, fontSize, fontFamily }: Config['xAxisLabel']): number {
return withTextMeasure((textMeasure) => {
const labelSample = formatter(Date.now());
const { width } = textMeasure(labelSample, padding, fontSize, fontFamily);
const maxTicks = Math.floor(chartWidth / width);
// Dividing by 2 is a temp fix to make sure {@link ScaleContinuous} won't produce
// to many ticks creating nice rounded tick values
// TODO add support for limiting the number of tick in {@link ScaleContinuous}
return maxTicks / 2;
});
}

/** @internal */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import { ScaleContinuous } from '../../../../scales';
import { ScaleType } from '../../../../scales/constants';
import { createCustomCachedSelector } from '../../../../state/create_selector';
import { CanvasTextBBoxCalculator } from '../../../../utils/bbox/canvas_text_bbox_calculator';
import { withTextMeasure } from '../../../../utils/bbox/canvas_text_bbox_calculator';
import { getHeatmapConfigSelector } from './get_heatmap_config';
import { getHeatmapTableSelector } from './get_heatmap_table';

Expand Down Expand Up @@ -37,13 +37,11 @@ export const getXAxisRightOverflow = createCustomCachedSelector(
timeZone,
},
);
const bboxCompute = new CanvasTextBBoxCalculator();
const maxTextWidth = timeScale.ticks().reduce((acc, d) => {
const text = formatter(d);
const textSize = bboxCompute.compute(text, padding, fontSize, fontFamily, 1);
return Math.max(acc, textSize.width + padding);
}, 0);
bboxCompute.destroy();
return maxTextWidth / 2;
return withTextMeasure(
(textMeasure) =>
timeScale.ticks().reduce((acc, d) => {
return Math.max(acc, textMeasure(formatter(d), padding, fontSize, fontFamily, 1).width + padding);
}, 0) / 2,
);
},
);
78 changes: 27 additions & 51 deletions packages/charts/src/chart_types/xy_chart/axes/axes_sizes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,14 @@

import { SmallMultiplesSpec } from '../../../specs';
import { Position } from '../../../utils/common';
import { getSimplePadding, PerSideDistance } from '../../../utils/dimensions';
import { innerPad, outerPad, PerSideDistance } from '../../../utils/dimensions';
import { AxisId } from '../../../utils/ids';
import { AxisStyle, Theme } from '../../../utils/themes/theme';
import { getSpecsById } from '../state/utils/spec';
import { isVerticalAxis } from '../utils/axis_type_utils';
import { AxisViewModel, getTitleDimension, shouldShowTicks } from '../utils/axis_utils';
import { AxisSpec } from '../utils/specs';

const nullPadding = (): PerSideDistance => ({ left: 0, right: 0, top: 0, bottom: 0 });

/** @internal */
export function computeAxesSizes(
{ axes: sharedAxesStyles, chartMargins }: Theme,
Expand All @@ -26,65 +24,43 @@ export function computeAxesSizes(
axisSpecs: AxisSpec[],
smSpec?: SmallMultiplesSpec,
): PerSideDistance & { margin: { left: number } } {
const axisMainSize = nullPadding();
const axisLabelOverflow = nullPadding();
const axisMainSize: PerSideDistance = { left: 0, right: 0, top: 0, bottom: 0 };
const axisLabelOverflow: PerSideDistance = { left: 0, right: 0, top: 0, bottom: 0 };

axisDimensions.forEach(({ maxLabelBboxWidth = 0, maxLabelBboxHeight = 0, isHidden }, id) => {
const axisSpec = getSpecsById<AxisSpec>(axisSpecs, id);
if (!axisSpec || isHidden) {
if (isHidden || !axisSpec) {
return;
}
const { tickLine, axisTitle, axisPanelTitle, tickLabel } = axesStyles.get(id) ?? sharedAxesStyles;
const showTicks = shouldShowTicks(tickLine, axisSpec.hide);
const { position, title } = axisSpec;
const labelPadding = getSimplePadding(tickLabel.padding);
const labelPaddingSum = tickLabel.visible ? labelPadding.inner + labelPadding.outer : 0;

const tickDimension = showTicks ? tickLine.size + tickLine.padding : 0;
const titleDimension = title ? getTitleDimension(axisTitle) : 0;
const { tickLine, axisTitle, axisPanelTitle, tickLabel } = axesStyles.get(id) ?? sharedAxesStyles;
const hasPanelTitle = Boolean(isVerticalAxis(position) ? smSpec?.splitVertically : smSpec?.splitHorizontally);
const panelTitleDimension = hasPanelTitle ? getTitleDimension(axisPanelTitle) : 0;
const labelPaddingSum = tickLabel.visible ? innerPad(tickLabel.padding) + outerPad(tickLabel.padding) : 0;
const titleDimension = title ? getTitleDimension(axisTitle) : 0;
const tickDimension = shouldShowTicks(tickLine, axisSpec.hide) ? tickLine.size + tickLine.padding : 0;
const axisDimension = labelPaddingSum + tickDimension + titleDimension + panelTitleDimension;
const maxAxisHeight = tickLabel.visible ? maxLabelBboxHeight + axisDimension : axisDimension;
const maxAxisWidth = tickLabel.visible ? maxLabelBboxWidth + axisDimension : axisDimension;

switch (position) {
case Position.Top:
axisMainSize.top += maxAxisHeight + chartMargins.top;
// find the max half label size to accommodate the left/right labels
// TODO use first and last labels
axisLabelOverflow.left = Math.max(axisLabelOverflow.left, maxLabelBboxWidth / 2);
axisLabelOverflow.right = Math.max(axisLabelOverflow.right, maxLabelBboxWidth / 2);
break;
case Position.Bottom:
axisMainSize.bottom += maxAxisHeight + chartMargins.bottom;
// find the max half label size to accommodate the left/right labels
// TODO use first and last labels
axisLabelOverflow.left = Math.max(axisLabelOverflow.left, maxLabelBboxWidth / 2);
axisLabelOverflow.right = Math.max(axisLabelOverflow.right, maxLabelBboxWidth / 2);
break;
case Position.Right:
axisMainSize.right += maxAxisWidth + chartMargins.right;
// TODO use first and last labels
axisLabelOverflow.top = Math.max(axisLabelOverflow.top, maxLabelBboxHeight / 2);
axisLabelOverflow.bottom = Math.max(axisLabelOverflow.bottom, maxLabelBboxHeight / 2);
break;
case Position.Left:
default:
axisMainSize.left += maxAxisWidth + chartMargins.left;
// TODO use first and last labels
axisLabelOverflow.top = Math.max(axisLabelOverflow.top, maxLabelBboxHeight / 2);
axisLabelOverflow.bottom = Math.max(axisLabelOverflow.bottom, maxLabelBboxHeight / 2);
// TODO use first and last labels
if (isVerticalAxis(position)) {
axisLabelOverflow.top = Math.max(axisLabelOverflow.top, maxLabelBboxHeight / 2);
axisLabelOverflow.bottom = Math.max(axisLabelOverflow.bottom, maxLabelBboxHeight / 2);
const maxAxisWidth = axisDimension + (tickLabel.visible ? maxLabelBboxWidth : 0);
axisMainSize.left += position === Position.Left ? maxAxisWidth + chartMargins.left : 0;
axisMainSize.right += position === Position.Right ? maxAxisWidth + chartMargins.right : 0;
} else {
// find the max half label size to accommodate the left/right labels
axisLabelOverflow.left = Math.max(axisLabelOverflow.left, maxLabelBboxWidth / 2);
axisLabelOverflow.right = Math.max(axisLabelOverflow.right, maxLabelBboxWidth / 2);
const maxAxisHeight = axisDimension + (tickLabel.visible ? maxLabelBboxHeight : 0);
axisMainSize.top += position === Position.Top ? maxAxisHeight + chartMargins.top : 0;
axisMainSize.bottom += position === Position.Bottom ? maxAxisHeight + chartMargins.bottom : 0;
}
});

const left = Math.max(axisLabelOverflow.left + chartMargins.left, axisMainSize.left);
return {
margin: {
left: left - axisMainSize.left,
},
left,
right: Math.max(axisLabelOverflow.right + chartMargins.right, axisMainSize.right),
top: Math.max(axisLabelOverflow.top + chartMargins.top, axisMainSize.top),
bottom: Math.max(axisLabelOverflow.bottom + chartMargins.bottom, axisMainSize.bottom),
};
const right = Math.max(axisLabelOverflow.right + chartMargins.right, axisMainSize.right);
const top = Math.max(axisLabelOverflow.top + chartMargins.top, axisMainSize.top);
const bottom = Math.max(axisLabelOverflow.bottom + chartMargins.bottom, axisMainSize.bottom);
return { left, right, top, bottom, margin: { left: left - axisMainSize.left } };
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,7 @@ export function renderLineAnnotations(
dash: lineStyle.line.dash,
};

annotations.forEach(({ linePathPoints, panel }) => {
withPanelTransform(ctx, panel, rotation, renderingArea, (ctx) => {
renderMultiLine(ctx, [linePathPoints], stroke);
});
});
annotations.forEach(({ linePathPoints, panel }) =>
withPanelTransform(ctx, panel, rotation, renderingArea, () => renderMultiLine(ctx, [linePathPoints], stroke)),
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,12 @@ export function renderRectAnnotations(
) {
const fillColor = stringToRGB(rectStyle.fill);
fillColor.opacity *= rectStyle.opacity;
const fill: Fill = {
color: fillColor,
};
const fill: Fill = { color: fillColor };
const strokeColor = stringToRGB(rectStyle.stroke);
strokeColor.opacity *= rectStyle.opacity;
const stroke: Stroke = {
color: strokeColor,
width: rectStyle.strokeWidth,
};
const stroke: Stroke = { color: strokeColor, width: rectStyle.strokeWidth };

const rectsLength = annotations.length;

for (let i = 0; i < rectsLength; i++) {
const { rect, panel } = annotations[i];
withPanelTransform(ctx, panel, rotation, renderingArea, (ctx) => {
renderRect(ctx, rect, fill, stroke);
});
}
annotations.forEach(({ rect, panel }) =>
withPanelTransform(ctx, panel, rotation, renderingArea, () => renderRect(ctx, rect, fill, stroke)),
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,12 @@ interface GridProps {
}

/** @internal */
export function renderGrids(ctx: CanvasRenderingContext2D, props: GridProps) {
const {
perPanelGridLines,
renderingArea: { left, top },
} = props;
export function renderGrids(
ctx: CanvasRenderingContext2D,
{ perPanelGridLines, renderingArea: { left, top } }: GridProps,
) {
withContext(ctx, () => {
ctx.translate(left, top);

perPanelGridLines.forEach(({ lineGroups, panelAnchor: { x, y } }) => {
withContext(ctx, () => {
ctx.translate(x, y);
Expand Down

This file was deleted.

Loading

0 comments on commit 4b43650

Please sign in to comment.