Merge pull request #298 from adobe/legendKeys

fix: improve legned prop keys to be more robust

src/specBuilder/legend/legendHighlightUtils.test.ts

@@ -9,7 +9,13 @@
  * OF ANY KIND, either express or implied. See the License for the specific language
  * governing permissions and limitations under the License.
  */
-import { DEFAULT_OPACITY_RULE, HIGHLIGHTED_SERIES, HIGHLIGHT_CONTRAST_RATIO, SERIES_ID } from '@constants';
+import {
+	DEFAULT_OPACITY_RULE,
+	HIGHLIGHTED_GROUP,
+	HIGHLIGHTED_SERIES,
+	HIGHLIGHT_CONTRAST_RATIO,
+	SERIES_ID,
+} from '@constants';
 import { Mark } from 'vega';
 
 import { getHighlightOpacityRule, setHoverOpacityForMarks } from './legendHighlightUtils';
@@ -38,8 +44,11 @@ describe('getHighlightOpacityRule()', () => {
 		);
 	});
 	test('should use keys in test if there are keys', () => {
-		const opacityRule = getHighlightOpacityRule(['key1'], 'scatter0');
-		expect(opacityRule).toHaveProperty('test', 'scatter0_highlight && scatter0_highlight !== datum.key1');
+		const opacityRule = getHighlightOpacityRule(['key1'], 'legend0');
+		expect(opacityRule).toHaveProperty(
+			'test',
+			`${HIGHLIGHTED_GROUP} && ${HIGHLIGHTED_GROUP} !== datum.legend0_groupId`
+		);
 	});
 });
 

src/specBuilder/legend/legendHighlightUtils.ts

@@ -9,7 +9,7 @@
  * OF ANY KIND, either express or implied. See the License for the specific language
  * governing permissions and limitations under the License.
  */
