[Lens] Threshold: add padding to avoid axis label collision with thre…

…shold 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 <[email protected]> Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Michael Marcialis <[email protected]>
elastic · Oct 4, 2021 · 8e25f5c · 8e25f5c
1 parent 9b7b322
commit 8e25f5c
Show file tree

Hide file tree

Showing 8 changed files with 340 additions and 17 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
@@ -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'],

diff --git a/x-pack/plugins/lens/public/shared_components/tooltip_wrapper.tsx b/x-pack/plugins/lens/public/shared_components/tooltip_wrapper.tsx
@@ -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;
 };
 

diff --git a/x-pack/plugins/lens/public/xy_visualization/__snapshots__/expression.test.tsx.snap b/x-pack/plugins/lens/public/xy_visualization/__snapshots__/expression.test.tsx.snap
diff --git a/x-pack/plugins/lens/public/xy_visualization/expression.tsx b/x-pack/plugins/lens/public/xy_visualization/expression.tsx
@@ -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>