From b24f2db0e05a87708ccfc3e903583e050ca41872 Mon Sep 17 00:00:00 2001 From: mattseddon <37993418+mattseddon@users.noreply.github.com> Date: Thu, 29 Sep 2022 07:12:09 +1000 Subject: [PATCH] Remove erroneous shape from vertical on hover line when shape dimension 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 | 28 ++++- extension/src/plots/vega/util.ts | 108 +++++++++++------- .../plotsDiff/templates/multiSource.ts | 75 ++++++++++++ 3 files changed, 162 insertions(+), 49 deletions(-) create mode 100644 extension/src/test/fixtures/plotsDiff/templates/multiSource.ts diff --git a/extension/src/plots/vega/util.test.ts b/extension/src/plots/vega/util.test.ts index 8179f71c78..30bdca8828 100644 --- a/extension/src/plots/vega/util.test.ts +++ b/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: [] } } diff --git a/extension/src/plots/vega/util.ts b/extension/src/plots/vega/util.ts index fd6be2f7ac..cce14a7e75 100644 --- a/extension/src/plots/vega/util.ts +++ b/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 }] }[] +} + type EncodingUpdate = { encoding: Encoding +} & ShapePatchUpdate + +const specHasVerticalLineOnHover = ( + spec: any +): spec is { layer: { layer: [{ encoding: Record }] }[] } => + 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,7 +312,7 @@ export const extendVegaSpec = ( return updatedSpec } - const update = getSpecEncodingUpdate(encoding) + const update = getSpecEncodingUpdate(updatedSpec, encoding) return mergeUpdate(updatedSpec, update) } @@ -307,15 +320,22 @@ export const extendVegaSpec = ( 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' } } } diff --git a/extension/src/test/fixtures/plotsDiff/templates/multiSource.ts b/extension/src/test/fixtures/plotsDiff/templates/multiSource.ts new file mode 100644 index 0000000000..f830fdaa8c --- /dev/null +++ b/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: '', + title: '', + width: 300, + height: 300, + layer: [ + { + encoding: { + x: { + field: '', + type: 'quantitative', + title: '' + }, + y: { + field: '', + type: 'quantitative', + title: '', + scale: { zero: false } + }, + color: { + field: 'rev' + } + }, + layer: [ + { mark: 'line' }, + { + selection: { + label: { + type: 'single', + nearest: true, + on: 'mouseover', + encodings: [''], + 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: '', type: 'quantitative' } } + }, + { + encoding: { + text: { type: 'quantitative', field: '' }, + x: { field: '', type: 'quantitative' }, + y: { field: '', 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