fix: outside rect annotation placement and group relations (#2471)

elastic · Jun 24, 2024 · d46fb41 · d46fb41
1 parent ae16815
commit d46fb41
Show file tree

Hide file tree

Showing 9 changed files with 180 additions and 66 deletions.
diff --git a/...nnotations/should-render-outside-annotations-with-no-group-ids-chrome-linux.png b/...nnotations/should-render-outside-annotations-with-no-group-ids-chrome-linux.png
diff --git a/e2e/tests/annotations_stories.test.ts b/e2e/tests/annotations_stories.test.ts
@@ -195,5 +195,11 @@ test.describe('Annotations stories', () => {
         'http://localhost:9001/?path=/story/annotations-rects--outside&knob-debug=&knob-chartRotation=0&knob-Tick size=10&knob-Hide all axes=true&knob-Domain axis_Annotations=x&knob-Render outside chart_Annotations=true&knob-Outside dimension_Annotations=5&knob-Red groupId_Annotations=primary&knob-Blue groupId_Annotations=secondary',
       );
     });
+
+    test('should render outside annotations with no groupIds', async ({ page }) => {
+      await common.expectChartAtUrlToMatchScreenshot(page)(
+        'http://localhost:9001/?path=/story/annotations-rects--outside&globals=toggles.showHeader:true;toggles.showChartTitle:false;toggles.showChartDescription:false;toggles.showChartBoundary:false;theme:light&knob-disable isolated point styles=true&knob-isolatedPoint.stroke - series level=orange&knob-isolatedPoint.stroke - theme level=green&knob-point.stroke - series level=blue&knob-point.stroke - theme level=red&knob-series type=line&knob-stroke - pointStyleAccessor=black&knob-use series iso overrides=true&knob-use series overrides=true&knob-use groupIds_Annotations=false&knob-debug=&knob-chartRotation=0&knob-Tick size=10&knob-Hide all axes=&knob-Domain axis_Annotations=x&knob-Render outside chart_Annotations=true&knob-Outside dimension_Annotations=5&knob-Red groupId_Annotations=primary&knob-Blue groupId_Annotations=secondary',
+      );
+    });
   });
 });
diff --git a/package.json b/package.json
@@ -36,7 +36,7 @@
     "lint:fix:it": "yarn lint:it --fix",
     "prettier:check": "prettier --check \"**/*.{json,html,css,scss}\"",
     "prettier:fix": "prettier --w \"**/*.{json,html,css,scss}\"",
-    "playground": "cd playground && RNG_SEED='elastic-charts' webpack serve",
+    "playground": "export NODE_OPTIONS=--openssl-legacy-provider; cd playground && RNG_SEED='elastic-charts' webpack serve",
     "pq": "pretty-quick",
     "semantic-release": "semantic-release --debug",
     "start": "yarn storybook",

diff --git a/packages/charts/src/chart_types/xy_chart/annotations/rect/dimensions.ts b/packages/charts/src/chart_types/xy_chart/annotations/rect/dimensions.ts
@@ -8,14 +8,17 @@
 
 import { AnnotationRectProps } from './types';
 import { getPanelSize, SmallMultipleScales } from '../../../../common/panel_utils';
+import { Rect } from '../../../../geoms/types';
 import { ScaleBand, ScaleContinuous } from '../../../../scales';
 import { isBandScale, isContinuousScale } from '../../../../scales/types';
 import { isDefined, isNil, Position, Rotation } from '../../../../utils/common';
+import { Size } from '../../../../utils/dimensions';
 import { AxisId, GroupId } from '../../../../utils/ids';
+import { Logger } from '../../../../utils/logger';
 import { Point } from '../../../../utils/point';
 import { AxisStyle } from '../../../../utils/themes/theme';
 import { PrimitiveValue } from '../../../partition_chart/layout/utils/group_by_rollup';
