Skip to content

Commit

Permalink
[XY] Reference lines overlay fix. (#132607)
Browse files Browse the repository at this point in the history
  • Loading branch information
Kuznietsov committed May 20, 2022
1 parent 759f13f commit 1b4ac7d
Show file tree
Hide file tree
Showing 6 changed files with 103 additions and 52 deletions.
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;
};

0 comments on commit 1b4ac7d

Please sign in to comment.