Skip to content

Commit

Permalink
fix(heatmap): respect margins and paddings (#2577)
Browse files Browse the repository at this point in the history
* [heatmap] Add margin and padding knobs to storybook

* [heatmap] Fix chart margins and paddings

* fix(heatmap): update x and y label origins

* fix(heatmap): Update points for group titles

* Add margins and paddings knobs to heatmap small multiple story

* Add common renderers and colors

* heatmap(fix): Respect paddings and add debug

* fix: resolve errors

* Update heatmap stories to not use the legacy margins

* Update additional heatmap stories

* test: Add test for magins and paddings

* Use legacy margings for heatmap stories

* Extract isPointWithinYLabelArea method

* fix(heatmap): fix brush mask and area

* Update brush mask rect y

* Update rgba colors

* Share ChartDimensions interface

* Apply margins and paddings to dragArea in viewmodel

* Remove ctx from margins and paddins renderers

* Apply suggestions

* Use satisfies with RgbaTuple

* Apply debug renderers suggestions

* Fix error

* Add debug elements to a single function

* test(vrt): update screenshots [skip ci]

* Add test for debug elements

* test(vrt): update screenshots [skip ci]

* Delete not used screenshot

---------

Co-authored-by: elastic-datavis[bot] <98618603+elastic-datavis[bot]@users.noreply.github.com>
  • Loading branch information
mariairiartef and elastic-datavis[bot] authored Jan 28, 2025
1 parent 8f6872f commit c24566d
Show file tree
Hide file tree
Showing 73 changed files with 661 additions and 108 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
12 changes: 12 additions & 0 deletions e2e/tests/heatmap_stories.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,18 @@ test.describe('Heatmap stories', () => {
);
});

test('should render heatmap with margins and paddings', async ({ page }) => {
await common.expectChartAtUrlToMatchScreenshot(page)(
'http://localhost:9001/?path=/story/heatmap-alpha--theming&globals=toggles.showHeader:true;toggles.showChartTitle:false;toggles.showChartDescription:false;toggles.showChartBoundary:false;theme:light&knob-Debug=&knob-Enable debug state=true&knob-axisTitle fontSize_Axis Title=12&knob-axisTitle inner pad_Axis Title=8&knob-axisTitle outer pad_Axis Title=8&knob-axisTitle textColor_Axis Title=black&knob-border stroke color_Theme=gray&knob-border strokeWidth_Theme=0&knob-brushArea fill_Theme=black&knob-brushArea strokeWidth_Theme=2&knob-yAxisLabel width type_Theme=auto&knob-yAxisLabel width max/static_Theme=100&knob-brushArea visible_Theme=true&knob-brushArea stroke_Theme=&knob-brushMask visible_Theme=true&knob-brushMask fill_Theme=rgb(115 115 115 / 50%)&knob-brushTool visible_Theme=&knob-brushTool fill color_Theme=gray&knob-xAxisLabel visible_Theme=true&knob-xAxisLabel fontSize_Theme=12&knob-xAxisLabel textColor_Theme=black&knob-xAxisLabel padding_Theme=6&knob-yAxisLabel visible_Theme=true&knob-yAxisLabel fontSize_Theme=12&knob-yAxisLabel textColor_Theme=black&knob-yAxisLabel padding_Theme=5&knob-grid stroke color_Theme=gray&knob-grid stroke width_Theme=1&knob-cell label visible_Theme=&knob-cell label textColor_Theme=black&knob-cell label use global min fontSize_Theme=true&knob-cell label min fontSize_Theme=6&knob-cell label max fontSize_Theme=12&knob-chart margin left_Chart Margins and Paddings=20&knob-chart margin right_Chart Margins and Paddings=20&knob-chart margin top_Chart Margins and Paddings=20&knob-chart margin bottom_Chart Margins and Paddings=20&knob-chart padding left_Chart Margins and Paddings=20&knob-chart padding right_Chart Margins and Paddings=20&knob-chart padding top_Chart Margins and Paddings=20&knob-chart padding bottom_Chart Margins and Paddings=20&knob-xAxisTitle_Axis Title=xAxis&knob-yAxisTitle_Axis Title=yAxis',
);
});

