fix(line_annotation): use scaleAndValidate for line annotations (#236)

elastic · Jun 14, 2019 · 48b180a · 48b180a
1 parent 01d2b60
commit 48b180a
Show file tree

Hide file tree

Showing 4 changed files with 65 additions and 13 deletions.
diff --git a/src/state/annotation_marker.test.tsx b/src/state/annotation_marker.test.tsx
@@ -57,6 +57,7 @@ describe('annotation marker', () => {
       xScale,
       Position.Left,
       0,
+      false,
     );
     const expectedDimensions = [
       {
@@ -96,6 +97,7 @@ describe('annotation marker', () => {
       xScale,
       Position.Left,
       0,
+      false,
     );
     const expectedDimensions = [
       {
@@ -135,6 +137,7 @@ describe('annotation marker', () => {
       xScale,
       Position.Left,
       0,
+      false,
     );
     const expectedDimensions = [
       {

diff --git a/src/state/annotation_utils.test.ts b/src/state/annotation_utils.test.ts
@@ -203,6 +203,7 @@ describe('annotation utils', () => {
       xScale,
       Position.Left,
       0,
+      false,
     );
     const expectedDimensions = [
       {
@@ -239,6 +240,7 @@ describe('annotation utils', () => {
       xScale,
       Position.Right,
       0,
+      false,
     );
     const expectedDimensions = [
       {
@@ -275,6 +277,7 @@ describe('annotation utils', () => {
       xScale,
       Position.Left,
       0,
+      false,
     );
     const expectedDimensions = [
       {
@@ -309,6 +312,7 @@ describe('annotation utils', () => {
       xScale,
       Position.Left,
       0,
+      false,
     );
     expect(dimensions).toEqual(null);
   });
@@ -336,6 +340,7 @@ describe('annotation utils', () => {
       xScale,
       Position.Left,
       0,
+      false,
     );
     const expectedDimensions = [
       {
@@ -370,6 +375,7 @@ describe('annotation utils', () => {
       xScale,
       Position.Top,
       0,
+      false,
     );
     const expectedDimensions = [
       {
@@ -404,6 +410,7 @@ describe('annotation utils', () => {
       xScale,
       Position.Bottom,
       0,
+      false,
     );
     const expectedDimensions = [
       {
@@ -415,6 +422,41 @@ describe('annotation utils', () => {
     expect(dimensions).toEqual(expectedDimensions);
   });
 
+  test('should compute line annotation dimensions for xDomain in histogramMode with extended upper bound', () => {
+    const chartRotation: Rotation = 0;
+    const yScales: Map<GroupId, Scale> = new Map();
+    const xScale: Scale = continuousScale;
+
+    const annotationId = getAnnotationId('foo-line');
+    const lineAnnotation: LineAnnotationSpec = {
+      annotationType: 'line',
+      annotationId,
+      domainType: AnnotationDomainTypes.XDomain,
+      dataValues: [{ dataValue: 10.5, details: 'foo' }],
+      groupId,
+      style: DEFAULT_ANNOTATION_LINE_STYLE,
+    };
+
+    const dimensions = computeLineAnnotationDimensions(
+      lineAnnotation,
+      chartDimensions,
+      chartRotation,
+      yScales,
+      xScale,
+      Position.Bottom,
+      0,
+      true,
+    );
+    const expectedDimensions = [
+      {
+        position: [105, 0, 105, 20],
+        details: { detailsText: 'foo', headerText: '10.5' },
+        tooltipLinePosition: [105, 0, 105, 20],
+      },
+    ];
+    expect(dimensions).toEqual(expectedDimensions);
+  });
+
   test('should compute line annotation dimensions for xDomain on a xScale (chartRotation 90, ordinal scale)', () => {
     const chartRotation: Rotation = 90;
     const yScales: Map<GroupId, Scale> = new Map();
@@ -439,6 +481,7 @@ describe('annotation utils', () => {
       xScale,
       Position.Left,
       0,
+      false,
     );
     const expectedDimensions = [
       {
@@ -474,6 +517,7 @@ describe('annotation utils', () => {
       xScale,
       Position.Left,
       0,
+      false,
     );
     const expectedDimensions = [
       {
@@ -509,6 +553,7 @@ describe('annotation utils', () => {
       xScale,
       Position.Left,
       0,
+      false,
     );
     const expectedDimensions = [
       {
@@ -544,6 +589,7 @@ describe('annotation utils', () => {
       xScale,
       Position.Top,
       0,
+      false,
     );
     const expectedDimensions = [
       {
@@ -578,6 +624,7 @@ describe('annotation utils', () => {
       xScale,
       Position.Bottom,
       0,
+      false,
     );
     const expectedDimensions = [
       {
@@ -614,6 +661,7 @@ describe('annotation utils', () => {
       xScale,
       Position.Right,
       0,
+      false,
     );
 
     expect(emptyXDimensions).toEqual([]);
@@ -635,6 +683,7 @@ describe('annotation utils', () => {
       continuousScale,
       Position.Right,
       0,
+      false,
     );
 
     expect(invalidStringXDimensions).toEqual([]);
@@ -656,6 +705,7 @@ describe('annotation utils', () => {
       continuousScale,
       Position.Right,
       0,
+      false,
     );
 
     expect(emptyOutOfBoundsXDimensions).toEqual([]);
@@ -677,6 +727,7 @@ describe('annotation utils', () => {
       xScale,
       Position.Right,
       0,
+      false,
     );
 
     expect(emptyYDimensions).toEqual([]);
@@ -698,6 +749,7 @@ describe('annotation utils', () => {
       xScale,
       Position.Right,
       0,
+      false,
     );
 
     expect(emptyOutOfBoundsYDimensions).toEqual([]);
@@ -719,6 +771,7 @@ describe('annotation utils', () => {
       continuousScale,
       Position.Right,
       0,
+      false,
     );
 
     expect(invalidStringYDimensions).toEqual([]);
@@ -741,6 +794,7 @@ describe('annotation utils', () => {
       continuousScale,
       Position.Right,
       0,
+      false,
     );
 
     expect(hiddenAnnotationDimensions).toEqual(null);

diff --git a/src/state/annotation_utils.ts b/src/state/annotation_utils.ts
@@ -179,6 +179,7 @@ export function computeXDomainLineAnnotationDimensions(
   chartDimensions: Dimensions,
   lineColor: string,
   xScaleOffset: number,
+  enableHistogramMode: boolean,
   marker?: JSX.Element,
   markerDimensions?: { width: number; height: number },
 ): AnnotationLineProps[] {
@@ -187,6 +188,7 @@ export function computeXDomainLineAnnotationDimensions(
   const markerOffsets = markerDimensions || { width: 0, height: 0 };
   const lineProps: AnnotationLineProps[] = [];
 
+  const alignWithTick = xScale.bandwidth > 0 && !enableHistogramMode;
   dataValues.forEach((datum: LineAnnotationDatum) => {
     const { dataValue } = datum;
     const details = {
@@ -195,23 +197,13 @@ export function computeXDomainLineAnnotationDimensions(
     };
 
     const offset = xScale.bandwidth / 2 - xScaleOffset;
-    const isContinuous = xScale.type !== ScaleType.Ordinal;
 
-    const scaledXValue = xScale.scale(dataValue);
+    const scaledXValue = scaleAndValidateDatum(dataValue, xScale, alignWithTick);
 
-    // d3.scale will return 0 for '', rendering the line incorrectly at 0
-    if (isNaN(scaledXValue) || (isContinuous && dataValue === '')) {
+    if (scaledXValue == null) {
       return;
     }
 
-    if (isContinuous) {
-      const [domainStart, domainEnd] = xScale.domain;
-
-      if (domainStart > dataValue || domainEnd < dataValue) {
-        return;
-      }
-    }
-
     const xDomainPosition = scaledXValue + offset;
 
     let linePosition: AnnotationLinePosition = [0, 0, 0, 0];
@@ -280,6 +272,7 @@ export function computeLineAnnotationDimensions(
   xScale: Scale,
   axisPosition: Position,
   xScaleOffset: number,
+  enableHistogramMode: boolean,
 ): AnnotationLineProps[] | null {
   const { domainType, dataValues, marker, markerDimensions, hideLines } = annotationSpec;
 
@@ -304,6 +297,7 @@ export function computeLineAnnotationDimensions(
       chartDimensions,
       lineColor,
       xScaleOffset,
+      enableHistogramMode,
       marker,
       markerDimensions,
     );
@@ -545,6 +539,7 @@ export function computeAnnotationDimensions(
         xScale,
         annotationAxisPosition,
         xScaleOffset - clusterOffset,
+        enableHistogramMode,
       );
 
       if (dimensions) {

diff --git a/stories/bar_chart.tsx b/stories/bar_chart.tsx
@@ -1404,7 +1404,7 @@ storiesOf('Bar Chart', module)
         <LineAnnotation
           annotationId={getAnnotationId('line-annotation')}
           domainType={AnnotationDomainTypes.XDomain}
-          dataValues={[{ dataValue: 2 }, { dataValue: 2.5 }]}
+          dataValues={[{ dataValue: 2 }, { dataValue: 2.5 }, { dataValue: 3.5 }]}
           style={lineAnnotationStyle}
           marker={<div style={{ background: 'red', width: 10, height: 10 }} />}
         />