[Lens] Thresholds: Add text to markers body (elastic#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>
stephmilovic · Oct 11, 2021 · 1bf09e6 · 1bf09e6
1 parent 9d2c536
commit 1bf09e6
Show file tree

Hide file tree

Showing 8 changed files with 248 additions and 109 deletions.
diff --git 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'],

diff --git 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 }
                 );
               }}
             />

diff --git 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: {

diff --git 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>

diff --git 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%;
+    }
+  }
+}
diff --git 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);
+function getMarkerBody(label: string | undefined, isHorizontal: boolean) {
+  if (!label) {
+    return;
+  }
   if (isHorizontal) {
-    return mapVerticalToHorizontalPlacement(vPosition);
+    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 = [];