test('should render heatmap with debug elements', async ({ page }) => {
await common.expectChartAtUrlToMatchScreenshot(page)(
'http://localhost:9001/?path=/story/heatmap-alpha--theming&globals=toggles.showHeader:true;toggles.showChartTitle:false;toggles.showChartDescription:false;toggles.showChartBoundary:false;theme:light&knob-Debug=true&knob-Enable debug state=true&knob-axisTitle fontSize_Axis Title=12&knob-axisTitle inner pad_Axis Title=8&knob-axisTitle outer pad_Axis Title=8&knob-axisTitle textColor_Axis Title=black&knob-border stroke color_Theme=gray&knob-border strokeWidth_Theme=0&knob-brushArea fill_Theme=black&knob-brushArea strokeWidth_Theme=2&knob-yAxisLabel width type_Theme=auto&knob-yAxisLabel width max/static_Theme=100&knob-brushArea visible_Theme=true&knob-brushArea stroke_Theme=&knob-brushMask visible_Theme=true&knob-brushMask fill_Theme=rgb(115 115 115 / 50%)&knob-brushTool visible_Theme=&knob-brushTool fill color_Theme=gray&knob-xAxisLabel visible_Theme=true&knob-xAxisLabel fontSize_Theme=12&knob-xAxisLabel textColor_Theme=black&knob-xAxisLabel padding_Theme=6&knob-yAxisLabel visible_Theme=true&knob-yAxisLabel fontSize_Theme=12&knob-yAxisLabel textColor_Theme=black&knob-yAxisLabel padding_Theme=5&knob-grid stroke color_Theme=gray&knob-grid stroke width_Theme=1&knob-cell label visible_Theme=&knob-cell label textColor_Theme=black&knob-cell label use global min fontSize_Theme=true&knob-cell label min fontSize_Theme=6&knob-cell label max fontSize_Theme=12&knob-chart margin left_Chart Margins and Paddings=20&knob-chart margin right_Chart Margins and Paddings=20&knob-chart margin top_Chart Margins and Paddings=20&knob-chart margin bottom_Chart Margins and Paddings=20&knob-chart padding left_Chart Margins and Paddings=20&knob-chart padding right_Chart Margins and Paddings=20&knob-chart padding top_Chart Margins and Paddings=20&knob-chart padding bottom_Chart Margins and Paddings=20&knob-xAxisTitle_Axis Title=xAxis&knob-yAxisTitle_Axis Title=yAxis',
);
});