-import { COLOR_SCALE, HIGHLIGHTED_SERIES, HIGHLIGHT_CONTRAST_RATIO, SERIES_ID } from '@constants';
+import { COLOR_SCALE, HIGHLIGHTED_GROUP, HIGHLIGHTED_SERIES, HIGHLIGHT_CONTRAST_RATIO, SERIES_ID } from '@constants';
 import { GroupMark, Mark, NumericValueRef } from 'vega';
 
 /**
@@ -31,7 +31,7 @@ export const setHoverOpacityForMarks = (marks: Mark[], keys?: string[], name?: s
 		const { opacity } = update;
 
 		if (opacity !== undefined) {
-			// // the new production rule for highlighting
+			// the new production rule for highlighting
 			const highlightOpacityRule = getHighlightOpacityRule(keys, name);
 
 			if (!Array.isArray(update.opacity)) {
@@ -46,8 +46,8 @@ export const setHoverOpacityForMarks = (marks: Mark[], keys?: string[], name?: s
 
 export const getHighlightOpacityRule = (keys?: string[], name?: string): { test?: string } & NumericValueRef => {
 	let test = `${HIGHLIGHTED_SERIES} && ${HIGHLIGHTED_SERIES} !== datum.${SERIES_ID}`;
-	if (keys) {
-		test = `${name}_highlight && ${name}_highlight !== datum.${keys[0]}`;
+	if (keys?.length) {
+		test = `${HIGHLIGHTED_GROUP} && ${HIGHLIGHTED_GROUP} !== datum.${name}_groupId`;
 	}
 	return { test, value: 1 / HIGHLIGHT_CONTRAST_RATIO };
 };

src/specBuilder/legend/legendSpecBuilder.test.ts

@@ -26,6 +26,7 @@ import {
 	defaultSelectedSeriesSignal,
 	defaultSignals,
 } from '@specBuilder/specTestUtils';
+import { baseData } from '@specBuilder/specUtils';
 import { Data, Legend, LegendEncode, Scale, Spec, SymbolEncodeEntry } from 'vega';
 
 import { addData, addLegend, addSignals, formatFacetRefsWithPresets, getContinuousLegend } from './legendSpecBuilder';
@@ -204,8 +205,8 @@ describe('addLegend()', () => {
 						...hiddenSeriesLabelUpdateEncoding,
 						text: [
 							{
-								test: "indexof(pluck(legendLabels, 'seriesName'), datum.value) > -1",
-								signal: "legendLabels[indexof(pluck(legendLabels, 'seriesName'), datum.value)].label",
+								test: "indexof(pluck(legend0_labels, 'seriesName'), datum.value) > -1",
+								signal: "legend0_labels[indexof(pluck(legend0_labels, 'seriesName'), datum.value)].label",
 							},
 							{
 								signal: 'datum.value',
@@ -233,8 +234,8 @@ describe('addLegend()', () => {
 						...defaultHighlightLegendEncoding.labels?.update,
 						text: [
 							{
-								test: "indexof(pluck(legendLabels, 'seriesName'), datum.value) > -1",
-								signal: "legendLabels[indexof(pluck(legendLabels, 'seriesName'), datum.value)].label",
+								test: "indexof(pluck(legend0_labels, 'seriesName'), datum.value) > -1",
+								signal: "legend0_labels[indexof(pluck(legend0_labels, 'seriesName'), datum.value)].label",
 							},
 							{
 								signal: 'datum.value',
@@ -256,7 +257,7 @@ describe('addLegend()', () => {
 			).toStrictEqual([
 				...defaultSignals,
 				{
-					name: 'legendLabels',
+					name: 'legend0_labels',
 					value: [
 						{ seriesName: 1, label: 'Any event' },
 						{ seriesName: 2, label: 'Any event' },
@@ -325,6 +326,19 @@ describe('addData()', () => {
 			},
 		]);
 	});
+	test('should add legend group Id if keys has length', () => {
+		const data = addData(baseData, { ...defaultLegendProps, facets: [DEFAULT_COLOR], keys: ['key1', 'key2'] });
+		expect(data[0].transform).toHaveLength(2);
+		expect(data[0].transform?.[1]).toHaveProperty('as', 'legend0_groupId');
+	});
+	test('should add transform to table if they do not exist', () => {
+		const data = addData([{ ...baseData[0], transform: undefined }, ...baseData], {
+			...defaultLegendProps,
+			facets: [DEFAULT_COLOR],
+			keys: ['key1', 'key2'],
+		});
+		expect(data[0].transform).toHaveLength(1);
+	});
 });
 
 describe('formatFacetRefsWithPresets()', () => {
@@ -379,7 +393,7 @@ describe('addSignals()', () => {
 	test('should add legendLabels signal if legendLabels are defined', () => {
 		expect(
 			addSignals(defaultSignals, { ...defaultLegendProps, legendLabels: [] }).find(
-				(signal) => signal.name === 'legendLabels'
+				(signal) => signal.name === 'legend0_labels'
 			)
 		).toBeDefined();
 	});

src/specBuilder/legend/legendSpecBuilder.ts

@@ -18,6 +18,7 @@ import {
 	SYMBOL_SHAPE_SCALE,
 	SYMBOL_SIZE_SCALE,
 } from '@constants';
+import { getTableData } from '@specBuilder/data/dataUtils';
 import { addFieldToFacetScaleDomain } from '@specBuilder/scale/scaleSpecBuilder';
 import {
 	getColorValue,
@@ -38,11 +39,7 @@ import {
 	LineWidthFacet,
 	SymbolShapeFacet,
 } from '../../types';
-import {
-	addHighlighSignalLegendHoverEvents,
-	getLegendLabelsSeriesSignal,
-	hasSignalByName,
-} from '../signal/signalSpecBuilder';
+import { addHighlighSignalLegendHoverEvents, getGenericSignal } from '../signal/signalSpecBuilder';
 import { getFacets, getFacetsFromKeys } from './legendFacetUtils';
 import { setHoverOpacityForMarks } from './legendHighlightUtils';
 import { Facet, getColumns, getEncodings, getHiddenEntriesFilter, getSymbolType } from './legendUtils';
@@ -255,7 +252,7 @@ const addMarks = produce<Mark[], [LegendSpecProps]>((marks, { highlight, keys, n
  * Each unique combination gets joined with a pipe to create a single string to use as legend entries
  */
 export const addData = produce<Data[], [LegendSpecProps & { facets: string[] }]>(
-	(data, { facets, hiddenEntries, name }) => {
+	(data, { facets, hiddenEntries, keys, name }) => {
 		// expression for combining all the facets into a single key
 		const expr = facets.map((facet) => `datum.${facet}`).join(' + " | " + ');
 		data.push({
@@ -274,19 +271,29 @@ export const addData = produce<Data[], [LegendSpecProps & { facets: string[] }]>
 				...getHiddenEntriesFilter(hiddenEntries, name),
 			],
 		});
+
+		if (keys?.length) {
+			const tableData = getTableData(data);
+			if (!tableData.transform) {
+				tableData.transform = [];
+			}
+			tableData.transform.push({
+				type: 'formula',
+				as: `${name}_groupId`,
+				expr: keys.map((key) => `datum.${key}`).join(' + " | " + '),
+			});
+		}
 	}
 );
 
 export const addSignals = produce<Signal[], [LegendSpecProps]>(
-	(signals, { hiddenSeries, highlight, isToggleable, legendLabels, name }) => {
+	(signals, { hiddenSeries, highlight, isToggleable, keys, legendLabels, name }) => {
 		if (highlight) {
-			addHighlighSignalLegendHoverEvents(signals, name, Boolean(isToggleable || hiddenSeries));
+			addHighlighSignalLegendHoverEvents(signals, name, Boolean(isToggleable || hiddenSeries), keys);
 		}
 
 		if (legendLabels) {
-			if (!hasSignalByName(signals, 'legendLabels')) {
-				signals.push(getLegendLabelsSeriesSignal(legendLabels));
-			}
+			signals.push(getGenericSignal(`${name}_labels`, legendLabels));
 		}
 	}
 );

src/specBuilder/legend/legendUtils.ts

@@ -12,6 +12,7 @@
 import {
 	COLOR_SCALE,
 	DEFAULT_OPACITY_RULE,
+	HIGHLIGHTED_GROUP,
 	HIGHLIGHTED_SERIES,
 	HIGHLIGHT_CONTRAST_RATIO,
 	LINE_TYPE_SCALE,
@@ -87,22 +88,22 @@ export const getHiddenEntriesFilter = (hiddenEntries: string[], name: string): F
 export const getEncodings = (facets: Facet[], legendProps: LegendSpecProps): LegendEncode => {
 	const symbolEncodings = getSymbolEncodings(facets, legendProps);
 	const hoverEncodings = getHoverEncodings(facets, legendProps);
-	const legendLabelsEncodings = getLegendLabelsEncodings(legendProps.legendLabels);
+	const legendLabelsEncodings = getLegendLabelsEncodings(legendProps.name, legendProps.legendLabels);
 	const showHideEncodings = getShowHideEncodings(legendProps);
 	// merge the encodings together
 	return mergeLegendEncodings([symbolEncodings, legendLabelsEncodings, hoverEncodings, showHideEncodings]);
 };
 
-const getLegendLabelsEncodings = (legendLabels: LegendLabel[] | undefined): LegendEncode => {
+const getLegendLabelsEncodings = (name: string, legendLabels: LegendLabel[] | undefined): LegendEncode => {
 	if (legendLabels) {
 		return {
 			labels: {
 				update: {
 					text: [
 						{
 							// Test whether a legendLabel exists for the seriesName, if not use the seriesName
-							test: "indexof(pluck(legendLabels, 'seriesName'), datum.value) > -1",
-							signal: "legendLabels[indexof(pluck(legendLabels, 'seriesName'), datum.value)].label",
+							test: `indexof(pluck(${name}_labels, 'seriesName'), datum.value) > -1`,
+							signal: `${name}_labels[indexof(pluck(${name}_labels, 'seriesName'), datum.value)].label`,
 						},
 						{ signal: 'datum.value' },
 					],
@@ -169,9 +170,8 @@ export const getOpacityEncoding = ({
 	highlight,
 	highlightedSeries,
 	keys,
-	name,
 }: LegendSpecProps): ProductionRule<NumericValueRef> | undefined => {
-	const highlightSignalName = keys ? `${name}_highlight` : HIGHLIGHTED_SERIES;
+	const highlightSignalName = keys?.length ? HIGHLIGHTED_GROUP : HIGHLIGHTED_SERIES;
 	// only add symbol opacity if highlight is true or highlightedSeries is defined
 	if (highlight || highlightedSeries) {
 		return [

src/specBuilder/signal/signalSpecBuilder.ts

@@ -9,7 +9,7 @@
  * OF ANY KIND, either express or implied. See the License for the specific language
  * governing permissions and limitations under the License.
  */
-import { HIGHLIGHTED_ITEM, HIGHLIGHTED_SERIES, MARK_ID, SERIES_ID } from '@constants';
+import { HIGHLIGHTED_GROUP, HIGHLIGHTED_ITEM, HIGHLIGHTED_SERIES, MARK_ID, SERIES_ID } from '@constants';
 import { Signal } from 'vega';
 
 /**
@@ -49,9 +49,11 @@ export const getControlledHoveredGroupSignal = (name: string): Signal => {
 export const addHighlighSignalLegendHoverEvents = (
 	signals: Signal[],
 	legendName: string,
-	includeHiddenSeries: boolean
+	includeHiddenSeries: boolean,
+	keys?: string[]
 ) => {
-	const highlightedItemSignal = signals.find((signal) => signal.name === HIGHLIGHTED_SERIES);
+	const signalName = keys?.length ? HIGHLIGHTED_GROUP : HIGHLIGHTED_SERIES;
+	const highlightedItemSignal = signals.find((signal) => signal.name === signalName);
 	if (highlightedItemSignal) {
 		if (highlightedItemSignal.on === undefined) {
 			highlightedItemSignal.on = [];