Skip to content

Commit 5cc1936

Browse files
marshallpetemarshallpete
authored
Merge pull request #636 from adobe/highlightingRefactor
refactor: get rid of old highlighted items signal events
2 parents e437128 + efd7acc commit 5cc1936

38 files changed

+286
-332
lines changed

‎packages/constants/constants.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ export const HIGHLIGHTED_SERIES = 'highlightedSeries'; // series
7979
export const SELECTED_ITEM = 'selectedItem'; // data point
8080
export const SELECTED_SERIES = 'selectedSeries'; // series
8181
export const SELECTED_GROUP = 'selectedGroup'; // data point
82-
export const MOUSE_OVER_SERIES = 'mousedOverSeries'; // mouse over series for dual y-axis
8382
export const FIRST_RSC_SERIES_ID = 'firstRscSeriesId'; // first series for dual y-axis
8483
export const LAST_RSC_SERIES_ID = 'lastRscSeriesId'; // last series for dual y-axis
8584

‎packages/react-spectrum-charts/src/stories/components/Combo/Combo.test.tsx‎

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,10 @@
1212
import React from 'react';
1313

1414
import { Combo } from '../../../alpha';
15-
import { findAllMarksByGroupName, findChart, hoverNthElement, render, screen, within } from '../../../test-utils';
15+
import { findAllMarksByGroupName, findChart, findMarksByGroupName, hoverNthElement, render, screen, unhoverNthElement, within } from '../../../test-utils';
1616
import '../../../test-utils/__mocks__/matchMedia.mock.js';
1717
import { Basic, DualAxis, Tooltip } from './Combo.story';
18+
import { FADE_FACTOR } from '@spectrum-charts/constants';
1819