-import { isHorizontalRotation, isVerticalRotation } from '../../state/utils/common';
+import { isHorizontalRotation } from '../../state/utils/common';
 import { getAxesSpecForSpecId } from '../../state/utils/spec';
 import { AxisSpec, RectAnnotationDatum, RectAnnotationSpec } from '../../utils/specs';
 import { Bounds } from '../types';
@@ -40,7 +43,7 @@ export function computeRectAnnotationDimensions(
   isHistogram: boolean = false,
 ): AnnotationRectProps[] | null {
   const { dataValues, groupId, outside, id: annotationSpecId } = annotationSpec;
-  const { xAxis, yAxis } = getAxesSpecForSpecId(axesSpecs, groupId);
+  const { xAxis, yAxis } = getAxesSpecForSpecId(axesSpecs, groupId, chartRotation);
   const yScale = yScales.get(groupId);
   const rectsProps: Omit<AnnotationRectProps, 'id' | 'panel'>[] = [];
   const panelSize = getPanelSize(smallMultiplesScales);
@@ -72,25 +75,19 @@ export function computeRectAnnotationDimensions(
       return;
     }
 
+    const hasYValues = isDefined(initialY0) || isDefined(initialY1);
+    const outsideDim = annotationSpec.outsideDimension ?? getOutsideDimension(getAxisStyle(xAxis?.id ?? yAxis?.id));
+
     if (!yScale) {
-      if (!isDefined(initialY0) && !isDefined(initialY1)) {
-        const isLeftSide =
-          (chartRotation === 0 && xAxis?.position === Position.Bottom) ||
-          (chartRotation === 180 && xAxis?.position === Position.Top) ||
-          (chartRotation === -90 && yAxis?.position === Position.Right) ||
-          (chartRotation === 90 && yAxis?.position === Position.Left);
-        const orthoDimension = isHorizontalRotation(chartRotation) ? panelSize.height : panelSize.width;
-        const outsideDim = annotationSpec.outsideDimension ?? getOutsideDimension(getAxisStyle(xAxis?.id ?? yAxis?.id));
-        const rectDimensions = {
+      if (!hasYValues) {
+        // only x values present, just fill full height of chart space
+        const rectDimensions: Rect = {
           ...xAndWidth,
           ...(outside
-            ? {
-                y: isLeftSide ? orthoDimension : -outsideDim,
-                height: outsideDim,
-              }
+            ? getXOutsideAnnotationDimensions(panelSize, chartRotation, xAxis?.position ?? 'bottom', outsideDim)
             : {
                 y: 0,
-                height: orthoDimension,
+                height: isHorizontalRotation(chartRotation) ? panelSize.height : panelSize.width,
               }),
         };
         rectsProps.push({
@@ -99,10 +96,33 @@ export function computeRectAnnotationDimensions(
           datum,
         });
       }
-      return;
+      return; // cannot scale y values without a scale
+    }
+
+    const hasXValues = isDefined(initialX0) || isDefined(initialX1);
+
+    if (outside) {
+      if (hasXValues && hasYValues) {
+        Logger.warn(
+          `The RectAnnotation (${annotationSpecId}) was defined as outside but has both x and y values defined.`,
+        );
+      } else if (hasXValues) {
+        const rectDimensions: Rect = {
+          ...xAndWidth,
+          ...getXOutsideAnnotationDimensions(panelSize, chartRotation, xAxis?.position ?? 'bottom', outsideDim),
+        };
+
+        rectsProps.push({
+          specId: annotationSpecId,
+          rect: rectDimensions,
+          datum,
+        });
+        return;
+      }
     }
 
     const [y0, y1] = limitValueToDomainRange(yScale, initialY0, initialY1);
+
     // something is wrong with the data types, don't draw this annotation
     if (!Number.isFinite(y0) || !Number.isFinite(y1)) return;
 
@@ -111,29 +131,21 @@ export function computeRectAnnotationDimensions(
     if (Number.isNaN(scaledY1) || Number.isNaN(scaledY0)) return;
 
     height = Math.abs(scaledY0 - scaledY1);
-    // if the annotation height is 0 override it with the height from chart dimension and if the values in the domain are the same
+
+    // if the annotation height is 0, override it with the height from chart dimension and if the values in the domain are the same
     if (height === 0 && yScale.domain.length === 2 && yScale.domain[0] === yScale.domain[1]) {
       // eslint-disable-next-line prefer-destructuring
       height = panelSize.height;
       scaledY1 = 0;
     }
 
-    const orthoDimension = isVerticalRotation(chartRotation) ? panelSize.height : panelSize.width;
-    const isLeftSide =
-      (chartRotation === 0 && yAxis?.position === Position.Left) ||
-      (chartRotation === 180 && yAxis?.position === Position.Right) ||
-      (chartRotation === -90 && xAxis?.position === Position.Bottom) ||
-      (chartRotation === 90 && xAxis?.position === Position.Top);
-    const outsideDim = annotationSpec.outsideDimension ?? getOutsideDimension(getAxisStyle(xAxis?.id ?? yAxis?.id));
-    const rectDimensions = {
-      ...(!isDefined(initialX0) && !isDefined(initialX1) && outside
-        ? {
-            x: isLeftSide ? -outsideDim : orthoDimension,
-            width: outsideDim,
-          }
-        : xAndWidth),
+    const rectDimensions: Rect = {
+      ...xAndWidth,
       y: scaledY1,
       height,
+      ...(outside &&
+        !(hasXValues && hasYValues) &&
+        getYOutsideAnnotationDimensions(panelSize, chartRotation, yAxis?.position ?? 'left', outsideDim)),
     };
 
     rectsProps.push({
@@ -244,7 +256,7 @@ function maxOf(base: number, value: number | string | null | undefined): number
 }
 
 function getOutsideDimension({ tickLine: { visible, size } }: AxisStyle): number {
-  return visible ? size : 0;
+  return visible ? size : 1;
 }
 
 /**
@@ -259,3 +271,69 @@ export function getAnnotationRectPropsId(
 ) {
   return [specId, verticalValue, horizontalValue, ...Object.values(datum.coordinates), datum.details, index].join('__');
 }
+
+function getXOutsideAnnotationDimensions(
+  panelSize: Size,
+  rotation: Rotation,
+  axisPosition: Position,
+  thickness: number,
+): Pick<Rect, 'y' | 'height'> {
+  const { height, width } = panelSize;
+
+  switch (axisPosition) {
+    case Position.Top:
+      return {
+        y: rotation === 180 ? height : rotation === 90 ? width : -thickness,
+        height: thickness,
+      };
+    case Position.Bottom:
+      return {
+        y: rotation === 0 ? height : rotation === 90 ? width : -thickness,
+        height: thickness,
+      };
+    case Position.Left:
+      return {
+        y: rotation === -90 ? -thickness : width,
+        height: thickness,
+      };
+    case Position.Right:
+    default:
+      return {
+        y: rotation === -90 ? width : -thickness,
+        height: thickness,
+      };
+  }
+}
+
+function getYOutsideAnnotationDimensions(
+  panelSize: Size,
+  rotation: Rotation,
+  axisPosition: Position,
+  thickness: number,
+): Pick<Rect, 'x' | 'width'> {
+  const { height, width } = panelSize;
+
+  switch (axisPosition) {
+    case Position.Left:
+      return {
+        x: rotation === 180 ? width : rotation === 90 ? height : -thickness,
+        width: thickness,
+      };
+    case Position.Right:
+      return {
+        x: rotation === 0 ? width : rotation === 90 ? height : -thickness,
+        width: thickness,
+      };
+    case Position.Top:
+      return {
+        x: rotation === -90 ? height : -thickness,
+        width: thickness,
+      };
+    case Position.Bottom:
+    default:
+      return {
+        x: rotation === -90 ? -thickness : height,
+        width: thickness,
+      };
+  }
+}
diff --git a/packages/charts/src/chart_types/xy_chart/state/utils/common.ts b/packages/charts/src/chart_types/xy_chart/state/utils/common.ts
@@ -26,6 +26,7 @@ export function isHorizontalRotation(chartRotation: Rotation) {
 export function isVerticalRotation(chartRotation: Rotation) {
   return chartRotation === -90 || chartRotation === 90;
 }
+
 /**
  * Check if a specs map contains only line or area specs
  * @param specs Map<SpecId, BasicSeriesSpec>

diff --git a/packages/charts/src/chart_types/xy_chart/state/utils/spec.ts b/packages/charts/src/chart_types/xy_chart/state/utils/spec.ts
@@ -18,7 +18,7 @@ export function getSpecsById<T extends Spec>(specs: T[], id: string): T | undefi
 }
 
 /** @internal */
-export function getAxesSpecForSpecId(axesSpecs: AxisSpec[], groupId: GroupId, chartRotation: Rotation = 0) {
+export function getAxesSpecForSpecId(axesSpecs: AxisSpec[], groupId: GroupId, chartRotation: Rotation) {
   return axesSpecs.reduce<{ xAxis?: AxisSpec; yAxis?: AxisSpec }>((result, spec) => {
     if (spec.groupId === groupId && isXDomain(spec.position, chartRotation)) result.xAxis = spec;
     else if (spec.groupId === groupId && !isXDomain(spec.position, chartRotation)) result.yAxis = spec;

diff --git a/storybook/stories/annotations/rects/6_zero_domain.story.tsx b/storybook/stories/annotations/rects/6_zero_domain.story.tsx
@@ -35,10 +35,17 @@ export const Example: ChartsStory = (_, { title, description }) => {
   const xAxisKnobs = getKnobs();
   // only show the fit enable or disable if relevant
   const fit = xAxisKnobs.minY === xAxisKnobs.maxY ? boolean('fit to the domain', false) : undefined;
+  const outside = boolean('render outside', false);
 
   return (
     <Chart title={title} description={description}>
-      <RectAnnotation id="rect" dataValues={[{ coordinates: xAxisKnobs }]} style={{ fill: 'red' }} />
+      <RectAnnotation
+        id="rect"
+        dataValues={[{ coordinates: xAxisKnobs, details: 'My Annotation' }]}
+        style={{ fill: 'red' }}
+        outside={outside}
+        outsideDimension={4}
+      />
       <Settings baseTheme={useBaseTheme()} />
       <Axis id="bottom" position={Position.Bottom} title="x-domain axis" />
       <Axis

diff --git a/storybook/stories/annotations/rects/7_with_group_id.story.tsx b/storybook/stories/annotations/rects/7_with_group_id.story.tsx
@@ -14,6 +14,7 @@ import { Position } from '@elastic/charts/src/utils/common';
 
 import { ChartsStory } from '../../../types';
 import { useBaseTheme } from '../../../use_base_theme';
+import { customKnobs } from '../../utils/knobs';
 
 const getKnobs = () => {
   const enabled = boolean('enable annotation', true);
@@ -35,6 +36,7 @@ const getKnobs = () => {
     x1,
     y0: yDefined ? number('y0', 0) : undefined,
     y1: yDefined ? number('y1', 3) : undefined,
+    outside: boolean('outside', false),
   };
 };
 export const Example: ChartsStory = (_, { title, description }) => {
@@ -48,9 +50,11 @@ export const Example: ChartsStory = (_, { title, description }) => {
           id="x axis"
           dataValues={[{ coordinates: xAxisKnobs }]}
           style={{ fill: 'red' }}
+          outside={xAxisKnobs.outside}
+          outsideDimension={5}
         />
       )}
-      <Settings baseTheme={useBaseTheme()} />
+      <Settings baseTheme={useBaseTheme()} rotation={customKnobs.enum.rotation()} />
       <Axis id="bottom" groupId="group2" position={Position.Bottom} title="Bottom (groupId=group2)" />
       <Axis id="left" groupId="group1" position={Position.Left} title="Left (groupId=group1)" />
       <Axis id="top" groupId="group1" position={Position.Top} title="Top (groupId=group1)" />