Remove erroneous shape from vertical on hover line when shape dimensi…

…on is added (#2486)

* remove erroneous shape from vertical on hover line when shape dimension is added

* add test for spec update

* revert min cli version

extension/src/plots/vega/util.test.ts

@@ -1,6 +1,7 @@
 import { Text as VegaText, Title as VegaTitle } from 'vega'
 import { TopLevelSpec } from 'vega-lite'
 import merge from 'lodash.merge'
+import cloneDeep from 'lodash.clonedeep'
 import {
   isMultiViewPlot,
   isMultiViewByCommitPlot,
@@ -15,6 +16,7 @@ import defaultTemplate from '../../test/fixtures/plotsDiff/templates/default'
 import linearTemplate from '../../test/fixtures/plotsDiff/templates/linear'
 import scatterTemplate from '../../test/fixtures/plotsDiff/templates/scatter'
 import smoothTemplate from '../../test/fixtures/plotsDiff/templates/smooth'
+import multiSourceTemplate from '../../test/fixtures/plotsDiff/templates/multiSource'
 import { copyOriginalColors } from '../../experiments/model/status/colors'
 import { PlotSize } from '../webview/contract'
 
@@ -264,6 +266,25 @@ describe('extendVegaSpec', () => {
     expect(updatedSpecString).not.toContain(repeatedTitle)
     expect(updatedSpecString).toContain(truncatedTitle)
   })
+
+  it('should update the multi-source template to remove erroneous shape encoding from the vertical line displayed on hover', () => {
+    const updatedSpec = extendVegaSpec(multiSourceTemplate, PlotSize.LARGE, {
+      color: { domain: [], range: [] },
+      shape: {
+        field: 'field',
+        scale: { domain: [], range: [] }
+      }
+    })
+
+    expect(updatedSpec.encoding).not.toBeUndefined()
+    expect(updatedSpec.layer[1].layer[0].encoding.shape).toBeNull()
+
+    const updatedSpecCopy = cloneDeep(updatedSpec)
+    delete updatedSpecCopy.layer[1].layer[0].encoding.shape
+    delete updatedSpecCopy.encoding
+
+    expect(updatedSpecCopy).toStrictEqual(multiSourceTemplate)
+  })
 })
 
 describe('reverseOfLegendSuppressionUpdate', () => {
@@ -282,17 +303,14 @@ describe('reverseOfLegendSuppressionUpdate', () => {
       shape: {
         field: 'shape-field',
         legend: {
-          disable: true,
-          symbolFillColor: 'blue'
+          disable: true
         },
         scale: { domain: [], range: [] }
       },
       strokeDash: {
         field: 'strokeDash-field',
         legend: {
-          disable: true,
-          symbolFillColor: 'blue',
-          symbolStrokeColor: 'red'
+          disable: true
         },
         scale: { domain: [], range: [] }
       }

extension/src/plots/vega/util.ts

@@ -19,6 +19,7 @@ import {
   RepeatMapping
 } from 'vega-lite/build/src/spec/repeat'
 import { TopLevelUnitSpec } from 'vega-lite/build/src/spec/unit'
+import isEqual from 'lodash.isequal'
 import { ColorScale, PlotSize, Revision } from '../webview/contract'
 import { ShapeEncoding, StrokeDashEncoding } from '../multiSource/constants'
 
@@ -99,44 +100,61 @@ export const getColorScale = (
   return acc.domain.length > 0 ? acc : undefined
 }
 
-export type Encoding = {
-  strokeDash?: StrokeDashEncoding & {
-    legend: {
-      disable: boolean
-      symbolFillColor: string
-      symbolStrokeColor: string
-    }
-  }
-  shape?: ShapeEncoding & {
-    legend: {
-      disable: boolean
-      symbolFillColor: string
-    }
-  }
-  detail?: {
-    field: string
-  }
-  color?: {
-    legend: {
-      disable: boolean
-    }
-    scale: ColorScale
+type LegendDisabled = {
+  legend: {
+    disable: boolean
   }
 }
 
+export type Encoding = {
+  strokeDash?: StrokeDashEncoding & LegendDisabled
+  shape?: ShapeEncoding & LegendDisabled
+  detail?: { field: string }
+  color?: { scale: ColorScale } & LegendDisabled
+}
+
+type ShapePatchUpdate = {
+  layer?: { layer: [{ encoding: Record<string, unknown> }] }[]
+}
+
 type EncodingUpdate = {
   encoding: Encoding
+} & ShapePatchUpdate
+
+const specHasVerticalLineOnHover = (
+  spec: any
+): spec is { layer: { layer: [{ encoding: Record<string, unknown> }] }[] } =>
+  spec.layer?.[1]?.layer?.[0]?.encoding?.x &&
+  isEqual(spec.layer[1].layer[0].mark, {
+    color: 'gray',
+    type: 'rule'
+  })
+
+const patchShapeEncoding = (spec: TopLevelSpec, encoding: Encoding) => {
+  const update: EncodingUpdate = {
+    encoding
+  }
+
+  if (encoding.shape && specHasVerticalLineOnHover(spec)) {
+    update.layer = spec.layer
+    update.layer[1].layer[0].encoding.shape = null
+  }
+
+  return update
 }
 
-export const getSpecEncodingUpdate = ({
-  color,
-  shape,
-  strokeDash
-}: {
-  color?: ColorScale
-  shape?: ShapeEncoding
-  strokeDash?: StrokeDashEncoding
-}): EncodingUpdate => {
+const getSpecEncodingUpdate = (
+  spec: TopLevelSpec,
+  {
+    color,
+    shape,
+    strokeDash
+  }: {
+    color?: ColorScale
+    shape?: ShapeEncoding
+    strokeDash?: StrokeDashEncoding
+  }
+): EncodingUpdate => {
   const encoding: Encoding = {}
   if (color) {
     encoding.color = {
@@ -149,9 +167,7 @@ export const getSpecEncodingUpdate = ({
     encoding.strokeDash = {
       ...strokeDash,
       legend: {
-        disable: true,
-        symbolFillColor: 'transparent',
-        symbolStrokeColor: 'grey'
+        disable: true
       }
     }
   }
@@ -160,16 +176,13 @@ export const getSpecEncodingUpdate = ({
     encoding.shape = {
       ...shape,
       legend: {
-        disable: true,
-        symbolFillColor: 'grey'
+        disable: true
       }
     }
-    encoding.detail = shape
+    encoding.detail = { field: shape.field }
   }
 
-  return {
-    encoding
-  }
+  return patchShapeEncoding(spec, encoding)
 }
 
 const mergeUpdate = (spec: TopLevelSpec, update: EncodingUpdate) => {
@@ -299,23 +312,30 @@ export const extendVegaSpec = (
     return updatedSpec
   }
 
-  const update = getSpecEncodingUpdate(encoding)
+  const update = getSpecEncodingUpdate(updatedSpec, encoding)
 
   return mergeUpdate(updatedSpec, update)
 }
 
 export const reverseOfLegendSuppressionUpdate = () => ({
   spec: {
     encoding: {
-      color: { legend: { disable: false } },
-      shape: {
+      color: {
         legend: {
           disable: false
         }
       },
+      shape: {
+        legend: {
+          disable: false,
+          symbolFillColor: 'grey'
+        }
+      },
       strokeDash: {
         legend: {
-          disable: false
+          disable: false,
+          symbolFillColor: 'transparent',
+          symbolStrokeColor: 'grey'
         }
       }
     }

extension/src/test/fixtures/plotsDiff/templates/multiSource.ts

@@ -0,0 +1,75 @@
+import { TopLevelSpec } from 'vega-lite'
+
+const data = {
+  $schema: 'https://vega.github.io/schema/vega-lite/v5.json',
+  data: '<DVC_METRIC_DATA>',
+  title: '<DVC_METRIC_TITLE>',
+  width: 300,
+  height: 300,
+  layer: [
+    {
+      encoding: {
+        x: {
+          field: '<DVC_METRIC_X>',
+          type: 'quantitative',
+          title: '<DVC_METRIC_X_LABEL>'
+        },
+        y: {
+          field: '<DVC_METRIC_Y>',
+          type: 'quantitative',
+          title: '<DVC_METRIC_Y_LABEL>',
+          scale: { zero: false }
+        },
+        color: {
+          field: 'rev'
+        }
+      },
+      layer: [
+        { mark: 'line' },
+        {
+          selection: {
+            label: {
+              type: 'single',
+              nearest: true,
+              on: 'mouseover',
+              encodings: ['<DVC_METRIC_X>'],
+              empty: 'none',
+              clear: 'mouseout'
+            }
+          },
+          mark: 'point',
+          encoding: {
+            opacity: {
+              condition: { selection: 'label', value: 1 },
+              value: 0
+            }
+          }
+        }
+      ]
+    },
+    {
+      transform: [{ filter: { selection: 'label' } }],
+      layer: [
+        {
+          mark: { type: 'rule', color: 'gray' },
+          encoding: { x: { field: '<DVC_METRIC_X>', type: 'quantitative' } }
+        },
+        {
+          encoding: {
+            text: { type: 'quantitative', field: '<DVC_METRIC_Y>' },
+            x: { field: '<DVC_METRIC_X>', type: 'quantitative' },
+            y: { field: '<DVC_METRIC_Y>', type: 'quantitative' }
+          },
+          layer: [
+            {
+              mark: { type: 'text', align: 'left', dx: 5, dy: -5 },
+              encoding: { color: { type: 'nominal', field: 'rev' } }
+            }
+          ]
+        }
+      ]
+    }
+  ]
+} as unknown as TopLevelSpec
+
+export default data