1920
describe('Combo', () => {
2021
// Combo is not a real React component. This test just provides test coverage for sonarqube
@@ -66,6 +67,35 @@ describe('Combo', () => {
6667
expect(within(tooltip).getByText('Nov 8')).toBeInTheDocument();
6768
});
6869

70+
test('Correct items are highlighted when hovering over a combo', async () => {
71+
render(<Tooltip {...Tooltip.args} />);
72+
const chart = await findChart();
73+
expect(chart).toBeInTheDocument();
74+
75+
const bars = await findAllMarksByGroupName(chart, 'combo0Bar0');
76+
const lines = await findAllMarksByGroupName(chart, 'combo0Line0');
77+
78+
const paths = await findAllMarksByGroupName(chart, 'combo0Line0_hover0');
79+
80+
// hover and validate all hover components are visible
81+
await hoverNthElement(paths, 0);
82+
expect(lines[0]).toHaveAttribute('opacity', '1');
83+
expect(bars[0]).toHaveAttribute('opacity', `${FADE_FACTOR}`);
84+
expect(bars[1]).toHaveAttribute('opacity', `${FADE_FACTOR}`);
85+
86+
const highlightRule = await findMarksByGroupName(chart, 'combo0Line0_hoverRule', 'line');
87+
expect(highlightRule).toBeInTheDocument();
88+
const highlightPoint = await findMarksByGroupName(chart, 'combo0Line0_point_highlight');
89+
expect(highlightPoint).toBeInTheDocument();
90+
91+
await unhoverNthElement(paths, 0);
92+
93+
await hoverNthElement(bars, 0);
94+
expect(lines[0]).toHaveAttribute('opacity', `${FADE_FACTOR}`);
95+
expect(bars[0]).toHaveAttribute('opacity', '1');
96+
expect(bars[1]).toHaveAttribute('opacity', `${FADE_FACTOR}`);
97+
});
98+
6999
test('Dual Axis renders properly', async () => {
70100
render(<DualAxis {...DualAxis.args} />);
71101
const chart = await findChart();

‎packages/vega-spec-builder/src/area/areaSpecBuilder.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ export const addSignals = produce<Signal[], [AreaSpecOptions]>((signals, areaOpt
192192
const { chartTooltips, name } = areaOptions;
193193
if (!isInteractive(areaOptions)) return;
194194
addHighlightedSeriesSignalEvents(signals, name, 1, chartTooltips[0]?.excludeDataKeys);
195-
addHoveredItemSignal(signals, name);
195+
addHoveredItemSignal(signals, name, undefined, 1, chartTooltips[0]?.excludeDataKeys);
196196
if (areaOptions.highlightedItem) {
197197
addHighlightedItemEvents(signals, name);
198198
}

‎packages/vega-spec-builder/src/area/areaUtils.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -108,7 +108,8 @@ export function getAreaOpacity(areaOptions: AreaMarkOptions): ProductionRule<Num
108108
return [
109109
{ test: `isValid(${HIGHLIGHTED_SERIES}) && ${HIGHLIGHTED_SERIES} === datum.${SERIES_ID}`, value: 1 },
110110
{
111-
test: `isValid(${HIGHLIGHTED_ITEM}) && indexof(pluck(data('${name}_highlightedData'), '${SERIES_ID}'), datum.${SERIES_ID}) > -1`,
111+
test: `isArray(${HIGHLIGHTED_ITEM}) && indexof(pluck(data('${name}_highlightedData'), '${SERIES_ID}'), datum.${SERIES_ID}) > -1`,
112+
value: 1
112113
},
113114
{ test: `isValid(${SELECTED_SERIES}) && ${SELECTED_SERIES} === datum.${SERIES_ID}`, value: 1 },
114115
{ value: 0 },

‎packages/vega-spec-builder/src/axis/axisSpecBuilder.test.ts‎

Lines changed: 24 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,10 @@ import { Axis, ColorValueRef, GroupMark, NumericValueRef, ProductionRule, Scale,
1313

1414
import {
1515
DEFAULT_LABEL_FONT_WEIGHT,
16-
FILTERED_TABLE,
1716
FADE_FACTOR,
17+
FILTERED_TABLE,
1818
LAST_RSC_SERIES_ID,
19-
MOUSE_OVER_SERIES
19+
SERIES_ID,
2020
} from '@spectrum-charts/constants';
2121

2222
import { SubLabel } from '../types';
@@ -305,12 +305,12 @@ describe('Spec builder, Axis', () => {
305305
position: 'left',
306306
scaleName: 'yLinear',
307307
scaleType: 'linear',
308-
usermeta: {},
308+
usermeta: { interactiveMarks: ['bar0'] },
309309
})[0].encode?.labels?.update?.fillOpacity;
310310
expect(labelFillOpacityEncoding).toHaveLength(1);
311311
expect(labelFillOpacityEncoding?.[0]).toEqual({
312-
test: `${MOUSE_OVER_SERIES} === ${LAST_RSC_SERIES_ID}`,
313-
value: FADE_FACTOR,
312+
test: 'isValid(bar0_hoveredItem)',
313+
signal: `bar0_hoveredItem.${SERIES_ID} !== lastRscSeriesId ? 1 : ${FADE_FACTOR}`,
314314
});
315315
});
316316

@@ -338,12 +338,12 @@ describe('Spec builder, Axis', () => {
338338
position: 'left',
339339
scaleName: 'yLinear',
340340
scaleType: 'linear',
341-
usermeta: {},
341+
usermeta: { interactiveMarks: ['bar0'] },
342342
})[0].encode?.title?.update?.fillOpacity;
343343
expect(titleFillOpacityEncoding).toHaveLength(1);
344344
expect(titleFillOpacityEncoding?.[0]).toEqual({
345-
test: `${MOUSE_OVER_SERIES} === ${LAST_RSC_SERIES_ID}`,
346-
value: FADE_FACTOR,
345+
test: 'isValid(bar0_hoveredItem)',
346+
signal: `bar0_hoveredItem.${SERIES_ID} !== lastRscSeriesId ? 1 : ${FADE_FACTOR}`,
347347
});
348348
});
349349

@@ -373,12 +373,12 @@ describe('Spec builder, Axis', () => {
373373
scaleName: 'yLinear',
374374
scaleType: 'linear',
375375
subLabels: defaultSubLabels,
376-
usermeta: {},
376+
usermeta: { interactiveMarks: ['bar0'] },
377377
})[1].encode?.labels?.update?.fillOpacity;
378378
expect(labelFillOpacityEncoding).toHaveLength(1);
379379
expect(labelFillOpacityEncoding?.[0]).toEqual({
380-
test: `${MOUSE_OVER_SERIES} === ${LAST_RSC_SERIES_ID}`,
381-
value: FADE_FACTOR,
380+
test: 'isValid(bar0_hoveredItem)',
381+
signal: `bar0_hoveredItem.${SERIES_ID} !== lastRscSeriesId ? 1 : ${FADE_FACTOR}`,
382382
});
383383
});
384384

@@ -429,12 +429,12 @@ describe('Spec builder, Axis', () => {
429429
position: 'left',
430430
scaleName: 'yLinear',
431431
scaleType: 'linear',
432-
usermeta: { metricAxisCount: 1 },
432+
usermeta: { metricAxisCount: 1, interactiveMarks: ['bar0'] },
433433
})[0].encode?.labels?.update?.fillOpacity;
434434
expect(labelFillOpacityEncoding).toHaveLength(1);
435435
expect(labelFillOpacityEncoding?.[0]).toEqual({
436-
test: `isValid(${MOUSE_OVER_SERIES}) && ${MOUSE_OVER_SERIES} !== ${LAST_RSC_SERIES_ID}`,
437-
value: FADE_FACTOR,
436+
test: 'isValid(bar0_hoveredItem)',
437+
signal: `bar0_hoveredItem.${SERIES_ID} === lastRscSeriesId ? 1 : ${FADE_FACTOR}`,
438438
});
439439
});
440440

