fix(goal): chart placement and overlap issues (#1620)

* fix goal chart vertical placement
* enforce max angle without overlapping

.eslintrc.js

@@ -468,5 +468,11 @@ module.exports = {
         'promise/no-promise-in-callback': 0,
       },
     },
+    {
+      files: ['./integration/**/*.test.ts?(x)'],
+      rules: {
+        'jest/expect-expect': 0,
+      },
+    },
   ],
 };

integration/tests/goal_stories.test.ts

@@ -70,4 +70,45 @@ describe('Goal stories', () => {
       );
     });
   });
+
+  it('should prevent overlapping angles - clockwise', async () => {
+    await common.expectChartAtUrlToMatchScreenshot(
+      'http://localhost:9001/?path=/story/goal-alpha--full-circle&globals=theme:light&knob-endAngle%20(%CF%80)=-0.625&knob-startAngle%20(%CF%80)=1.5',
+    );
+  });
+
+  it('should prevent overlapping angles - counterclockwise', async () => {
+    await common.expectChartAtUrlToMatchScreenshot(
+      'http://localhost:9001/?path=/story/goal-alpha--full-circle&globals=theme:light&knob-endAngle%20(%CF%80)=1.625&knob-startAngle%20(%CF%80)=-0.5',
+    );
+  });
+
+  describe('sagitta shifted goal charts', () => {
+    it.each<[title: string, startAngle: number, endAngle: number]>([
+      // top openings
+      ['π/8 -> neg π', 1 / 8, -1],
+      ['neg π/8 -> neg π', -1 / 8, -1],
+      ['neg π -> π/8', -1, 1 / 8],
+      ['neg π -> neg π/8', -1, -1 / 8],
+      ['neg π/4 -> neg 3π/4', -1 / 4, -3 / 4],
+      ['neg 3π/4 -> neg π/4', -3 / 4, -1 / 4],
+    ])('should apply correct top shift (%s)', async (_, startAngle, endAngle) => {
+      await common.expectChartAtUrlToMatchScreenshot(
+        `http://localhost:9001/?path=/story/goal-alpha--full-circle&globals=theme:light&knob-startAngle%20(%CF%80)=${startAngle}&knob-endAngle%20(%CF%80)=${endAngle}`,
+      );
+    });
+    it.each<[title: string, startAngle: number, endAngle: number]>([
+      // bottom openings
+      ['π -> 0', 1, 0],
+      ['3π/4 -> 0', 3 / 4, 0],
+      ['neg π/8 -> π', -1 / 8, 1],
+      ['π/8 -> π', 1 / 8, 1],
+      ['3π/4 -> π/4', 3 / 4, 1 / 4],
+      ['π/4 -> 3π/4', 1 / 4, 3 / 4],
+    ])('should apply correct bottom shift (%s)', async (_, startAngle, endAngle) => {
+      await common.expectChartAtUrlToMatchScreenshot(
+        `http://localhost:9001/?path=/story/goal-alpha--full-circle&globals=theme:light&knob-startAngle%20(%CF%80)=${startAngle}&knob-endAngle%20(%CF%80)=${endAngle}`,
+      );
+    });
+  });
 });

packages/charts/api/charts.api.md

