[Lens] Threshold: add padding to avoid axis label collision with threshold markers (#112952)

* 🐛 Add padding to the tick label to fit threshold markers

* 🐛 Better icon detection

* 🐛 Fix edge cases with no title or labels

* 📸 Update snapshots

*  Add icon placement flag

*  Sync padding computation with marker positioning

* 👌 Make disabled when no icon is selected

* 🐛 Fix some edge cases with auto positioning

* Update x-pack/plugins/lens/public/xy_visualization/xy_config_panel/threshold_panel.tsx

Co-authored-by: Michael Marcialis <michael@marcial.is>

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Co-authored-by: Michael Marcialis <michael@marcial.is>
This commit is contained in:
Marco Liberati 2021-10-04 13:01:14 +02:00 committed by GitHub
parent 9b7b3228ec
commit 8e25f5cc0f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 340 additions and 17 deletions

View file

@ -30,6 +30,7 @@ interface AxisConfig {
export type YAxisMode = 'auto' | 'left' | 'right' | 'bottom';
export type LineStyle = 'solid' | 'dashed' | 'dotted';
export type FillStyle = 'none' | 'above' | 'below';
export type IconPosition = 'auto' | 'left' | 'right' | 'above' | 'below';
export interface YConfig {
forAccessor: string;
@ -39,6 +40,7 @@ export interface YConfig {
lineWidth?: number;
lineStyle?: LineStyle;
fill?: FillStyle;
iconPosition?: IconPosition;
}
export type AxisTitlesVisibilityConfigResult = AxesSettingsConfig & {
@ -180,6 +182,11 @@ export const yAxisConfig: ExpressionFunctionDefinition<
types: ['string'],
help: 'An optional icon used for threshold lines',
},
iconPosition: {
types: ['string'],
options: ['auto', 'above', 'below', 'left', 'right'],
help: 'The placement of the icon for the threshold line',
},
fill: {
types: ['string'],
options: ['none', 'above', 'below'],

View file

@ -10,6 +10,7 @@ import { EuiToolTip, EuiToolTipProps } from '@elastic/eui';
export type TooltipWrapperProps = Partial<Omit<EuiToolTipProps, 'content'>> & {
tooltipContent: string;
/** When the condition is truthy, the tooltip will be shown */
condition: boolean;
};

View file

@ -28,6 +28,7 @@ exports[`xy_expression XYChart component it renders area 1`] = `
"color": undefined,
},
"barSeriesStyle": Object {},
"chartMargins": Object {},
"legend": Object {
"labelOptions": Object {
"maxLines": 0,
@ -55,9 +56,11 @@ exports[`xy_expression XYChart component it renders area 1`] = `
style={
Object {
"axisTitle": Object {
"padding": undefined,
"visible": true,
},
"tickLabel": Object {
"padding": undefined,
"rotation": 0,
"visible": true,
},
@ -86,9 +89,11 @@ exports[`xy_expression XYChart component it renders area 1`] = `
style={
Object {
"axisTitle": Object {
"padding": undefined,
"visible": true,
},
"tickLabel": Object {
"padding": undefined,
"rotation": -90,
"visible": false,
},
@ -252,6 +257,7 @@ exports[`xy_expression XYChart component it renders bar 1`] = `
"color": undefined,
},
"barSeriesStyle": Object {},
"chartMargins": Object {},
"legend": Object {
"labelOptions": Object {
"maxLines": 0,
@ -279,9 +285,11 @@ exports[`xy_expression XYChart component it renders bar 1`] = `
style={
Object {
"axisTitle": Object {
"padding": undefined,
"visible": true,
},
"tickLabel": Object {
"padding": undefined,
"rotation": 0,
"visible": true,
},
@ -310,9 +318,11 @@ exports[`xy_expression XYChart component it renders bar 1`] = `
style={
Object {
"axisTitle": Object {
"padding": undefined,
"visible": true,
},
"tickLabel": Object {
"padding": undefined,
"rotation": -90,
"visible": false,
},
@ -490,6 +500,7 @@ exports[`xy_expression XYChart component it renders horizontal bar 1`] = `
"color": undefined,
},
"barSeriesStyle": Object {},
"chartMargins": Object {},
"legend": Object {
"labelOptions": Object {
"maxLines": 0,
@ -517,9 +528,11 @@ exports[`xy_expression XYChart component it renders horizontal bar 1`] = `
style={
Object {
"axisTitle": Object {
"padding": undefined,
"visible": true,
},
"tickLabel": Object {
"padding": undefined,
"rotation": 0,
"visible": true,
},
@ -548,9 +561,11 @@ exports[`xy_expression XYChart component it renders horizontal bar 1`] = `
style={
Object {
"axisTitle": Object {
"padding": undefined,
"visible": true,
},
"tickLabel": Object {
"padding": undefined,
"rotation": -90,
"visible": false,
},
@ -728,6 +743,7 @@ exports[`xy_expression XYChart component it renders line 1`] = `
"color": undefined,
},
"barSeriesStyle": Object {},
"chartMargins": Object {},
"legend": Object {
"labelOptions": Object {
"maxLines": 0,
@ -755,9 +771,11 @@ exports[`xy_expression XYChart component it renders line 1`] = `
style={
Object {
"axisTitle": Object {
"padding": undefined,
"visible": true,
},
"tickLabel": Object {
"padding": undefined,
"rotation": 0,
"visible": true,
},
@ -786,9 +804,11 @@ exports[`xy_expression XYChart component it renders line 1`] = `
style={
Object {
"axisTitle": Object {
"padding": undefined,
"visible": true,
},
"tickLabel": Object {
"padding": undefined,
"rotation": -90,
"visible": false,
},
@ -952,6 +972,7 @@ exports[`xy_expression XYChart component it renders stacked area 1`] = `
"color": undefined,
},
"barSeriesStyle": Object {},
"chartMargins": Object {},
"legend": Object {
"labelOptions": Object {
"maxLines": 0,
@ -979,9 +1000,11 @@ exports[`xy_expression XYChart component it renders stacked area 1`] = `
style={
Object {
"axisTitle": Object {
"padding": undefined,
"visible": true,
},
"tickLabel": Object {
"padding": undefined,
"rotation": 0,
"visible": true,
},
@ -1010,9 +1033,11 @@ exports[`xy_expression XYChart component it renders stacked area 1`] = `
style={
Object {
"axisTitle": Object {
"padding": undefined,
"visible": true,
},
"tickLabel": Object {
"padding": undefined,
"rotation": -90,
"visible": false,
},
@ -1184,6 +1209,7 @@ exports[`xy_expression XYChart component it renders stacked bar 1`] = `
"color": undefined,
},
"barSeriesStyle": Object {},
"chartMargins": Object {},
"legend": Object {
"labelOptions": Object {
"maxLines": 0,
@ -1211,9 +1237,11 @@ exports[`xy_expression XYChart component it renders stacked bar 1`] = `
style={
Object {
"axisTitle": Object {
"padding": undefined,
"visible": true,
},
"tickLabel": Object {
"padding": undefined,
"rotation": 0,
"visible": true,
},
@ -1242,9 +1270,11 @@ exports[`xy_expression XYChart component it renders stacked bar 1`] = `
style={
Object {
"axisTitle": Object {
"padding": undefined,
"visible": true,
},
"tickLabel": Object {
"padding": undefined,
"rotation": -90,
"visible": false,
},
@ -1430,6 +1460,7 @@ exports[`xy_expression XYChart component it renders stacked horizontal bar 1`] =
"color": undefined,
},
"barSeriesStyle": Object {},
"chartMargins": Object {},
"legend": Object {
"labelOptions": Object {
"maxLines": 0,
@ -1457,9 +1488,11 @@ exports[`xy_expression XYChart component it renders stacked horizontal bar 1`] =
style={
Object {
"axisTitle": Object {
"padding": undefined,
"visible": true,
},
"tickLabel": Object {
"padding": undefined,
"rotation": 0,
"visible": true,
},
@ -1488,9 +1521,11 @@ exports[`xy_expression XYChart component it renders stacked horizontal bar 1`] =
style={
Object {
"axisTitle": Object {
"padding": undefined,
"visible": true,
},
"tickLabel": Object {
"padding": undefined,
"rotation": -90,
"visible": false,
},

View file

@ -59,7 +59,11 @@ import { getAxesConfiguration, GroupsConfiguration, validateExtent } from './axe
import { getColorAssignments } from './color_assignment';
import { getXDomain, XyEndzones } from './x_domain';
import { getLegendAction } from './get_legend_action';
import { ThresholdAnnotations } from './expression_thresholds';
import {
computeChartMargins,
getThresholdRequiredPaddings,
ThresholdAnnotations,
} from './expression_thresholds';
declare global {
interface Window {
@ -314,6 +318,12 @@ export function XYChart({
Boolean(isHistogramViz)
);
const yAxesMap = {
left: yAxesConfiguration.find(({ groupId }) => groupId === 'left'),
right: yAxesConfiguration.find(({ groupId }) => groupId === 'right'),
};
const thresholdPaddings = getThresholdRequiredPaddings(thresholdLayers, yAxesMap);
const getYAxesTitles = (
axisSeries: Array<{ layer: string; accessor: string }>,
groupId: string
@ -330,23 +340,38 @@ export function XYChart({
);
};
const getYAxesStyle = (groupId: string) => {
const getYAxesStyle = (groupId: 'left' | 'right') => {
const tickVisible =
groupId === 'right'
? tickLabelsVisibilitySettings?.yRight
: tickLabelsVisibilitySettings?.yLeft;
const style = {
tickLabel: {
visible:
groupId === 'right'
? tickLabelsVisibilitySettings?.yRight
: tickLabelsVisibilitySettings?.yLeft,
visible: tickVisible,
rotation:
groupId === 'right'
? args.labelsOrientation?.yRight || 0
: args.labelsOrientation?.yLeft || 0,
padding:
thresholdPaddings[groupId] != null
? {
inner: thresholdPaddings[groupId],
}
: undefined,
},
axisTitle: {
visible:
groupId === 'right'
? axisTitlesVisibilitySettings?.yRight
: axisTitlesVisibilitySettings?.yLeft,
// if labels are not visible add the padding to the title
padding:
!tickVisible && thresholdPaddings[groupId] != null
? {
inner: thresholdPaddings[groupId],
}
: undefined,
},
};
return style;
@ -510,6 +535,17 @@ export function XYChart({
legend: {
labelOptions: { maxLines: legend.shouldTruncate ? legend?.maxLines ?? 1 : 0 },
},
// if not title or labels are shown for axes, add some padding if required by threshold markers
chartMargins: {
...chartTheme.chartPaddings,
...computeChartMargins(
thresholdPaddings,
tickLabelsVisibilitySettings,
axisTitlesVisibilitySettings,
yAxesMap,
shouldRotate
),
},
}}
baseTheme={chartBaseTheme}
tooltip={{
@ -545,9 +581,15 @@ export function XYChart({
tickLabel: {
visible: tickLabelsVisibilitySettings?.x,
rotation: labelsOrientation?.x,
padding:
thresholdPaddings.bottom != null ? { inner: thresholdPaddings.bottom } : undefined,
},
axisTitle: {
visible: axisTitlesVisibilitySettings.x,
padding:
!tickLabelsVisibilitySettings?.x && thresholdPaddings.bottom != null
? { inner: thresholdPaddings.bottom }
: undefined,
},
}}
/>
@ -568,7 +610,7 @@ export function XYChart({
}}
hide={filteredLayers[0].hide}
tickFormat={(d) => axis.formatter?.convert(d) || ''}
style={getYAxesStyle(axis.groupId)}
style={getYAxesStyle(axis.groupId as 'left' | 'right')}
domain={getYAxisDomain(axis)}
/>
);
@ -839,10 +881,15 @@ export function XYChart({
syncColors={syncColors}
paletteService={paletteService}
formatters={{
left: yAxesConfiguration.find(({ groupId }) => groupId === 'left')?.formatter,
right: yAxesConfiguration.find(({ groupId }) => groupId === 'right')?.formatter,
left: yAxesMap.left?.formatter,
right: yAxesMap.right?.formatter,
bottom: xAxisFormatter,
}}
axesMap={{
left: Boolean(yAxesMap.left),
right: Boolean(yAxesMap.right),
}}
isHorizontal={shouldRotate}
/>
) : null}
</Chart>

View file

@ -8,25 +8,144 @@
import React from 'react';
import { groupBy } from 'lodash';
import { EuiIcon } from '@elastic/eui';
import { RectAnnotation, AnnotationDomainType, LineAnnotation } from '@elastic/charts';
import { RectAnnotation, AnnotationDomainType, LineAnnotation, Position } from '@elastic/charts';
import type { PaletteRegistry } from 'src/plugins/charts/public';
import type { FieldFormat } from 'src/plugins/field_formats/common';
import { euiLightVars } from '@kbn/ui-shared-deps-src/theme';
import type { LayerArgs } from '../../common/expressions';
import type { LayerArgs, YConfig } from '../../common/expressions';
import type { LensMultiTable } from '../../common/types';
const THRESHOLD_ICON_SIZE = 20;
export const computeChartMargins = (
thresholdPaddings: 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 && thresholdPaddings.bottom) {
const placement = isHorizontal ? mapVerticalToHorizontalPlacement('bottom') : 'bottom';
result[placement] = thresholdPaddings.bottom;
}
if (
thresholdPaddings.left &&
(isHorizontal || (!labelVisibility?.yLeft && !titleVisibility?.yLeft))
) {
const placement = isHorizontal ? mapVerticalToHorizontalPlacement('left') : 'left';
result[placement] = thresholdPaddings.left;
}
if (
thresholdPaddings.right &&
(isHorizontal || !axesMap.right || (!labelVisibility?.yRight && !titleVisibility?.yRight))
) {
const placement = isHorizontal ? mapVerticalToHorizontalPlacement('right') : 'right';
result[placement] = thresholdPaddings.right;
}
// there's no top axis, so just check if a margin has been computed
if (thresholdPaddings.top) {
const placement = isHorizontal ? mapVerticalToHorizontalPlacement('top') : 'top';
result[placement] = thresholdPaddings.top;
}
return result;
};
function hasIcon(icon: string | undefined): icon is string {
return icon != null && icon !== 'none';
}
// Note: it does not take into consideration whether the threshold is in view or not
export const getThresholdRequiredPaddings = (
thresholdLayers: LayerArgs[],
axesMap: Record<'left' | 'right', unknown>
) => {
const positions = Object.keys(Position);
return thresholdLayers.reduce((memo, layer) => {
if (positions.some((pos) => !(pos in memo))) {
layer.yConfig?.forEach(({ axisMode, icon, iconPosition }) => {
if (axisMode && hasIcon(icon)) {
const placement = getBaseIconPlacement(iconPosition, axisMode, axesMap);
memo[placement] = THRESHOLD_ICON_SIZE;
}
});
}
return memo;
}, {} as Partial<Record<Position, number>>);
};
function mapVerticalToHorizontalPlacement(placement: Position) {
switch (placement) {
case Position.Top:
return Position.Right;
case Position.Bottom:
return Position.Left;
case Position.Left:
return Position.Bottom;
case Position.Right:
return Position.Top;
}
}
// if there's just one axis, put it on the other one
// otherwise use the same axis
// this function assume the chart is vertical
function getBaseIconPlacement(
iconPosition: YConfig['iconPosition'],
axisMode: YConfig['axisMode'],
axesMap: Record<string, unknown>
) {
if (iconPosition === 'auto') {
if (axisMode === 'bottom') {
return Position.Top;
}
if (axisMode === 'left') {
return axesMap.right ? Position.Left : Position.Right;
}
return axesMap.left ? Position.Right : Position.Left;
}
if (iconPosition === 'left') {
return Position.Left;
}
if (iconPosition === 'right') {
return Position.Right;
}
if (iconPosition === 'below') {
return Position.Bottom;
}
return Position.Top;
}
function getIconPlacement(
iconPosition: YConfig['iconPosition'],
axisMode: YConfig['axisMode'],
axesMap: Record<string, unknown>,
isHorizontal: boolean
) {
const vPosition = getBaseIconPlacement(iconPosition, axisMode, axesMap);
if (isHorizontal) {
return mapVerticalToHorizontalPlacement(vPosition);
}
return vPosition;
}
export const ThresholdAnnotations = ({
thresholdLayers,
data,
formatters,
paletteService,
syncColors,
axesMap,
isHorizontal,
}: {
thresholdLayers: LayerArgs[];
data: LensMultiTable;
formatters: Record<'left' | 'right' | 'bottom', FieldFormat | undefined>;
paletteService: PaletteRegistry;
syncColors: boolean;
axesMap: Record<'left' | 'right', boolean>;
isHorizontal: boolean;
}) => {
return (
<>
@ -63,7 +182,13 @@ export const ThresholdAnnotations = ({
const props = {
groupId,
marker: yConfig.icon ? <EuiIcon type={yConfig.icon} /> : undefined,
marker: hasIcon(yConfig.icon) ? <EuiIcon type={yConfig.icon} /> : undefined,
markerPosition: getIconPlacement(
yConfig.iconPosition,
yConfig.axisMode,
axesMap,
isHorizontal
),
};
const annotations = [];

View file

@ -340,6 +340,7 @@ export const buildExpression = (
lineWidth: [yConfig.lineWidth || 1],
fill: [yConfig.fill || 'none'],
icon: yConfig.icon ? [yConfig.icon] : [],
iconPosition: [yConfig.iconPosition || 'auto'],
},
},
],

View file

@ -565,7 +565,7 @@ export function DimensionEditor(
}
if (layer.layerType === 'threshold') {
return <ThresholdPanel {...props} />;
return <ThresholdPanel {...props} isHorizontal={isHorizontal} />;
}
return (

View file

@ -14,11 +14,11 @@ import type { VisualizationDimensionEditorProps } from '../../types';
import { State, XYState } from '../types';
import { FormatFactory } from '../../../common';
import { YConfig } from '../../../common/expressions';
import { LineStyle, FillStyle } from '../../../common/expressions/xy_chart';
import { LineStyle, FillStyle, IconPosition } from '../../../common/expressions/xy_chart';
import { ColorPicker } from './color_picker';
import { updateLayer, idPrefix } from '.';
import { useDebouncedValue } from '../../shared_components';
import { TooltipWrapper, useDebouncedValue } from '../../shared_components';
const icons = [
{
@ -109,13 +109,82 @@ const IconSelect = ({
);
};
function getIconPositionOptions({
isHorizontal,
axisMode,
}: {
isHorizontal: boolean;
axisMode: YConfig['axisMode'];
}) {
const options = [
{
id: `${idPrefix}auto`,
label: i18n.translate('xpack.lens.xyChart.thresholdMarker.auto', {
defaultMessage: 'Auto',
}),
'data-test-subj': 'lnsXY_markerPosition_auto',
},
];
const topLabel = i18n.translate('xpack.lens.xyChart.markerPosition.above', {
defaultMessage: 'Top',
});
const bottomLabel = i18n.translate('xpack.lens.xyChart.markerPosition.below', {
defaultMessage: 'Bottom',
});
const leftLabel = i18n.translate('xpack.lens.xyChart.markerPosition.left', {
defaultMessage: 'Left',
});
const rightLabel = i18n.translate('xpack.lens.xyChart.markerPosition.right', {
defaultMessage: 'Right',
});
if (axisMode === 'bottom') {
const bottomOptions = [
{
id: `${idPrefix}above`,
label: isHorizontal ? rightLabel : topLabel,
'data-test-subj': 'lnsXY_markerPosition_above',
},
{
id: `${idPrefix}below`,
label: isHorizontal ? leftLabel : bottomLabel,
'data-test-subj': 'lnsXY_markerPosition_below',
},
];
if (isHorizontal) {
// above -> below
// left -> right
bottomOptions.reverse();
}
return [...options, ...bottomOptions];
}
const yOptions = [
{
id: `${idPrefix}left`,
label: isHorizontal ? bottomLabel : leftLabel,
'data-test-subj': 'lnsXY_markerPosition_left',
},
{
id: `${idPrefix}right`,
label: isHorizontal ? topLabel : rightLabel,
'data-test-subj': 'lnsXY_markerPosition_right',
},
];
if (isHorizontal) {
// left -> right
// above -> below
yOptions.reverse();
}
return [...options, ...yOptions];
}
export const ThresholdPanel = (
props: VisualizationDimensionEditorProps<State> & {
formatFactory: FormatFactory;
paletteService: PaletteRegistry;
isHorizontal: boolean;
}
) => {
const { state, setState, layerId, accessor } = props;
const { state, setState, layerId, accessor, isHorizontal } = props;
const { inputValue: localState, handleInputChange: setLocalState } = useDebouncedValue<XYState>({
value: state,
@ -265,7 +334,7 @@ export const ThresholdPanel = (
<EuiFormRow
display="columnCompressed"
fullWidth
label={i18n.translate('xpack.lens.xyChart.axisSide.icon', {
label={i18n.translate('xpack.lens.xyChart.thresholdMarker.icon', {
defaultMessage: 'Icon',
})}
>
@ -276,6 +345,44 @@ export const ThresholdPanel = (
}}
/>
</EuiFormRow>
<EuiFormRow
display="columnCompressed"
fullWidth
isDisabled={currentYConfig?.icon == null || currentYConfig?.icon === 'none'}
label={i18n.translate('xpack.lens.xyChart.thresholdMarker.position', {
defaultMessage: 'Icon position',
})}
>
<TooltipWrapper
tooltipContent={i18n.translate('xpack.lens.thresholdMarker.positionRequirementTooltip', {
defaultMessage: 'You must select an icon in order to alter its position',
})}
condition={currentYConfig?.icon == null || currentYConfig?.icon === 'none'}
position="top"
delay="regular"
display="block"
>
<EuiButtonGroup
isFullWidth
legend={i18n.translate('xpack.lens.xyChart.thresholdMarker.position', {
defaultMessage: 'Icon position',
})}
data-test-subj="lnsXY_markerPosition_threshold"
name="markerPosition"
isDisabled={currentYConfig?.icon == null || currentYConfig?.icon === 'none'}
buttonSize="compressed"
options={getIconPositionOptions({
isHorizontal,
axisMode: currentYConfig!.axisMode,
})}
idSelected={`${idPrefix}${currentYConfig?.iconPosition || 'auto'}`}
onChange={(id) => {
const newMode = id.replace(idPrefix, '') as IconPosition;
setYConfig({ forAccessor: accessor, iconPosition: newMode });
}}
/>
</TooltipWrapper>
</EuiFormRow>
</>
);
};