@@ -460,12 +460,12 @@ describe('Spec builder, Axis', () => {
460460
scaleName: 'yLinear',
461461
scaleType: 'linear',
462462
subLabels: defaultSubLabels,
463-
usermeta: { metricAxisCount: 1 },
463+
usermeta: { metricAxisCount: 1, interactiveMarks: ['bar0'] },
464464
})[1].encode?.labels?.update?.fillOpacity;
465465
expect(labelFillOpacityEncoding).toHaveLength(1);
466466
expect(labelFillOpacityEncoding?.[0]).toEqual({
467-
test: `isValid(${MOUSE_OVER_SERIES}) && ${MOUSE_OVER_SERIES} !== ${LAST_RSC_SERIES_ID}`,
468-
value: FADE_FACTOR,
467+
test: 'isValid(bar0_hoveredItem)',
468+
signal: `bar0_hoveredItem.${SERIES_ID} === lastRscSeriesId ? 1 : ${FADE_FACTOR}`,
469469
});
470470
});
471471

@@ -849,25 +849,18 @@ describe('Spec builder, Axis', () => {
849849
const titleFillOpacity = marks[0].axes?.[0].encode?.title?.update
850850
?.fillOpacity as ProductionRule<NumericValueRef>[];
851851

852-
function getDualMetricAxisFillEncoding(encoding: ProductionRule<ColorValueRef>[]) {
853-
return encoding?.find(
854-
(rule) =>
855-
'test' in rule &&
856-
rule.test === `isValid(${MOUSE_OVER_SERIES}) && ${MOUSE_OVER_SERIES} !== ${LAST_RSC_SERIES_ID}`
857-
);
858-
}
859-
function getDualMetricAxisFillOpacityEncoding(encoding: ProductionRule<NumericValueRef>[]) {
852+
function getDualMetricAxisEncoding(encoding: ProductionRule<ColorValueRef>[] | ProductionRule<NumericValueRef>[]) {
860853
return encoding?.find(
861854
(rule) =>
862-
'test' in rule &&
863-
rule.test === `isValid(${MOUSE_OVER_SERIES}) && ${MOUSE_OVER_SERIES} !== ${LAST_RSC_SERIES_ID}`
855+
'signal' in rule &&
856+
rule.signal.includes(LAST_RSC_SERIES_ID)
864857
);
865858
}
866859

867-
expect(getDualMetricAxisFillEncoding(labelFillEncoding)).toBeUndefined();
868-
expect(getDualMetricAxisFillOpacityEncoding(labelFillOpacity)).toBeUndefined();
869-
expect(getDualMetricAxisFillEncoding(titleFillUpdate)).toBeUndefined();
870-
expect(getDualMetricAxisFillOpacityEncoding(titleFillOpacity)).toBeUndefined();
860+
expect(getDualMetricAxisEncoding(labelFillEncoding)).toBeUndefined();
861+
expect(getDualMetricAxisEncoding(labelFillOpacity)).toBeUndefined();
862+
expect(getDualMetricAxisEncoding(titleFillUpdate)).toBeUndefined();
863+
expect(getDualMetricAxisEncoding(titleFillOpacity)).toBeUndefined();
871864
});
872865
});
873866
});

‎packages/vega-spec-builder/src/axis/axisSpecBuilder.ts‎

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,11 @@ import {
2929
DEFAULT_LABEL_ALIGN,
3030
DEFAULT_LABEL_FONT_WEIGHT,
3131
DEFAULT_LABEL_ORIENTATION,
32-
FIRST_RSC_SERIES_ID,
3332
FADE_FACTOR,
33+
FIRST_RSC_SERIES_ID,
34+
HOVERED_ITEM,
3435
LAST_RSC_SERIES_ID,
35-
MOUSE_OVER_SERIES
36+
SERIES_ID
3637
} from '@spectrum-charts/constants';
3738
import { spectrumColors } from '@spectrum-charts/themes';
3839

