[Lens] Thresholds: Add text to markers body (#113629)

* 🐛 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 * ✨ First text on marker implementation * 🐛 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> * 🐛 Fix minor details * 💄 Small tweak * ✨ Reduce the padding if no icon is shown on the axis * ✅ Fix broken unit tests * 💄 Fix vertical text centering * 🚨 Fix linting issue * 🐛 Fix issue * 💄 Reorder panel inputs * 💄 Move styling to sass * 👌 Address feedback Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Michael Marcialis <michael@marcial.is>
2025-04-23 09:19:04 -04:00 · 2021-10-11 12:32:19 +02:00 · 2021-10-11 12:32:19 +02:00 · 1bf09e6930
commit 1bf09e6930
parent 9d2c536ccb
8 changed files with 249 additions and 110 deletions
--- a/x-pack/plugins/lens/common/expressions/xy_chart/axis_config.ts
+++ b/x-pack/plugins/lens/common/expressions/xy_chart/axis_config.ts
@ -41,6 +41,7 @@ export interface YConfig {
  lineStyle?: LineStyle;
  fill?: FillStyle;
  iconPosition?: IconPosition;
+  textVisibility?: boolean;
 }

 export type AxisTitlesVisibilityConfigResult = AxesSettingsConfig & {
@ -187,6 +188,10 @@ export const yAxisConfig: ExpressionFunctionDefinition<
      options: ['auto', 'above', 'below', 'left', 'right'],
      help: 'The placement of the icon for the threshold line',
    },
+    textVisibility: {
+      types: ['boolean'],
+      help: 'Visibility of the label on the threshold line',
+    },
    fill: {
      types: ['string'],
      options: ['none', 'above', 'below'],
--- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx
+++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx
@ -107,16 +107,19 @@ export function DimensionEditor(props: DimensionEditorProps) {
  );

  const setStateWrapper = (
-    setter: IndexPatternLayer | ((prevLayer: IndexPatternLayer) => IndexPatternLayer)
+    setter: IndexPatternLayer | ((prevLayer: IndexPatternLayer) => IndexPatternLayer),
+    options: { forceRender?: boolean } = {}
  ) => {
    const hypotheticalLayer = typeof setter === 'function' ? setter(state.layers[layerId]) : setter;
+    const isDimensionComplete = Boolean(hypotheticalLayer.columns[columnId]);
    setState(
      (prevState) => {
        const layer = typeof setter === 'function' ? setter(prevState.layers[layerId]) : setter;
        return mergeLayer({ state: prevState, layerId, newLayer: layer });
      },
      {
-        isDimensionComplete: Boolean(hypotheticalLayer.columns[columnId]),
+        isDimensionComplete,
+        ...options,
      }
    );
  };
@ -169,20 +172,8 @@ export function DimensionEditor(props: DimensionEditorProps) {
  ) => {
    if (temporaryStaticValue) {
      setTemporaryState('none');
-      if (typeof setter === 'function') {
-        return setState(
-          (prevState) => {
-            const layer = setter(addStaticValueColumn(prevState.layers[layerId]));
-            return mergeLayer({ state: prevState, layerId, newLayer: layer });
-          },
-          {
-            isDimensionComplete: true,
-            forceRender: true,
-          }
-        );
-      }
    }
-    return setStateWrapper(setter);
+    return setStateWrapper(setter, { forceRender: true });
  };

  const ParamEditor = getParamEditor(
@ -314,7 +305,7 @@ export function DimensionEditor(props: DimensionEditorProps) {
              temporaryQuickFunction &&
              isQuickFunction(newLayer.columns[columnId].operationType)
            ) {
-              // Only switch the tab once the formula is fully removed
+              // Only switch the tab once the "non quick function" is fully removed
              setTemporaryState('none');
            }
            setStateWrapper(newLayer);
@ -344,13 +335,12 @@ export function DimensionEditor(props: DimensionEditorProps) {
                visualizationGroups: dimensionGroups,
                targetGroup: props.groupId,
              });
-              // );
            }
            if (
              temporaryQuickFunction &&
              isQuickFunction(newLayer.columns[columnId].operationType)
            ) {
-              // Only switch the tab once the formula is fully removed
+              // Only switch the tab once the "non quick function" is fully removed
              setTemporaryState('none');
            }
            setStateWrapper(newLayer);
@ -508,6 +498,9 @@ export function DimensionEditor(props: DimensionEditorProps) {
              }
              incompleteOperation={incompleteOperation}
              onChoose={(choice) => {
+                if (temporaryQuickFunction) {
+                  setTemporaryState('none');
+                }
                setStateWrapper(
                  insertOrReplaceColumn({
                    layer: state.layers[layerId],
@ -518,7 +511,8 @@ export function DimensionEditor(props: DimensionEditorProps) {
                    visualizationGroups: dimensionGroups,
                    targetGroup: props.groupId,
                    incompleteParams,
-                  })
+                  }),
+                  { forceRender: temporaryQuickFunction }
                );
              }}
            />
--- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx
+++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx
@ -513,7 +513,10 @@ describe('IndexPatternDimensionEditorPanel', () => {
      comboBox.prop('onChange')!([option]);
    });

-    expect(setState.mock.calls[0]).toEqual([expect.any(Function), { isDimensionComplete: true }]);
+    expect(setState.mock.calls[0]).toEqual([
+      expect.any(Function),
+      { isDimensionComplete: true, forceRender: false },
+    ]);
    expect(setState.mock.calls[0][0](defaultProps.state)).toEqual({
      ...initialState,
      layers: {
@ -545,7 +548,10 @@ describe('IndexPatternDimensionEditorPanel', () => {
      comboBox.prop('onChange')!([option]);
    });

-    expect(setState.mock.calls[0]).toEqual([expect.any(Function), { isDimensionComplete: true }]);
+    expect(setState.mock.calls[0]).toEqual([
+      expect.any(Function),
+      { isDimensionComplete: true, forceRender: false },
+    ]);
    expect(setState.mock.calls[0][0](defaultProps.state)).toEqual({
      ...state,
      layers: {
@ -1037,7 +1043,10 @@ describe('IndexPatternDimensionEditorPanel', () => {
      });

      expect(setState.mock.calls.length).toEqual(2);
-      expect(setState.mock.calls[1]).toEqual([expect.any(Function), { isDimensionComplete: true }]);
+      expect(setState.mock.calls[1]).toEqual([
+        expect.any(Function),
+        { isDimensionComplete: true, forceRender: false },
+      ]);
      expect(setState.mock.calls[1][0](state)).toEqual({
        ...state,
        layers: {
@ -1921,7 +1930,10 @@ describe('IndexPatternDimensionEditorPanel', () => {
      comboBox.prop('onChange')!([option]);
    });

-    expect(setState.mock.calls[0]).toEqual([expect.any(Function), { isDimensionComplete: true }]);
+    expect(setState.mock.calls[0]).toEqual([
+      expect.any(Function),
+      { isDimensionComplete: true, forceRender: false },
+    ]);
    expect(setState.mock.calls[0][0](defaultProps.state)).toEqual({
      ...state,
      layers: {
--- a/x-pack/plugins/lens/public/xy_visualization/expression.tsx
+++ b/x-pack/plugins/lens/public/xy_visualization/expression.tsx
@ -924,6 +924,7 @@ export function XYChart({
            right: Boolean(yAxesMap.right),
          }}
          isHorizontal={shouldRotate}
+          thresholdPaddingMap={thresholdPaddings}
        />
      ) : null}
    </Chart>
--- a/x-pack/plugins/lens/public/xy_visualization/expression_thresholds.scss
+++ b/x-pack/plugins/lens/public/xy_visualization/expression_thresholds.scss
@ -0,0 +1,18 @@
+.lnsXyDecorationRotatedWrapper {
+  display: inline-block;
+  overflow: hidden;
+  line-height: $euiLineHeight;
+
+  .lnsXyDecorationRotatedWrapper__label {
+    display: inline-block;
+    white-space: nowrap;
+    transform: translate(0, 100%) rotate(-90deg);
+    transform-origin: 0 0;
+
+    &::after {
+      content: '';
+      float: left;
+      margin-top: 100%;
+    }
+  }
+}
--- a/x-pack/plugins/lens/public/xy_visualization/expression_thresholds.tsx
+++ b/x-pack/plugins/lens/public/xy_visualization/expression_thresholds.tsx
@ -5,6 +5,7 @@
 * 2.0.
 */

+import './expression_thresholds.scss';
 import React from 'react';
 import { groupBy } from 'lodash';
 import { EuiIcon } from '@elastic/eui';
@ -14,8 +15,9 @@ import type { FieldFormat } from 'src/plugins/field_formats/common';
 import { euiLightVars } from '@kbn/ui-shared-deps-src/theme';
 import type { LayerArgs, YConfig } from '../../common/expressions';
 import type { LensMultiTable } from '../../common/types';
+import { hasIcon } from './xy_config_panel/threshold_panel';

-const THRESHOLD_ICON_SIZE = 20;
+const THRESHOLD_MARKER_SIZE = 20;

 export const computeChartMargins = (
  thresholdPaddings: Partial<Record<Position, number>>,
@ -51,27 +53,35 @@ export const computeChartMargins = (
  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;
-        }
-      });
+  // collect all paddings for the 4 axis: if any text is detected double it.
+  const paddings: Partial<Record<Position, number>> = {};
+  const icons: Partial<Record<Position, number>> = {};
+  thresholdLayers.forEach((layer) => {
+    layer.yConfig?.forEach(({ axisMode, icon, iconPosition, textVisibility }) => {
+      if (axisMode && (hasIcon(icon) || textVisibility)) {
+        const placement = getBaseIconPlacement(iconPosition, axisMode, axesMap);
+        paddings[placement] = Math.max(
+          paddings[placement] || 0,
+          THRESHOLD_MARKER_SIZE * (textVisibility ? 2 : 1) // double the padding size if there's text
+        );
+        icons[placement] = (icons[placement] || 0) + (hasIcon(icon) ? 1 : 0);
+      }
+    });
+  });
+  // post-process the padding based on the icon presence:
+  // if no icon is present for the placement, just reduce the padding
+  (Object.keys(paddings) as Position[]).forEach((placement) => {
+    if (!icons[placement]) {
+      paddings[placement] = THRESHOLD_MARKER_SIZE;
    }
-    return memo;
-  }, {} as Partial<Record<Position, number>>);
+  });
+
+  return paddings;
 };

 function mapVerticalToHorizontalPlacement(placement: Position) {
@ -117,17 +127,57 @@ function getBaseIconPlacement(
  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);
+function getMarkerBody(label: string | undefined, isHorizontal: boolean) {
+  if (!label) {
+    return;
+  }
+  if (isHorizontal) {
+    return (
+      <div className="eui-textTruncate" style={{ maxWidth: THRESHOLD_MARKER_SIZE * 3 }}>
+        {label}
+      </div>
+    );
+  }
+  return (
+    <div
+      className="lnsXyDecorationRotatedWrapper"
+      style={{
+        width: THRESHOLD_MARKER_SIZE,
+      }}
+    >
+      <div
+        className="eui-textTruncate lnsXyDecorationRotatedWrapper__label"
+        style={{
+          maxWidth: THRESHOLD_MARKER_SIZE * 3,
+        }}
+      >
+        {label}
+      </div>
+    </div>
+  );
+}
+
+function getMarkerToShow(
+  yConfig: YConfig,
+  label: string | undefined,
+  isHorizontal: boolean,
+  hasReducedPadding: boolean
+) {
+  // show an icon if present
+  if (hasIcon(yConfig.icon)) {
+    return <EuiIcon type={yConfig.icon} />;
+  }
+  // if there's some text, check whether to show it as marker, or just show some padding for the icon
+  if (yConfig.textVisibility) {
+    if (hasReducedPadding) {
+      return getMarkerBody(
+        label,
+        (!isHorizontal && yConfig.axisMode === 'bottom') ||
+          (isHorizontal && yConfig.axisMode !== 'bottom')
+      );
+    }
+    return <EuiIcon type="empty" />;
  }
-  return vPosition;
 }

 export const ThresholdAnnotations = ({
@ -138,6 +188,7 @@ export const ThresholdAnnotations = ({
  syncColors,
  axesMap,
  isHorizontal,
+  thresholdPaddingMap,
 }: {
  thresholdLayers: LayerArgs[];
  data: LensMultiTable;
@ -146,6 +197,7 @@ export const ThresholdAnnotations = ({
  syncColors: boolean;
  axesMap: Record<'left' | 'right', boolean>;
  isHorizontal: boolean;
+  thresholdPaddingMap: Partial<Record<Position, number>>;
 }) => {
  return (
    <>
@ -180,15 +232,35 @@ export const ThresholdAnnotations = ({

          const defaultColor = euiLightVars.euiColorDarkShade;

+          // get the position for vertical chart
+          const markerPositionVertical = getBaseIconPlacement(
+            yConfig.iconPosition,
+            yConfig.axisMode,
+            axesMap
+          );
+          // the padding map is built for vertical chart
+          const hasReducedPadding =
+            thresholdPaddingMap[markerPositionVertical] === THRESHOLD_MARKER_SIZE;
+
          const props = {
            groupId,
-            marker: hasIcon(yConfig.icon) ? <EuiIcon type={yConfig.icon} /> : undefined,
-            markerPosition: getIconPlacement(
-              yConfig.iconPosition,
-              yConfig.axisMode,
-              axesMap,
-              isHorizontal
+            marker: getMarkerToShow(
+              yConfig,
+              columnToLabelMap[yConfig.forAccessor],
+              isHorizontal,
+              hasReducedPadding
            ),
+            markerBody: getMarkerBody(
+              yConfig.textVisibility && !hasReducedPadding
+                ? columnToLabelMap[yConfig.forAccessor]
+                : undefined,
+              (!isHorizontal && yConfig.axisMode === 'bottom') ||
+                (isHorizontal && yConfig.axisMode !== 'bottom')
+            ),
+            // rotate the position if required
+            markerPosition: isHorizontal
+              ? mapVerticalToHorizontalPlacement(markerPositionVertical)
+              : markerPositionVertical,
          };
          const annotations = [];

--- a/x-pack/plugins/lens/public/xy_visualization/to_expression.ts
+++ b/x-pack/plugins/lens/public/xy_visualization/to_expression.ts
@ -13,6 +13,7 @@ import { OperationMetadata, DatasourcePublicAPI } from '../types';
 import { getColumnToLabelMap } from './state_helpers';
 import type { ValidLayer, XYLayerConfig } from '../../common/expressions';
 import { layerTypes } from '../../common';
+import { hasIcon } from './xy_config_panel/threshold_panel';
 import { defaultThresholdColor } from './color_assignment';

 export const getSortedAccessors = (datasource: DatasourcePublicAPI, layer: XYLayerConfig) => {
@ -66,6 +67,7 @@ export function toPreviewExpression(
                ...config,
                lineWidth: 1,
                icon: undefined,
+                textVisibility: false,
              })),
            }
      ),
@ -344,8 +346,12 @@ export const buildExpression = (
                                lineStyle: [yConfig.lineStyle || 'solid'],
                                lineWidth: [yConfig.lineWidth || 1],
                                fill: [yConfig.fill || 'none'],
-                                icon: yConfig.icon ? [yConfig.icon] : [],
-                                iconPosition: [yConfig.iconPosition || 'auto'],
+                                icon: hasIcon(yConfig.icon) ? [yConfig.icon] : [],
+                                iconPosition:
+                                  hasIcon(yConfig.icon) || yConfig.textVisibility
+                                    ? [yConfig.iconPosition || 'auto']
+                                    : ['auto'],
+                                textVisibility: [yConfig.textVisibility || false],
                              },
                            },
                          ],
--- a/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/threshold_panel.tsx
+++ b/x-pack/plugins/lens/public/xy_visualization/xy_config_panel/threshold_panel.tsx
@ -8,7 +8,14 @@
 import './xy_config_panel.scss';
 import React, { useCallback, useState } from 'react';
 import { i18n } from '@kbn/i18n';
-import { EuiButtonGroup, EuiComboBox, EuiFormRow, EuiIcon, EuiRange } from '@elastic/eui';
+import {
+  EuiButtonGroup,
+  EuiComboBox,
+  EuiFormRow,
+  EuiIcon,
+  EuiRange,
+  EuiSwitch,
+} from '@elastic/eui';
 import type { PaletteRegistry } from 'src/plugins/charts/public';
 import type { VisualizationDimensionEditorProps } from '../../types';
 import { State, XYState } from '../types';
@ -177,6 +184,10 @@ function getIconPositionOptions({
  return [...options, ...yOptions];
 }

+export function hasIcon(icon: string | undefined): icon is string {
+  return icon != null && icon !== 'none';
+}
+
 export const ThresholdPanel = (
  props: VisualizationDimensionEditorProps<State> & {
    formatFactory: FormatFactory;
@ -220,6 +231,78 @@ export const ThresholdPanel = (

  return (
    <>
+      <EuiFormRow
+        label={i18n.translate('xpack.lens.thresholdMarker.textVisibility', {
+          defaultMessage: 'Show display name',
+        })}
+        display="columnCompressedSwitch"
+      >
+        <EuiSwitch
+          compressed
+          label={i18n.translate('xpack.lens.thresholdMarker.textVisibility', {
+            defaultMessage: 'Show display name',
+          })}
+          showLabel={false}
+          data-test-subj="lns-thresholdMaker-text-visibility"
+          checked={Boolean(currentYConfig?.textVisibility)}
+          onChange={() => {
+            setYConfig({ forAccessor: accessor, textVisibility: !currentYConfig?.textVisibility });
+          }}
+        />
+      </EuiFormRow>
+      <EuiFormRow
+        display="columnCompressed"
+        fullWidth
+        label={i18n.translate('xpack.lens.xyChart.thresholdMarker.icon', {
+          defaultMessage: 'Icon',
+        })}
+      >
+        <IconSelect
+          value={currentYConfig?.icon}
+          onChange={(newIcon) => {
+            setYConfig({ forAccessor: accessor, icon: newIcon });
+          }}
+        />
+      </EuiFormRow>
+      <EuiFormRow
+        display="columnCompressed"
+        fullWidth
+        isDisabled={!hasIcon(currentYConfig?.icon) && !currentYConfig?.textVisibility}
+        label={i18n.translate('xpack.lens.xyChart.thresholdMarker.position', {
+          defaultMessage: 'Decoration position',
+        })}
+      >
+        <TooltipWrapper
+          tooltipContent={i18n.translate('xpack.lens.thresholdMarker.positionRequirementTooltip', {
+            defaultMessage:
+              'You must select an icon or show the display name in order to alter its position',
+          })}
+          condition={!hasIcon(currentYConfig?.icon) && !currentYConfig?.textVisibility}
+          position="top"
+          delay="regular"
+          display="block"
+        >
+          <EuiButtonGroup
+            isFullWidth
+            legend={i18n.translate('xpack.lens.xyChart.thresholdMarker.position', {
+              defaultMessage: 'Decoration position',
+            })}
+            data-test-subj="lnsXY_markerPosition_threshold"
+            name="markerPosition"
+            isDisabled={!hasIcon(currentYConfig?.icon) && !currentYConfig?.textVisibility}
+            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>
      <ColorPicker
        {...props}
        disableHelpTooltip
@ -331,58 +414,6 @@ export const ThresholdPanel = (
          }}
        />
      </EuiFormRow>
-      <EuiFormRow
-        display="columnCompressed"
-        fullWidth
-        label={i18n.translate('xpack.lens.xyChart.thresholdMarker.icon', {
-          defaultMessage: 'Icon',
-        })}
-      >
-        <IconSelect
-          value={currentYConfig?.icon}
-          onChange={(newIcon) => {
-            setYConfig({ forAccessor: accessor, icon: newIcon });
-          }}
-        />
-      </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>
    </>
  );
 };