test.describe('Small multiples', () => {
const titleOptions = {
panel: ' without panel titles',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,9 @@ import { Color, Colors } from '../../../../common/colors';
import { Ratio } from '../../../../common/geometry';
import { cssFontShorthand } from '../../../../common/text_utils';
import { withContext, clearCanvas } from '../../../../renderers/canvas';
import { renderDebugRect } from '../../../../renderers/canvas/utils/debug';
import { A11ySettings } from '../../../../state/selectors/get_accessibility_config';
import { renderDebugPoint, renderDebugRect } from '../../../xy_chart/renderer/canvas/utils/debug';
import { renderDebugPoint } from '../../../xy_chart/renderer/canvas/utils/debug';
import { ActiveValue } from '../../selectors/get_active_values';
import { BulletDimensions } from '../../selectors/get_panel_dimensions';
import { BulletSpec, BulletSubtype } from '../../spec';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@
import { ColorScale } from '../../../../common/colors';
import { SmallMultipleScales, SmallMultiplesGroupBy } from '../../../../common/panel_utils';
import { withTextMeasure } from '../../../../utils/bbox/canvas_text_bbox_calculator';
import { ChartDimensions } from '../../../../utils/dimensions';
import { Theme } from '../../../../utils/themes/theme';
import { ChartDimensions } from '../../../xy_chart/utils/dimensions';
import { ShapeViewModel } from '../../layout/types/viewmodel_types';
import { shapeViewModel } from '../../layout/viewmodel/viewmodel';
import { HeatmapSpec } from '../../specs';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

import { ScaleBand, scaleBand, scaleQuantize } from 'd3-scale';

import { BaseDatum } from './../../../xy_chart/utils/specs';
import { colorToRgba } from '../../../../common/color_library_wrappers';
import { ColorScale } from '../../../../common/colors';
import { fillTextColor } from '../../../../common/fill_text_color';
Expand All @@ -25,15 +24,14 @@ import {
} from '../../../../common/panel_utils';
import { Box, Font, maximiseFontSize } from '../../../../common/text_utils';
import { ScaleType } from '../../../../scales/constants';
import { LinearScale, OrdinalScale, RasterTimeScale } from '../../../../specs';
import { LinearScale, OrdinalScale, RasterTimeScale, BaseDatum } from '../../../../specs';
import { TextMeasure } from '../../../../utils/bbox/canvas_text_bbox_calculator';
import { addIntervalToTime, roundDateToESInterval } from '../../../../utils/chrono/elasticsearch';
import { clamp, Datum, isFiniteNumber, isNil } from '../../../../utils/common';
import { innerPad, pad } from '../../../../utils/dimensions';
import { innerPad, pad, ChartDimensions } from '../../../../utils/dimensions';
import { Logger } from '../../../../utils/logger';
import { HeatmapStyle, Theme, Visible } from '../../../../utils/themes/theme';
import { PrimitiveValue } from '../../../partition_chart/layout/utils/group_by_rollup';
import { ChartDimensions } from '../../../xy_chart/utils/dimensions';
import { HeatmapSpec } from '../../specs';
import { ChartElementSizes } from '../../state/selectors/compute_chart_element_sizes';
import { HeatmapTable } from '../../state/selectors/get_heatmap_table';
Expand Down Expand Up @@ -79,7 +77,7 @@ export function clampWithOffset(value: number, lowerBound: number, upperBound: n
export function shapeViewModel<D extends BaseDatum = Datum>(
textMeasure: TextMeasure,
spec: HeatmapSpec<D>,
{ heatmap: heatmapTheme, axes: { axisTitle, axisPanelTitle }, background }: Theme,
{ heatmap: heatmapTheme, axes: { axisTitle, axisPanelTitle }, background, chartMargins, chartPaddings }: Theme,
{ chartDimensions }: ChartDimensions,
elementSizes: ChartElementSizes,
heatmapTable: HeatmapTable,
Expand Down Expand Up @@ -121,7 +119,14 @@ export function shapeViewModel<D extends BaseDatum = Datum>(
const cellHeight = yScale.bandwidth();

// compute the position of each column label
const textXValues = getXTicks(spec, heatmapTheme.xAxisLabel, xScale, heatmapTable.xValues);
const textXValues = getXTicks(
spec,
heatmapTheme.xAxisLabel,
xScale,
heatmapTable.xValues,
chartMargins,
chartPaddings,
);

const { padding } = heatmapTheme.yAxisLabel;

Expand All @@ -130,7 +135,7 @@ export function shapeViewModel<D extends BaseDatum = Datum>(
return {
...d,
// position of the Y labels
x: -pad(padding, 'right'),
x: -pad(padding, 'right') - chartPaddings.left,
y: cellHeight / 2 + (yScale(d.value) || 0),
align: 'right',
};
Expand Down Expand Up @@ -224,7 +229,11 @@ export function shapeViewModel<D extends BaseDatum = Datum>(
};

const getPanelPointCoordinates = (x: Pixels, y: Pixels) => {
const { category: v, panelValue: panelY, panelOffset: panelOffsetY } = getPanelPointCoordinate(y, 'vertical');
const {
category: v,
panelValue: panelY,
panelOffset: panelOffsetY,
} = getPanelPointCoordinate(y - chartDimensions.top, 'vertical');
const {
category: h,
panelValue: panelX,
Expand All @@ -241,6 +250,13 @@ export function shapeViewModel<D extends BaseDatum = Datum>(
};
};

// TODO: Make this method more precise to avoid Y title
const isPointWithinYLabelArea = (x: Pixels, y: Pixels) =>
x > chartMargins.left &&
x < chartDimensions.left - chartPaddings.left &&
y > chartDimensions.top &&
y < chartDimensions.top + chartDimensions.height;

/**
* Returns the corresponding x & y values of grid cell from the x & y positions
* @param x
Expand All @@ -265,14 +281,9 @@ export function shapeViewModel<D extends BaseDatum = Datum>(
* @param y
*/
const pickQuads = (x: Pixels, y: Pixels): Array<Cell> | TextBox => {
if (
x > 0 &&
x < chartDimensions.left &&
y > chartDimensions.top &&
y < chartDimensions.top + chartDimensions.height
) {
if (isPointWithinYLabelArea(x, y)) {
// look up for a Y axis elements
const { y: yLabelKey } = getPanelPointCoordinates(x - chartDimensions.left, y);
const { y: yLabelKey } = getPanelPointCoordinates(x, y);
const yLabelValue = textYValues.find((v) => v.value === yLabelKey);
if (yLabelValue) {
return yLabelValue;
Expand Down Expand Up @@ -313,7 +324,7 @@ export function shapeViewModel<D extends BaseDatum = Datum>(
'horizontal',
);
const { category: smVerticalAccessorValue, panelOffset: verticalPanelOffset } = getPanelPointCoordinate(
start.y,
start.y - chartDimensions.top,
'vertical',
);

Expand Down Expand Up @@ -405,7 +416,7 @@ export function shapeViewModel<D extends BaseDatum = Datum>(
);
return {
x: xStart,
y: yStart,
y: yStart + chartMargins.top + chartPaddings.top + gridStrokeWidth / 2,
width,
height: totalHeight,
};
Expand Down Expand Up @@ -504,7 +515,10 @@ export function shapeViewModel<D extends BaseDatum = Datum>(
elementSizes.xAxis.height +
axisPanelTitleHeight +
innerPad(axisTitle.padding) / 2 +
axisTitle.fontSize / 2,
axisTitle.fontSize / 2 -
chartPaddings.top +
chartPaddings.bottom -
chartMargins.top,
},
...axisTitleFont,
text: spec.xAxisTitle,
Expand All @@ -515,8 +529,8 @@ export function shapeViewModel<D extends BaseDatum = Datum>(
if (spec.yAxisTitle) {
titles.push({
origin: {
x: -chartDimensions.left + axisTitle.fontSize / 2,
y: chartDimensions.top + chartDimensions.height / 2,
x: -chartDimensions.left + axisTitle.fontSize / 2 + chartMargins.left,
y: chartDimensions.top + chartDimensions.height / 2 - chartMargins.top - chartPaddings.top,
},
...axisTitleFont,
text: spec.yAxisTitle,
Expand All @@ -534,7 +548,10 @@ export function shapeViewModel<D extends BaseDatum = Datum>(
chartDimensions.height +
elementSizes.xAxis.height +
innerPad(axisPanelTitle.padding) +
axisPanelTitle.fontSize / 2,
axisPanelTitle.fontSize / 2 -
chartPaddings.top +
chartPaddings.bottom -
chartMargins.top,
},
...axisPanelTitleFont,
text: getPanelTitle(false, v, h, groupBySpec),
Expand All @@ -546,8 +563,8 @@ export function shapeViewModel<D extends BaseDatum = Datum>(
const axisTitleWidth = axisTitle.visible ? axisTitle.fontSize + innerPad(axisTitle.padding) : 0;
titles.push({
origin: {
x: -chartDimensions.left + axisTitleWidth + axisPanelTitle.fontSize / 2,
y: chartDimensions.top + panelSize.height / 2,
x: -chartDimensions.left + axisTitleWidth + axisPanelTitle.fontSize / 2 + chartMargins.left,
y: chartDimensions.top + panelSize.height / 2 - chartMargins.top - chartPaddings.top,
},
...axisPanelTitleFont,
text: getPanelTitle(true, v, h, groupBySpec),
Expand Down Expand Up @@ -609,6 +626,8 @@ function getXTicks(
style: HeatmapStyle['xAxisLabel'],
scale: ScaleBand<NonNullable<PrimitiveValue>>,
values: NonNullable<PrimitiveValue>[],
chartMargins: Theme['chartMargins'],
chartPaddings: Theme['chartPaddings'],
): Array<TextBox> {
const isTimeScale = isRasterTimeScale(spec.xScale);
const isRotated = style.rotation !== 0;
Expand All @@ -618,7 +637,7 @@ function getXTicks(
value,
isValue: false,
...style,
y: style.fontSize / 2 + pad(style.padding, 'top'),
y: style.fontSize / 2 + pad(style.padding, 'top') - chartMargins.top - chartPaddings.top + chartPaddings.bottom,
x: (scale(value) ?? 0) + (isTimeScale ? 0 : scale.bandwidth() / 2),
align: isRotated ? 'right' : isTimeScale ? 'left' : 'center',
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,27 @@
*/

import { ReactiveChartStateProps } from './connected_component';
import { renderHeatmapDebugElements } from './debug';
import { getColorBandStyle, getGeometryStateStyle } from './utils';
import { clearCanvas, renderLayers, withContext } from '../../../../renderers/canvas';
import { renderMultiLine } from '../../../../renderers/canvas/primitives/line';
import { renderRect } from '../../../../renderers/canvas/primitives/rect';
import { renderText, TextFont, wrapLines } from '../../../../renderers/canvas/primitives/text';
import { radToDeg } from '../../../../utils/common';
import { horizontalPad } from '../../../../utils/dimensions';
import { renderMultiLine } from '../../../xy_chart/renderer/canvas/primitives/line';
import { renderRect } from '../../../xy_chart/renderer/canvas/primitives/rect';
import { renderText, TextFont, wrapLines } from '../../../xy_chart/renderer/canvas/primitives/text';

/** @internal */
export function renderHeatmapCanvas2d(ctx: CanvasRenderingContext2D, dpr: number, props: ReactiveChartStateProps) {
const { theme } = props.geometries;
const { heatmapViewModels } = props.geometries;
const {
theme: { sharedStyle: sharedGeometryStyle },
theme: { sharedStyle: sharedGeometryStyle, chartPaddings: paddings, chartMargins: margins },
background,
elementSizes,
highlightedLegendBands,
chartContainerDimensions: container,
chartDimensions: chart,
debug,
} = props;
if (heatmapViewModels.length === 0) return;

Expand Down Expand Up @@ -188,6 +192,8 @@ export function renderHeatmapCanvas2d(ctx: CanvasRenderingContext2D, dpr: number
});
});
}),

() => debug && renderHeatmapDebugElements({ ctx, container, chart, margins, paddings }),
]);
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import { deepEqual } from '../../../../utils/fast_deep_equal';
import { LIGHT_THEME } from '../../../../utils/themes/light_theme';
import { Theme } from '../../../../utils/themes/theme';
import { nullShapeViewModel, ShapeViewModel } from '../../layout/types/viewmodel_types';
import { computeChartDimensionsSelector } from '../../state/selectors/compute_chart_dimensions';
import { ChartElementSizes, computeChartElementSizesSelector } from '../../state/selectors/compute_chart_element_sizes';
import { getHeatmapContainerSizeSelector } from '../../state/selectors/get_heatmap_container_size';
import { getHighlightedLegendBandsSelector } from '../../state/selectors/get_highlighted_legend_bands';
Expand All @@ -45,6 +46,7 @@ export interface ReactiveChartStateProps {
background: Color;
elementSizes: ChartElementSizes;
debug: boolean;
chartDimensions: Dimensions;
}

interface ReactiveChartDispatchProps {
Expand Down Expand Up @@ -151,12 +153,7 @@ const mapDispatchToProps = (dispatch: Dispatch): ReactiveChartDispatchProps =>
const DEFAULT_PROPS: ReactiveChartStateProps = {
initialized: false,
geometries: nullShapeViewModel(),
chartContainerDimensions: {
width: 0,
height: 0,
left: 0,
top: 0,
},
chartContainerDimensions: { width: 0, height: 0, left: 0, top: 0 },
theme: LIGHT_THEME,
highlightedLegendBands: [],
a11ySettings: DEFAULT_A11Y_SETTINGS,
Expand All @@ -168,6 +165,7 @@ const DEFAULT_PROPS: ReactiveChartStateProps = {
xLabelRotation: 0,
},
debug: false,
chartDimensions: { top: 0, left: 0, width: 0, height: 0 },
};

const mapStateToProps = (state: GlobalChartState): ReactiveChartStateProps => {
Expand All @@ -184,6 +182,7 @@ const mapStateToProps = (state: GlobalChartState): ReactiveChartStateProps => {
background: getChartThemeSelector(state).background.color,
elementSizes: computeChartElementSizesSelector(state),
debug: getSettingsSpecSelector(state).debug,
chartDimensions: computeChartDimensionsSelector(state).chartDimensions,
};
};

Expand Down
Loading

0 comments on commit c24566d

Please sign in to comment.