@@ -232,31 +233,29 @@ export const getLabelSignalValue = (
232233
* Applies fill and opacity encoding rules to the secondary metric axis
233234
* @param axis Axis to apply encoding rules to
234235
*/
235-
export function applySecondaryMetricAxisEncodings(axis: Axis): void {
236+
export function applySecondaryMetricAxisEncodings(axis: Axis, interactiveMarks: string[]): void {
236237
const secondaryAxisFillRules = [{ signal: `scale('${COLOR_SCALE}', ${LAST_RSC_SERIES_ID})` }];
237238

238-
const secondaryAxisFillOpacityRules = [
239-
{
240-
test: `isValid(${MOUSE_OVER_SERIES}) && ${MOUSE_OVER_SERIES} !== ${LAST_RSC_SERIES_ID}`,
241-
value: FADE_FACTOR,
242-
},
243-
];
239+
const fillOpacity = interactiveMarks.map((interactiveMark) => ({
240+
test: `isValid(${interactiveMark}_${HOVERED_ITEM})`,
241+
signal: `${interactiveMark}_${HOVERED_ITEM}.${SERIES_ID} === ${LAST_RSC_SERIES_ID} ? 1 : ${FADE_FACTOR}`,
242+
}));
244243

245244
const encodings = {
246245
labels: {
247246
enter: {
248247
fill: secondaryAxisFillRules,
249248
},
250249
update: {
251-
fillOpacity: secondaryAxisFillOpacityRules,
250+
fillOpacity,
252251
},
253252
},
254253
title: {
255254
enter: {
256255
fill: secondaryAxisFillRules,
257256
},
258257
update: {
259-
fillOpacity: secondaryAxisFillOpacityRules,
258+
fillOpacity,
260259
},
261260
},
262261
};
@@ -273,7 +272,7 @@ export function applySecondaryMetricAxisEncodings(axis: Axis): void {
273272
* @param axis Axis to apply encoding rules to
274273
* @param colorScheme The color scheme (light or dark)
275274
*/
276-
export function applyPrimaryMetricAxisEncodings(axis: Axis, colorScheme: ColorScheme = DEFAULT_COLOR_SCHEME): void {
275+
export function applyPrimaryMetricAxisEncodings(axis: Axis, interactiveMarks: string[], colorScheme: ColorScheme = DEFAULT_COLOR_SCHEME): void {
277276
// Get the appropriate font color value based on the colorScheme (light/dark theme)
278277
const defaultFontColor = spectrumColors[colorScheme][DEFAULT_FONT_COLOR];
279278

@@ -284,24 +283,23 @@ export function applyPrimaryMetricAxisEncodings(axis: Axis, colorScheme: ColorSc
284283
},
285284
{ value: defaultFontColor },
286285
];
287-
const primaryAxisFillOpacityRules = [
288-
{
289-
test: `${MOUSE_OVER_SERIES} === ${LAST_RSC_SERIES_ID}`,
290-
value: FADE_FACTOR,
291-
},
292-
];
286+
287+
const fillOpacity = interactiveMarks.map((interactiveMark) => ({
288+
test: `isValid(${interactiveMark}_${HOVERED_ITEM})`,
289+
signal: `${interactiveMark}_${HOVERED_ITEM}.${SERIES_ID} !== ${LAST_RSC_SERIES_ID} ? 1 : ${FADE_FACTOR}`,
290+
}));
293291

294292
const encodings = {
295293
labels: {
296294
update: {
297295
fill: primaryAxisFillRules,
298-
fillOpacity: primaryAxisFillOpacityRules,
296+
fillOpacity,
299297
},
300298
},
301299
title: {
302300
update: {
303301
fill: primaryAxisFillRules,
304-
fillOpacity: primaryAxisFillOpacityRules,
302+
fillOpacity,
305303
},
306304
},
307305
};
@@ -323,17 +321,18 @@ export function addDualMetricAxisConfig(
323321
axis: Axis,
324322
isPrimaryMetricAxis: boolean,
325323
scaleName: string,
326-
colorScheme: ColorScheme = DEFAULT_COLOR_SCHEME
324+
interactiveMarks: string[],
325+
colorScheme: ColorScheme = DEFAULT_COLOR_SCHEME,
327326
) {
328327
const scaleNames = getDualAxisScaleNames(scaleName);
329328
const { primaryScale, secondaryScale } = scaleNames;
330329

331330
if (isPrimaryMetricAxis) {
332331
axis.scale = primaryScale;
333-
applyPrimaryMetricAxisEncodings(axis, colorScheme);
332+
applyPrimaryMetricAxisEncodings(axis, interactiveMarks, colorScheme);
334333
} else {
335334
axis.scale = secondaryScale;
336-
applySecondaryMetricAxisEncodings(axis);
335+
applySecondaryMetricAxisEncodings(axis, interactiveMarks);
337336
}
338337
}
339338

@@ -522,7 +521,7 @@ const handleDualMetricAxisConfig = ({
522521
if (!usermeta.metricAxisCount) {
523522
usermeta.metricAxisCount = 0;
524523
}
525-
addDualMetricAxisConfig(axis, usermeta.metricAxisCount === 0, scaleName, colorScheme);
524+
addDualMetricAxisConfig(axis, usermeta.metricAxisCount === 0, scaleName, usermeta.interactiveMarks ?? [], colorScheme);
526525
if (incrementMetricAxisCount) {
527526
usermeta.metricAxisCount++;
528527
}

‎packages/vega-spec-builder/src/bar/barSpecBuilder.test.ts‎

Lines changed: 8 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -242,6 +242,7 @@ const defaultSpec: ScSpec = {
242242
],
243243
usermeta: {
244244
chartOrientation: 'vertical',
245+
interactiveMarks: ['bar0'],
245246
},
246247
};
247248

@@ -275,19 +276,19 @@ describe('barSpecBuilder', () => {
275276
});
276277
test('should add hover events if tooltip is present', () => {
277278
const signals = addSignals(defaultSignals, { ...defaultBarOptions, chartTooltips: [{}] });
278-
expect(signals[0]).toHaveProperty('on');
279-
expect(signals[0].on).toHaveLength(2);
280-
expect(signals[0].on?.[0]).toHaveProperty('events', '@bar0:mouseover');
279+
expect(signals.at(-1)).toHaveProperty('on');
280+
expect(signals.at(-1)?.on).toHaveLength(2);
281+
expect(signals.at(-1)?.on?.[0]).toHaveProperty('events', '@bar0:mouseover');
281282
});
282283
test('should exclude data with key from update if tooltip has excludeDataKey', () => {
283284
const signals = addSignals(defaultSignals, {
284285
...defaultBarOptions,
285286
chartTooltips: [{ excludeDataKeys: ['excludeFromTooltip'] }],
286287
});
287-
expect(signals[0]).toHaveProperty('on');
288-
expect(signals[0].on).toHaveLength(2);
289-
expect(signals[0].on?.[0]).toHaveProperty('events', '@bar0:mouseover');
290-
expect(signals[0].on?.[0]).toHaveProperty('update', '(datum.excludeFromTooltip) ? null : datum.rscMarkId');
288+
expect(signals.at(-1)).toHaveProperty('on');
289+
expect(signals.at(-1)?.on).toHaveLength(2);
290+
expect(signals.at(-1)?.on?.[0]).toHaveProperty('events', '@bar0:mouseover');
291+
expect(signals.at(-1)?.on?.[0]).toHaveProperty('update', '(datum.excludeFromTooltip) ? null : datum');
291292
});
292293

293294
describe('dualMetricAxis signals', () => {
@@ -303,21 +304,6 @@ describe('barSpecBuilder', () => {
303304
return signal.name === 'lastRscSeriesId';
304305
}
305306

306-
test('should add mousedOverSeries if dualMetricAxis is true and type is dodged', () => {
307-
const signals = addSignals(defaultSignals, {
308-
...defaultBarOptions,
309-
type: 'dodged',
310-
dualMetricAxis: true,
311-
});
312-
const mousedOverSeriesSignal = signals.find(getMousedOverSeriesSignal);
313-
// update to moused over series id on mouseover
314-
expect(mousedOverSeriesSignal?.on?.[0]).toHaveProperty('events', '@bar0:mouseover');
315-
expect(mousedOverSeriesSignal?.on?.[0]).toHaveProperty('update', 'datum.rscSeriesId');
316-
// update to null on mouseout
317-
expect(mousedOverSeriesSignal?.on?.[1]).toHaveProperty('events', '@bar0:mouseout');
318-
expect(mousedOverSeriesSignal?.on?.[1]).toHaveProperty('update', 'null');
319-
});
320-
321307
test('should add firstRscSeriesId if dualMetricAxis is true and type is dodged', () => {
322308
const signals = addSignals(defaultSignals, {
323309
...defaultBarOptions,

0 commit comments

Comments
 (0)