@@ -953,8 +953,10 @@ export interface GeometryValue {
 // @public (undocumented)
 export function getNodeName(node: ArrayNode): string;
 
+// Warning: (ae-forgotten-export) The symbol "buildProps" needs to be exported by the entry point index.d.ts
+//
 // @alpha
-export const Goal: FC<SFProps<GoalSpec, "chartType" | "specType", "base" | "actual" | "bands" | "ticks" | "bandFillColor" | "tickValueFormatter" | "labelMajor" | "labelMinor" | "centralMajor" | "centralMinor" | "angleStart" | "angleEnd" | "bandLabels" | "tooltipValueFormatter", "target", "id" | "subtype">>;
+export const Goal: (props: SFProps<GoalSpec, keyof typeof buildProps['overrides'], keyof typeof buildProps['defaults'], keyof typeof buildProps['optionals'], keyof typeof buildProps['requires']>) => null;
 
 // @alpha (undocumented)
 export type GoalLabelAccessor = LabelAccessor<BandFillColorAccessorInput>;

packages/charts/src/chart_types/goal_chart/layout/viewmodel/geoms.ts

@@ -15,7 +15,7 @@ import { Dimensions } from '../../../../utils/dimensions';
 import { Theme } from '../../../../utils/themes/theme';
 import { GoalSubtype } from '../../specs/constants';
 import { BulletViewModel } from '../types/viewmodel_types';
-import { getSagitta, getMinSagitta } from './utils';
+import { getSagitta, getMinSagitta, getTranformDirection } from './utils';
 
 const referenceCircularSizeCap = 360; // goal/gauge won't be bigger even if there's ample room: it'd be a waste of space
 const referenceBulletSizeCap = 500; // goal/gauge won't be bigger even if there's ample room: it'd be a waste of space
@@ -247,8 +247,8 @@ export function geoms(
     labelMinor,
     centralMajor,
     centralMinor,
-    angleStart,
     angleEnd,
+    angleStart,
   } = bulletViewModel;
 
   const circular = subtype === GoalSubtype.Goal;
@@ -395,7 +395,8 @@ export function geoms(
   if (circular) {
     const sagitta = getMinSagitta(angleStart, angleEnd, r);
     const maxSagitta = getSagitta((3 / 2) * Math.PI, r);
-    data.yOffset.value = sagitta >= maxSagitta ? 0 : (maxSagitta - sagitta) / 2;
+    const direction = getTranformDirection(angleStart, angleEnd);
+    data.yOffset.value = Math.abs(sagitta) >= maxSagitta ? 0 : (direction * (maxSagitta - sagitta)) / 2;
   }
 
   const fullSize = referenceSize;

packages/charts/src/chart_types/goal_chart/layout/viewmodel/utils.test.ts

@@ -0,0 +1,89 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+import { Radian } from './../../../../common/geometry';
+import { normalizeAngles } from './utils';
+
+const normalize = (a: Radian): number => a / Math.PI;
+const denormalize = (a: number): Radian => a * Math.PI;
+
+/**
+ * Units of π
+ */
+type AngleTuple = [start: number, end: number];
+
+describe('Goal utils', () => {
+  type TestCase = [initial: AngleTuple, final: AngleTuple];
+  describe('#normalizeAngles', () => {
+    const testCases: TestCase[] = [
+      [
+        [1.5, 2.5],
+        [-0.5, 0.5],
+      ],
+      [
+        [-1.5, -2.5],
+        [0.5, -0.5],
+      ],
+      [
+        [0.5, 1],
+        [0.5, 1],
+      ],
+      [
+        [-0.5, -1],
+        [-0.5, -1],
+      ],
+      [
+        [0, 2],
+        [0, 2],
+      ],
+      [
+        [0, -2],
+        [0, -2],
+      ],
+      [
+        [2, 4],
+        [0, 2],
+      ],
+      [
+        [-2, -4],
+        [0, -2],
+      ],
+      [
+        [20, 21],
+        [0, 1],
+      ],
+      [
+        [-20, -21],
+        [0, -1],
+      ],
+    ];
+
+    describe('counterclockwise', () => {
+      it.each<TestCase>(testCases)('should normalize angles from %j to %j', (inital, final) => {
+        const initialAngles = inital.map(denormalize) as AngleTuple;
+        const result = normalizeAngles(...initialAngles).map(normalize);
+        // needed for rounding errrors with normalizing
+        expect(result[0]).toBeCloseTo(final[0]);
+        expect(result[1]).toBeCloseTo(final[1]);
+      });
+    });
+
+    describe('clockwise', () => {
+      it.each<TestCase>(testCases.map((arr) => arr.map((a) => a.reverse() as AngleTuple) as TestCase))(
+        'should normalize angles from %j to %j',
+        (inital, final) => {
+          const initialAngles = inital.map(denormalize) as AngleTuple;
+          const result = normalizeAngles(...initialAngles).map(normalize);
+          // needed for rounding errrors with normalizing
+          expect(result[0]).toBeCloseTo(final[0]);
+          expect(result[1]).toBeCloseTo(final[1]);
+        },
+      );
+    });
+  });
+});

packages/charts/src/chart_types/goal_chart/layout/viewmodel/utils.ts

@@ -6,6 +6,7 @@
  * Side Public License, v 1.
  */
 
+import { TAU } from '../../../../common/constants';
 import { Radian } from '../../../../common/geometry';
 import { round } from '../../../../utils/common';
 
@@ -16,22 +17,77 @@ import { round } from '../../../../utils/common';
 const LIMITING_ANGLE = Math.PI / 2;
 
 /**
- * Returns limiting angle form π/2 towards 3/2π from left and right
+ * Angles are relative to mathematical angles of a unit circle from -2π > θ > 2π
  */
-const controllingAngle = (...angles: Radian[]): Radian =>
-  angles.reduce((limitAngle, a) => {
-    if (a >= Math.PI / 2 && a <= (3 / 2) * Math.PI) {
-      const newA = Math.abs(a - Math.PI / 2);
-      return Math.max(limitAngle, newA);
-    }
-    if (a >= -Math.PI / 2 && a <= Math.PI / 2) {
-      const newA = Math.abs(a - Math.PI / 2);
-      return Math.max(limitAngle, newA);
-    }
-    return limitAngle;
-  }, LIMITING_ANGLE);
+const hasTopGap = (angleStart: Radian, angleEnd: Radian): boolean => {
+  const [a, b] = [angleStart, angleEnd].sort();
+  return a <= -Math.PI / 2 && a >= (-Math.PI * 3) / 2 && b >= -Math.PI / 2 && b <= Math.PI / 2;
+};
+
+/**
+ * Angles are relative to mathematical angles of a unit circle from -2π > θ > 2π
+ */
+const hasBottomGap = (angleStart: Radian, angleEnd: Radian): boolean => {
+  const [a, b] = [angleStart, angleEnd].sort();
+  return a >= -Math.PI / 2 && a <= Math.PI / 2 && b < (Math.PI * 3) / 2 && b >= Math.PI / 2;
+};
+
+/**
+ * Angles are relative to mathematical angles of a unit circle from -2π > θ > 2π
+ */
+const isOnlyTopHalf = (angleStart: Radian, angleEnd: Radian): boolean => {
+  const [a, b] = [angleStart, angleEnd].sort();
+  return a >= 0 && b <= Math.PI;
+};
+
+/**
+ * Angles are relative to mathematical angles of a unit circle from -2π > θ > 2π
+ */
+const isOnlyBottomHalf = (angleStart: Radian, angleEnd: Radian): boolean => {
+  const [a, b] = [angleStart, angleEnd].sort();
+  return (a >= Math.PI && b <= 2 * Math.PI) || (a >= -Math.PI && b <= 0);
+};
+
+/**
+ * Angles are relative to mathematical angles of a unit circle from -2π > θ > 2π
+ */
+const isWithinLimitedDomain = (angleStart: Radian, angleEnd: Radian): boolean => {
+  const [a, b] = [angleStart, angleEnd].sort();
+  return a > -2 * Math.PI && b < 2 * Math.PI;
+};
 
 /** @internal */
+export const getTranformDirection = (angleStart: Radian, angleEnd: Radian): 1 | -1 =>
+  hasTopGap(angleStart, angleEnd) || isOnlyBottomHalf(angleStart, angleEnd) ? -1 : 1;
+
+/**
+ * Returns limiting angle form π/2 towards 3/2π from left and right, top and bottom
+ * Angles are relative to mathematical angles of a unit circle from -2π > θ > 2π
+ */
+const controllingAngle = (angleStart: Radian, angleEnd: Radian): number => {
+  if (!isWithinLimitedDomain(angleStart, angleEnd)) return LIMITING_ANGLE * 2;
+  if (isOnlyTopHalf(angleStart, angleEnd) || isOnlyBottomHalf(angleStart, angleEnd)) return LIMITING_ANGLE;
+  if (!hasTopGap(angleStart, angleEnd) && !hasBottomGap(angleStart, angleEnd)) return LIMITING_ANGLE * 2;
+  const offset = hasBottomGap(angleStart, angleEnd) ? -Math.PI / 2 : Math.PI / 2;
+  return Math.max(Math.abs(angleStart + offset), Math.abs(angleEnd + offset), LIMITING_ANGLE);
+};
+
+/**
+ * Normalize angles to minimum equivalent pair within -2π >= θ >= 2π
+ * Assumes angles are no more that 2π apart.
+ * @internal
+ */
+export function normalizeAngles(angleStart: Radian, angleEnd: Radian): [angleStart: Radian, angleEnd: Radian] {
+  const maxOffset = Math.max(Math.ceil(Math.abs(angleStart) / TAU), Math.ceil(Math.abs(angleEnd) / TAU)) - 1;
+  const offsetDirection = angleStart > 0 && angleEnd > 0 ? -1 : 1;
+  const offset = offsetDirection * maxOffset * TAU;
+  return [angleStart + offset, angleEnd + offset];
+}
+
+/**
+ * Angles are relative to mathmatical angles of a unit circle from -2π > θ > 2π
+ * @internal
+ */
 export function getSagitta(angle: Radian, radius: number, fractionDigits: number = 1) {
   const arcLength = angle * radius;
   const halfCord = radius * Math.sin(arcLength / (2 * radius));
@@ -40,8 +96,12 @@ export function getSagitta(angle: Radian, radius: number, fractionDigits: number
   return round(sagitta, fractionDigits);
 }
 
-/** @internal */
-export function getMinSagitta(startAngle: Radian, endAngle: Radian, radius: number, fractionDigits?: number) {
-  const limitingAngle = controllingAngle(startAngle, endAngle);
+/**
+ * Angles are relative to mathmatical angles of a unit circle from -2π > θ > 2π
+ * @internal
+ */
+export function getMinSagitta(angleStart: Radian, angleEnd: Radian, radius: number, fractionDigits?: number) {
+  const normalizedAngles = normalizeAngles(angleStart, angleEnd);
+  const limitingAngle = controllingAngle(...normalizedAngles);
   return getSagitta(limitingAngle * 2, radius, fractionDigits);
 }

packages/charts/src/chart_types/goal_chart/specs/index.ts

@@ -10,10 +10,12 @@ import { ComponentProps } from 'react';
 
 import { ChartType } from '../..';
 import { Color } from '../../../common/colors';
+import { TAU } from '../../../common/constants';
 import { Spec } from '../../../specs';
 import { SpecType } from '../../../specs/constants';
-import { specComponentFactory } from '../../../state/spec_factory';
-import { LabelAccessor, ValueFormatter } from '../../../utils/common';
+import { buildSFProps, SFProps, useSpecFactory } from '../../../state/spec_factory';
+import { LabelAccessor, round, stripUndefined, ValueFormatter } from '../../../utils/common';
+import { Logger } from '../../../utils/logger';
 import { defaultGoalSpec } from '../layout/types/viewmodel_types';
 import { GoalSubtype } from './constants';
 
@@ -57,11 +59,7 @@ export interface GoalSpec extends Spec {
   tooltipValueFormatter: ValueFormatter;
 }
 
-/**
- * Add Goal spec to chart
- * @alpha
- */
-export const Goal = specComponentFactory<GoalSpec>()(
+const buildProps = buildSFProps<GoalSpec>()(
   {
     specType: SpecType.Series,
     chartType: ChartType.Goal,
@@ -71,5 +69,43 @@ export const Goal = specComponentFactory<GoalSpec>()(
   },
 );
 
+/**
+ * Add Goal spec to chart
+ * @alpha
+ */
+export const Goal = function (
+  props: SFProps<
+    GoalSpec,
+    keyof typeof buildProps['overrides'],
+    keyof typeof buildProps['defaults'],
+    keyof typeof buildProps['optionals'],
+    keyof typeof buildProps['requires']
+  >,
+) {
+  const { defaults, overrides } = buildProps;
+  const angleStart = props.angleStart ?? defaults.angleStart;
+  const angleEnd = props.angleEnd ?? defaults.angleEnd;
+  const constraints: Pick<typeof props, 'angleEnd'> = {};
+
+  if (Math.abs(angleEnd - angleStart) > TAU) {
+    constraints.angleEnd = angleStart + TAU * Math.sign(angleEnd - angleStart);
+
+    Logger.warn(`The total angle of the goal chart must not exceed 2π radians.\
+To prevent overlapping, the value of \`angleEnd\` will be replaced.
+
+  original: ${angleEnd} (~${round(angleEnd / Math.PI, 3)}π)
+  replaced: ${constraints.angleEnd} (~${round(constraints.angleEnd / Math.PI, 3)}π)
+`);
+  }
+
+  useSpecFactory<GoalSpec>({
+    ...defaults,
+    ...stripUndefined(props),
+    ...overrides,
+    ...constraints,
+  });
+  return null;
+};
+
 /** @public */
 export type GoalProps = ComponentProps<typeof Goal>;