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

[XY] Reference lines overlay fix. #132607

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
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ describe('referenceLine', () => {
test('produces the correct arguments for minimum arguments', async () => {
const args: ReferenceLineArgs = {
value: 100,
fill: 'above',
};

const result = referenceLineFunction.fn(null, args, createMockExecutionContext());
Expand Down Expand Up @@ -67,6 +68,7 @@ describe('referenceLine', () => {
const args: ReferenceLineArgs = {
name: 'some name',
value: 100,
fill: 'none',
};

const result = referenceLineFunction.fn(null, args, createMockExecutionContext());
Expand All @@ -90,6 +92,7 @@ describe('referenceLine', () => {
const args: ReferenceLineArgs = {
value: 100,
textVisibility: true,
fill: 'none',
};

const result = referenceLineFunction.fn(null, args, createMockExecutionContext());
Expand All @@ -115,6 +118,7 @@ describe('referenceLine', () => {
value: 100,
name: 'some text',
textVisibility,
fill: 'none',
};

const result = referenceLineFunction.fn(null, args, createMockExecutionContext());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -297,9 +297,10 @@ export type ExtendedAnnotationLayerConfigResult = ExtendedAnnotationLayerArgs &
layerType: typeof LayerTypes.ANNOTATIONS;
};

export interface ReferenceLineArgs extends Omit<ExtendedYConfig, 'forAccessor'> {
export interface ReferenceLineArgs extends Omit<ExtendedYConfig, 'forAccessor' | 'fill'> {
name?: string;
value: number;
fill: FillStyle;
}

export interface ReferenceLineLayerArgs {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ interface ReferenceLineProps {
formatters: Record<'left' | 'right' | 'bottom', FieldFormat | undefined>;
axesMap: Record<'left' | 'right', boolean>;
isHorizontal: boolean;
nextValue?: number;
}

export const ReferenceLine: FC<ReferenceLineProps> = ({
Expand All @@ -27,6 +28,7 @@ export const ReferenceLine: FC<ReferenceLineProps> = ({
formatters,
paddingMap,
isHorizontal,
nextValue,
}) => {
const {
yConfig: [yConfig],
Expand All @@ -46,7 +48,7 @@ export const ReferenceLine: FC<ReferenceLineProps> = ({

return (
<ReferenceLineAnnotations
config={{ id, ...yConfig }}
config={{ id, ...yConfig, nextValue }}
paddingMap={paddingMap}
axesMap={axesMap}
formatter={formatter}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ describe('ReferenceLines', () => {
['yAccessorLeft', 'below'],
['yAccessorRight', 'above'],
['yAccessorRight', 'below'],
] as Array<[string, ExtendedYConfig['fill']]>)(
] as Array<[string, Exclude<ExtendedYConfig['fill'], undefined>]>)(
'should render a RectAnnotation for a reference line with fill set: %s %s',
(layerPrefix, fill) => {
const axisMode = getAxisFromId(layerPrefix);
Expand Down Expand Up @@ -154,7 +154,7 @@ describe('ReferenceLines', () => {
it.each([
['xAccessor', 'above'],
['xAccessor', 'below'],
] as Array<[string, ExtendedYConfig['fill']]>)(
] as Array<[string, Exclude<ExtendedYConfig['fill'], undefined>]>)(
'should render a RectAnnotation for a reference line with fill set: %s %s',
(layerPrefix, fill) => {
const wrapper = shallow(
Expand Down Expand Up @@ -196,7 +196,7 @@ describe('ReferenceLines', () => {
['yAccessorLeft', 'below', { y0: undefined, y1: 5 }, { y0: 5, y1: 10 }],
['yAccessorRight', 'above', { y0: 5, y1: 10 }, { y0: 10, y1: undefined }],
['yAccessorRight', 'below', { y0: undefined, y1: 5 }, { y0: 5, y1: 10 }],
] as Array<[string, ExtendedYConfig['fill'], YCoords, YCoords]>)(
] as Array<[string, Exclude<ExtendedYConfig['fill'], undefined>, YCoords, YCoords]>)(
'should avoid overlap between two reference lines with fill in the same direction: 2 x %s %s',
(layerPrefix, fill, coordsA, coordsB) => {
const axisMode = getAxisFromId(layerPrefix);
Expand Down Expand Up @@ -252,7 +252,7 @@ describe('ReferenceLines', () => {
it.each([
['xAccessor', 'above', { x0: 1, x1: 2 }, { x0: 2, x1: undefined }],
['xAccessor', 'below', { x0: undefined, x1: 1 }, { x0: 1, x1: 2 }],
] as Array<[string, ExtendedYConfig['fill'], XCoords, XCoords]>)(
] as Array<[string, Exclude<ExtendedYConfig['fill'], undefined>, XCoords, XCoords]>)(
'should avoid overlap between two reference lines with fill in the same direction: 2 x %s %s',
(layerPrefix, fill, coordsA, coordsB) => {
const wrapper = shallow(
Expand Down Expand Up @@ -361,7 +361,7 @@ describe('ReferenceLines', () => {
it.each([
['above', { y0: 5, y1: 10 }, { y0: 10, y1: undefined }],
['below', { y0: undefined, y1: 5 }, { y0: 5, y1: 10 }],
] as Array<[ExtendedYConfig['fill'], YCoords, YCoords]>)(
] as Array<[Exclude<ExtendedYConfig['fill'], undefined>, YCoords, YCoords]>)(
'should be robust and works also for different axes when on same direction: 1x Left + 1x Right both %s',
(fill, coordsA, coordsB) => {
const wrapper = shallow(
Expand Down Expand Up @@ -438,7 +438,7 @@ describe('ReferenceLines', () => {
['yAccessorLeft', 'below'],
['yAccessorRight', 'above'],
['yAccessorRight', 'below'],
] as Array<[string, ExtendedYConfig['fill']]>)(
] as Array<[string, Exclude<ExtendedYConfig['fill'], undefined>]>)(
'should render a RectAnnotation for a reference line with fill set: %s %s',
(layerPrefix, fill) => {
const axisMode = getAxisFromId(layerPrefix);
Expand Down Expand Up @@ -479,7 +479,7 @@ describe('ReferenceLines', () => {
it.each([
['xAccessor', 'above'],
['xAccessor', 'below'],
] as Array<[string, ExtendedYConfig['fill']]>)(
] as Array<[string, Exclude<ExtendedYConfig['fill'], undefined>]>)(
'should render a RectAnnotation for a reference line with fill set: %s %s',
(layerPrefix, fill) => {
const value = 1;
Expand Down Expand Up @@ -519,7 +519,7 @@ describe('ReferenceLines', () => {
it.each([
['yAccessorLeft', 'above', { y0: 10, y1: undefined }, { y0: 10, y1: undefined }],
['yAccessorLeft', 'below', { y0: undefined, y1: 5 }, { y0: undefined, y1: 5 }],
] as Array<[string, ExtendedYConfig['fill'], YCoords, YCoords]>)(
] as Array<[string, Exclude<ExtendedYConfig['fill'], undefined>, YCoords, YCoords]>)(
'should avoid overlap between two reference lines with fill in the same direction: 2 x %s %s',
(layerPrefix, fill, coordsA, coordsB) => {
const axisMode = getAxisFromId(layerPrefix);
Expand Down Expand Up @@ -570,7 +570,7 @@ describe('ReferenceLines', () => {
it.each([
['xAccessor', 'above', { x0: 1, x1: undefined }, { x0: 1, x1: undefined }],
['xAccessor', 'below', { x0: undefined, x1: 1 }, { x0: undefined, x1: 1 }],
] as Array<[string, ExtendedYConfig['fill'], XCoords, XCoords]>)(
] as Array<[string, Exclude<ExtendedYConfig['fill'], undefined>, XCoords, XCoords]>)(
'should avoid overlap between two reference lines with fill in the same direction: 2 x %s %s',
(layerPrefix, fill, coordsA, coordsB) => {
const value = coordsA.x0 ?? coordsA.x1!;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,44 +11,11 @@ import './reference_lines.scss';
import React from 'react';
import { Position } from '@elastic/charts';
import type { FieldFormat } from '@kbn/field-formats-plugin/common';
import type { CommonXYReferenceLineLayerConfig } from '../../../common/types';
import { isReferenceLine, mapVerticalToHorizontalPlacement } from '../../helpers';
import type { CommonXYReferenceLineLayerConfig, ReferenceLineConfig } from '../../../common/types';
import { isReferenceLine } from '../../helpers';
import { ReferenceLineLayer } from './reference_line_layer';
import { ReferenceLine } from './reference_line';

export const computeChartMargins = (
referenceLinePaddings: Partial<Record<Position, number>>,
labelVisibility: Partial<Record<'x' | 'yLeft' | 'yRight', boolean>>,
titleVisibility: Partial<Record<'x' | 'yLeft' | 'yRight', boolean>>,
axesMap: Record<'left' | 'right', unknown>,
isHorizontal: boolean
) => {
const result: Partial<Record<Position, number>> = {};
if (!labelVisibility?.x && !titleVisibility?.x && referenceLinePaddings.bottom) {
const placement = isHorizontal ? mapVerticalToHorizontalPlacement('bottom') : 'bottom';
result[placement] = referenceLinePaddings.bottom;
}
if (
referenceLinePaddings.left &&
(isHorizontal || (!labelVisibility?.yLeft && !titleVisibility?.yLeft))
) {
const placement = isHorizontal ? mapVerticalToHorizontalPlacement('left') : 'left';
result[placement] = referenceLinePaddings.left;
}
if (
referenceLinePaddings.right &&
(isHorizontal || !axesMap.right || (!labelVisibility?.yRight && !titleVisibility?.yRight))
) {
const placement = isHorizontal ? mapVerticalToHorizontalPlacement('right') : 'right';
result[placement] = referenceLinePaddings.right;
}
// there's no top axis, so just check if a margin has been computed
if (referenceLinePaddings.top) {
const placement = isHorizontal ? mapVerticalToHorizontalPlacement('top') : 'top';
result[placement] = referenceLinePaddings.top;
}
return result;
};
import { getNextValuesForReferenceLines } from './utils';

export interface ReferenceLinesProps {
layers: CommonXYReferenceLineLayerConfig[];
Expand All @@ -59,20 +26,26 @@ export interface ReferenceLinesProps {
}

export const ReferenceLines = ({ layers, ...rest }: ReferenceLinesProps) => {
const referenceLines = layers.filter((layer): layer is ReferenceLineConfig =>
isReferenceLine(layer)
);

const referenceLinesNextValues = getNextValuesForReferenceLines(referenceLines);

return (
<>
{layers.flatMap((layer) => {
if (!layer.yConfig) {
return null;
}

const key = `referenceLine-${layer.layerId}`;
if (isReferenceLine(layer)) {
return <ReferenceLine key={`referenceLine-${layer.layerId}`} layer={layer} {...rest} />;
const nextValue = referenceLinesNextValues[layer.yConfig[0].fill][layer.layerId];
return <ReferenceLine key={key} layer={layer} {...rest} nextValue={nextValue} />;
}

return (
<ReferenceLineLayer key={`referenceLine-${layer.layerId}`} layer={layer} {...rest} />
);
return <ReferenceLineLayer key={key} layer={layer} {...rest} />;
})}
</>
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ import React from 'react';
import { Position } from '@elastic/charts';
import { euiLightVars } from '@kbn/ui-theme';
import { FieldFormat } from '@kbn/field-formats-plugin/common';
import { IconPosition, YAxisMode } from '../../../common/types';
import { groupBy, orderBy } from 'lodash';
import { IconPosition, ReferenceLineConfig, YAxisMode, FillStyle } from '../../../common/types';
import { FillStyles } from '../../../common/constants';
import {
LINES_MARKER_SIZE,
mapVerticalToHorizontalPlacement,
Expand Down Expand Up @@ -141,3 +143,72 @@ export const getHorizontalRect = (
header: headerLabel,
details: formatter?.convert(currentValue) || currentValue.toString(),
});

const sortReferenceLinesByGroup = (referenceLines: ReferenceLineConfig[], group: FillStyle) => {
if (group === FillStyles.ABOVE || group === FillStyles.BELOW) {
const order = group === FillStyles.ABOVE ? 'asc' : 'desc';
return orderBy(referenceLines, ({ yConfig: [{ value }] }) => value, [order]);
}
return referenceLines;
};

export const getNextValuesForReferenceLines = (referenceLines: ReferenceLineConfig[]) => {
const grouppedReferenceLines = groupBy(referenceLines, ({ yConfig: [yConfig] }) => yConfig.fill);
const groups = Object.keys(grouppedReferenceLines) as FillStyle[];

return groups.reduce<Record<FillStyle, Record<string, number | undefined>>>(
(nextValueByDirection, group) => {
const sordedReferenceLines = sortReferenceLinesByGroup(grouppedReferenceLines[group], group);

const nv = sordedReferenceLines.reduce<Record<string, number | undefined>>(
(nextValues, referenceLine, index, lines) => {
let nextValue: number | undefined;
if (index < lines.length - 1) {
const [yConfig] = lines[index + 1].yConfig;
nextValue = yConfig.value;
}

return { ...nextValues, [referenceLine.layerId]: nextValue };
},
{}
);

return { ...nextValueByDirection, [group]: nv };
},
{} as Record<FillStyle, Record<string, number>>
);
};

export const computeChartMargins = (
referenceLinePaddings: Partial<Record<Position, number>>,
labelVisibility: Partial<Record<'x' | 'yLeft' | 'yRight', boolean>>,
titleVisibility: Partial<Record<'x' | 'yLeft' | 'yRight', boolean>>,
axesMap: Record<'left' | 'right', unknown>,
isHorizontal: boolean
) => {
const result: Partial<Record<Position, number>> = {};
if (!labelVisibility?.x && !titleVisibility?.x && referenceLinePaddings.bottom) {
const placement = isHorizontal ? mapVerticalToHorizontalPlacement('bottom') : 'bottom';
result[placement] = referenceLinePaddings.bottom;
}
if (
referenceLinePaddings.left &&
(isHorizontal || (!labelVisibility?.yLeft && !titleVisibility?.yLeft))
) {
const placement = isHorizontal ? mapVerticalToHorizontalPlacement('left') : 'left';
result[placement] = referenceLinePaddings.left;
}
if (
referenceLinePaddings.right &&
(isHorizontal || !axesMap.right || (!labelVisibility?.yRight && !titleVisibility?.yRight))
) {
const placement = isHorizontal ? mapVerticalToHorizontalPlacement('right') : 'right';
result[placement] = referenceLinePaddings.right;
}
// there's no top axis, so just check if a margin has been computed
if (referenceLinePaddings.top) {
const placement = isHorizontal ? mapVerticalToHorizontalPlacement('top') : 'top';
result[placement] = referenceLinePaddings.top;
}
return result;
};