adobe · marshallpete · Mar 6, 2024 · Mar 5, 2024 · Mar 5, 2024 · Mar 5, 2024
diff --git a/src/specBuilder/area/areaSpecBuilder.test.ts b/src/specBuilder/area/areaSpecBuilder.test.ts
@@ -21,13 +21,15 @@ import {
 	DEFAULT_TIME_DIMENSION,
 	DEFAULT_TRANSFORMED_TIME_DIMENSION,
 	FILTERED_TABLE,
+	HIGHLIGHTED_ITEM,
+	HIGHLIGHTED_SERIES,
 	MARK_ID,
-	SERIES_ID,
 	TABLE,
 } from '@constants';
 import { AreaSpecProps } from 'types';
 import { Data, GroupMark, Spec } from 'vega';
 
+import { defaultSignals } from '@specBuilder/specTestUtils';
 import { initializeSpec } from '../specUtils';
 import { addArea, addAreaMarks, addData, addSignals, setScales } from './areaSpecBuilder';
 
@@ -150,20 +152,6 @@ const defaultPointScale = {
 	type: 'point',
 };
 
-const defaultSignals = [
-	{ name: 'area0_controlledHoveredId', on: [{ events: '@area0:mouseout', update: 'null' }], value: null },
-	{
-		name: 'area0_hoveredSeries',
-		on: [
-			{ events: '@area0:mouseover', update: `datum.${SERIES_ID}` },
-			{ events: '@area0:mouseout', update: 'null' },
-		],
-		value: null,
-	},
-	{ name: 'area0_selectedId', value: null },
-	{ name: 'area0_selectedSeries', value: null },
-];
-
 describe('areaSpecBuilder', () => {
 	describe('addArea()', () => {
 		test('should add area', () => {
@@ -201,14 +189,19 @@ describe('areaSpecBuilder', () => {
 
 	describe('addSignals()', () => {
 		test('no children: should return nothing', () => {
-			expect(addSignals(startingSpec.signals ?? [], defaultAreaProps)).toStrictEqual([]);
+			expect(addSignals(defaultSignals, defaultAreaProps)).toStrictEqual(defaultSignals);
 		});
 
 		test('children: should add signals', () => {
 			const tooltip = createElement(ChartTooltip);
-			expect(addSignals(startingSpec.signals ?? [], { ...defaultAreaProps, children: [tooltip] })).toStrictEqual(
-				defaultSignals,
-			);
+			const signals = addSignals(defaultSignals, { ...defaultAreaProps, children: [tooltip] });
+			expect(signals).toHaveLength(5);
+			expect(signals[0]).toHaveProperty('name', HIGHLIGHTED_ITEM);
+			expect(signals[1]).toHaveProperty('name', HIGHLIGHTED_SERIES);
+			expect(signals[1].on).toHaveLength(2);
+			expect(signals[2]).toHaveProperty('name', 'area0_controlledHoveredId');
+			expect(signals[3]).toHaveProperty('name', 'area0_selectedId');
+			expect(signals[4]).toHaveProperty('name', 'area0_selectedSeries');
 		});
 	});
 

diff --git a/src/specBuilder/area/areaSpecBuilder.ts b/src/specBuilder/area/areaSpecBuilder.ts
@@ -21,9 +21,9 @@ import {
 	MARK_ID,
 } from '@constants';
 import {
+	addHighlightedSeriesSignalEvents,
 	getControlledHoverSignal,
 	getGenericSignal,
-	getSeriesHoveredSignal,
 	hasSignalByName,
 } from '@specBuilder/signal/signalSpecBuilder';
 import { spectrumColors } from '@themes';
@@ -147,9 +147,7 @@ export const addSignals = produce<Signal[], [AreaSpecProps]>((signals, { childre
 	if (!hasSignalByName(signals, `${name}_controlledHoveredId`)) {
 		signals.push(getControlledHoverSignal(name));
 	}
-	if (!hasSignalByName(signals, `${name}_hoveredSeries`)) {
-		signals.push(getSeriesHoveredSignal(name));
-	}
+	addHighlightedSeriesSignalEvents(signals, name);
 	if (!hasSignalByName(signals, `${name}_selectedId`)) {
 		signals.push(getGenericSignal(`${name}_selectedId`));
 	}

diff --git a/src/specBuilder/area/areaUtils.ts b/src/specBuilder/area/areaUtils.ts
@@ -10,7 +10,12 @@
  * governing permissions and limitations under the License.
  */
 import { ChartPopover } from '@components/ChartPopover';
-import { DEFAULT_TRANSFORMED_TIME_DIMENSION, HIGHLIGHT_CONTRAST_RATIO, SERIES_ID } from '@constants';
+import {
+	DEFAULT_TRANSFORMED_TIME_DIMENSION,
+	HIGHLIGHTED_SERIES,
+	HIGHLIGHT_CONTRAST_RATIO,
+	SERIES_ID,
+} from '@constants';
 import {
 	getBorderStrokeEncodings,
 	getColorProductionRule,
@@ -81,16 +86,15 @@ export function getFillOpacity(
 	children: MarkChildElement[],
 	isMetricRange?: boolean,
 	parentName?: string,
-	displayOnHover?: boolean
+	displayOnHover?: boolean,
 ): ProductionRule<NumericValueRef> | undefined {
-	const hoverSignal = isMetricRange && parentName ? `${parentName}_hoveredSeries` : `${name}_hoveredSeries`;
 	const selectSignal = `${name}_selectedSeries`;
 	const metricRangeSelectSignal = isMetricRange && parentName ? `${parentName}_selectedSeries` : selectSignal;
 
 	// if metric ranges only display when hovering, we don't need to include other hover rules for this specific area
 	if (isMetricRange && displayOnHover) {
 		return [
-			{ test: `${hoverSignal} && ${hoverSignal} === datum.${color}`, value: opacity },
+			{ test: `${HIGHLIGHTED_SERIES} && ${HIGHLIGHTED_SERIES} === datum.${color}`, value: opacity },
 			{ test: `${metricRangeSelectSignal} && ${metricRangeSelectSignal} === datum.${color}`, value: opacity },
 			{ test: `highlightedSeries && highlightedSeries === datum.${SERIES_ID}`, value: opacity },
 			{ value: 0 },
@@ -106,7 +110,7 @@ export function getFillOpacity(
 	if (children.some((child) => child.type === ChartPopover && !isMetricRange)) {
 		return [
 			{
-				test: `!${selectSignal} && ${hoverSignal} && ${hoverSignal} !== datum.${color}`,
+				test: `!${selectSignal} && ${HIGHLIGHTED_SERIES} && ${HIGHLIGHTED_SERIES} !== datum.${color}`,
 				value: opacity / HIGHLIGHT_CONTRAST_RATIO,
 			},
 			{
@@ -119,7 +123,10 @@ export function getFillOpacity(
 	}
 
 	return [
-		{ test: `${hoverSignal} && ${hoverSignal} !== datum.${color}`, value: opacity / HIGHLIGHT_CONTRAST_RATIO },
+		{
+			test: `${HIGHLIGHTED_SERIES} && ${HIGHLIGHTED_SERIES} !== datum.${color}`,
+			value: opacity / HIGHLIGHT_CONTRAST_RATIO,
+		},
 		{ value: opacity },
 	];
 }

diff --git a/src/specBuilder/bar/barSpecBuilder.test.ts b/src/specBuilder/bar/barSpecBuilder.test.ts
@@ -232,8 +232,8 @@ describe('barSpecBuilder', () => {
 	describe('addSignals()', () => {
 		test('should add padding signal', () => {
 			const signals = addSignals(defaultSignals, defaultBarProps);
-			expect(signals).toHaveLength(2);
-			expect(signals[1]).toHaveProperty('name', 'paddingInner');
+			expect(signals).toHaveLength(3);
+			expect(signals[2]).toHaveProperty('name', 'paddingInner');
 		});
 		test('should add hover events if tooltip is present', () => {
 			const signals = addSignals(defaultSignals, { ...defaultBarProps, children: [createElement(ChartTooltip)] });
@@ -246,8 +246,8 @@ describe('barSpecBuilder', () => {
 				...defaultBarProps,
 				children: [createElement(ChartTooltip), createElement(ChartPopover)],
 			});
-			expect(signals).toHaveLength(3);
-			expect(signals[2]).toHaveProperty('name', 'bar0_selectedId');
+			expect(signals).toHaveLength(4);
+			expect(signals[3]).toHaveProperty('name', 'bar0_selectedId');
 		});
 	});
 

diff --git a/src/specBuilder/chartSpecBuilder.test.ts b/src/specBuilder/chartSpecBuilder.test.ts
@@ -37,7 +37,6 @@ import { spectrumColors } from '@themes';
 import colorSchemes from '@themes/colorSchemes';
 import {
 	addData,
-	addHighlight,
 	buildSpec,
 	getColorScale,
 	getDefaultSignals,
@@ -51,8 +50,7 @@ import {
 	getTwoDimensionalLineTypes,
 	getTwoDimensionalOpacities,
 } from './chartSpecBuilder';
-import { setHoverOpacityForMarks } from './legend/legendHighlightUtils';
-import { defaultHighlightedItemSignal } from './specTestUtils';
+import { defaultSignals } from './specTestUtils';
 import { baseData } from './specUtils';
 
 const defaultData: Data[] = [{ name: TABLE, values: [], transform: [{ type: 'identifier', as: MARK_ID }] }];
@@ -409,21 +407,6 @@ describe('Chart spec builder', () => {
 			});
 		});
 
-		test('does not add highlightedSeries signal if there is no highlighted series', () => {
-			const spec = buildSpec({
-				children: [createBar()],
-				colors: 'categorical12',
-				lineTypes: ['solid', 'dashed', 'dotted', 'dotDash', 'longDash', 'twoDash'],
-				lineWidths: ['M'],
-				symbolShapes: ['rounded-square'],
-				colorScheme: 'light',
-				hiddenSeries: undefined,
-				highlightedSeries: undefined,
-			});
-
-			expect(spec.signals?.find((signal) => signal.name === 'highlightedSeries')).toBeUndefined();
-		});
-
 		test('does not apply controlled highlighting if uncontrolled highlighting is applied', () => {
 			const spec = buildSpec({
 				children: [createBar(), createLegend(true)],
@@ -441,11 +424,11 @@ describe('Chart spec builder', () => {
 				on: [
 					{
 						events: '@legend0_legendEntry:mouseover',
-						update: 'indexof(hiddenSeries, domain("legend0Entries")[datum.index]) === -1 ? domain("legend0Entries")[datum.index] : ""',
+						update: 'indexof(hiddenSeries, domain("legend0Entries")[datum.index]) === -1 ? domain("legend0Entries")[datum.index] : null',
 					},
 					{
 						events: '@legend0_legendEntry:mouseout',
-						update: '""',
+						update: 'null',
 					},
 				],
 			};
@@ -456,40 +439,6 @@ describe('Chart spec builder', () => {
 		});
 	});
 
-	describe('addHighlight()', () => {
-		test('creates spec signals and adds highlight signal if no signals are provided', () => {
-			expect(addHighlight({}, { children: [], hiddenSeries: [], highlightedSeries: undefined })).toStrictEqual({
-				signals: [{ name: 'highlightedSeries', value: null }],
-			});
-		});
-
-		test('adds highlight signal to existing spec signals', () => {
-			const testSignal = { name: 'testName', value: 'testValue' };
-			expect(
-				addHighlight(
-					{ signals: [testSignal] },
-					{ children: [], hiddenSeries: [], highlightedSeries: undefined },
-				),
-			).toStrictEqual({
-				signals: [testSignal, { name: 'highlightedSeries', value: null }],
-			});
-		});
-
-		test('adds highlight signal using the highlightedSeries value', () => {
-			expect(addHighlight({}, { children: [], hiddenSeries: [], highlightedSeries: 'testSeries' })).toStrictEqual(
-				{
-					signals: [{ name: 'highlightedSeries', value: 'testSeries' }],
-				},
-			);
-		});
-
-		test('adds hover opacity to marks', () => {
-			addHighlight({}, { children: [], hiddenSeries: [], highlightedSeries: 'testSeries' });
-			expect(setHoverOpacityForMarks).toHaveBeenCalledTimes(1);
-			expect(setHoverOpacityForMarks).toHaveBeenCalledWith([]);
-		});
-	});
-
 	describe('getDefaultSignals()', () => {
 		const beginningSignals = [
 			{ name: BACKGROUND_COLOR, value: 'rgb(255, 255, 255)' },
@@ -514,7 +463,7 @@ describe('Chart spec builder', () => {
 			{ name: 'opacities', value: [[1]] },
 		];
 
-		const endSignals = [defaultHighlightedItemSignal];
+		const endSignals = defaultSignals;
 
 		test('hiddenSeries is empty when no hidden series', () => {
 			expect(

diff --git a/src/specBuilder/chartSpecBuilder.ts b/src/specBuilder/chartSpecBuilder.ts
@@ -16,6 +16,7 @@ import {
 	DEFAULT_LINE_TYPES,
 	FILTERED_TABLE,
 	HIGHLIGHTED_ITEM,
+	HIGHLIGHTED_SERIES,
 	LINEAR_COLOR_SCALE,
 	LINE_TYPE_SCALE,
 	LINE_WIDTH_SCALE,
@@ -63,7 +64,7 @@ import { addLegend } from './legend/legendSpecBuilder';
 import { addLine } from './line/lineSpecBuilder';
 import { getOrdinalScale } from './scale/scaleSpecBuilder';
 import { addScatter } from './scatter/scatterSpecBuilder';
-import { getGenericSignal, hasSignalByName } from './signal/signalSpecBuilder';
+import { getGenericSignal } from './signal/signalSpecBuilder';
 import {
 	getColorValue,
 	getFacetsFromScales,
@@ -91,7 +92,15 @@ export function buildSpec({
 	title,
 }: SanitizedSpecProps) {
 	let spec = initializeSpec(null, { backgroundColor, colorScheme, description, title });
-	spec.signals = getDefaultSignals(backgroundColor, colors, colorScheme, lineTypes, opacities, hiddenSeries);
+	spec.signals = getDefaultSignals(
+		backgroundColor,
+		colors,
+		colorScheme,
+		lineTypes,
+		opacities,
+		hiddenSeries,
+		highlightedSeries,
+	);
 	spec.scales = getDefaultScales(colors, colorScheme, lineTypes, lineWidths, opacities, symbolShapes, symbolSizes);
 
 	// need to build the spec in a specific order
@@ -161,8 +170,8 @@ export function buildSpec({
 	spec.data = addData(spec.data ?? [], { facets: getFacetsFromScales(spec.scales) });
 
 	// add signals and update marks for controlled highlighting if there isn't a legend with highlight enabled
-	if (highlightedSeries && !hasSignalByName(spec.signals ?? [], 'highlightedSeries')) {
-		spec = addHighlight(spec, { children, hiddenSeries, highlightedSeries });
+	if (highlightedSeries) {
+		setHoverOpacityForMarks(spec.marks ?? []);
 	}
 
 	// clear out all scales that don't have any fields on the domain
@@ -171,12 +180,6 @@ export function buildSpec({
 	return spec;
 }
 
-export const addHighlight = produce<Spec, [SanitizedSpecProps]>((spec, { highlightedSeries }) => {
-	if (!spec.signals) spec.signals = [];
-	spec.signals.push(getGenericSignal(`highlightedSeries`, highlightedSeries));
-	setHoverOpacityForMarks(spec.marks ?? []);
-});
-
 export const removeUnusedScales = produce<Spec>((spec) => {
 	spec.scales = spec.scales?.filter((scale) => {
 		if ('domain' in scale && scale.domain && 'fields' in scale.domain && scale.domain.fields.length === 0) {
@@ -205,6 +208,7 @@ export const getDefaultSignals = (
 	lineTypes: LineTypes,
 	opacities: Opacities | undefined,
 	hiddenSeries?: string[],
+	highlightedSeries?: string,
 ): Signal[] => {
 	// if the background color is transparent, then we want to set the signal background color to gray-50
 	// if the signal background color were transparent then backgroundMarks and annotation fill would also be transparent
@@ -216,6 +220,7 @@ export const getDefaultSignals = (
 		getGenericSignal('opacities', getTwoDimensionalOpacities(opacities)),
 		getGenericSignal('hiddenSeries', hiddenSeries ?? []),
 		getGenericSignal(HIGHLIGHTED_ITEM),
+		getGenericSignal(HIGHLIGHTED_SERIES, highlightedSeries),
 	];
 };
 

diff --git a/src/specBuilder/donut/donutSpecBuilder.test.ts b/src/specBuilder/donut/donutSpecBuilder.test.ts
@@ -13,13 +13,14 @@
 import { createElement } from 'react';
 
 import { ChartPopover } from '@components/ChartPopover';
-import { COLOR_SCALE, FILTERED_TABLE, HIGHLIGHTED_ITEM, MARK_ID } from '@constants';
+import { COLOR_SCALE, FILTERED_TABLE, HIGHLIGHTED_ITEM, HIGHLIGHTED_SERIES, MARK_ID } from '@constants';
 import { DonutSpecProps } from 'types';
 
 import { defaultHighlightSignal } from '@specBuilder/signal/signalSpecBuilder.test';
-import { defaultSignals } from '@specBuilder/specTestUtils';
+import { defaultHighlightedSeriesSignal, defaultSignals } from '@specBuilder/specTestUtils';
 import { addData, addDonut, addMarks, addScales, addSignals } from './donutSpecBuilder';
 import { getAggregateMetricMark, getArcMark, getDirectLabelMark, getPercentMetricMark } from './donutUtils';
+import { ChartTooltip } from '@components/ChartTooltip';
 
 const defaultDonutProps: DonutSpecProps = {
 	index: 0,
@@ -169,13 +170,19 @@ describe('addMarks', () => {
 });
 
 describe('addSignals()', () => {
-	test('should add signals correctly when there is no popover', () => {
+	test('should not add signals when there are not interactive children', () => {
 		const signals = addSignals(defaultSignals, defaultDonutProps);
-		expect(signals).toHaveLength(1);
+		expect(signals).toStrictEqual(defaultSignals);
+	});
+
+	test('should add hover events when tooltip is present', () => {
+		const signals = addSignals(defaultSignals, { ...defaultDonutProps, children: [createElement(ChartTooltip)] });
+		expect(signals).toHaveLength(2);
 		expect(signals[0]).toHaveProperty('name', HIGHLIGHTED_ITEM);
 		expect(signals[0].on).toHaveLength(2);
 		expect(signals[0].on?.[0]).toHaveProperty('events', '@testName:mouseover');
 		expect(signals[0].on?.[1]).toHaveProperty('events', '@testName:mouseout');
+		expect(signals[1]).toStrictEqual(defaultHighlightedSeriesSignal);
 	});
 
 	test('doesnt double add hovoredId signal', () => {
@@ -217,12 +224,13 @@ describe('addSignals()', () => {
 
 	test('should add signals correctly when there is a popover', () => {
 		const signals = addSignals(defaultSignals, { ...defaultDonutProps, children: [createElement(ChartPopover)] });
-		expect(signals).toHaveLength(2);
+		expect(signals).toHaveLength(3);
 		expect(signals[0]).toHaveProperty('name', HIGHLIGHTED_ITEM);
 		expect(signals[0].on).toHaveLength(2);
 		expect(signals[0].on?.[0]).toHaveProperty('events', '@testName:mouseover');
 		expect(signals[0].on?.[1]).toHaveProperty('events', '@testName:mouseout');
-		expect(signals[1]).toHaveProperty('name', 'testName_selectedId');
+		expect(signals[1]).toHaveProperty('name', HIGHLIGHTED_SERIES);
+		expect(signals[2]).toHaveProperty('name', 'testName_selectedId');
 	});
 
 	test('doesnt double add selectedId signal', () => {