Skip to content

Commit fdc35ec

Browse files
marshallpetemarshallpete
authored
Merge pull request #635 from adobe/highlightingRefactor
refactor: change HILIGHT_CONTRAST_RATIO to FADE_FACTOR
2 parents d9ca6fb + 5066ef0 commit fdc35ec

27 files changed

+123
-131
lines changed

‎packages/constants/constants.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,11 @@ export const DEFAULT_VENN_LABEL = 'label';
128128

129129
// ratio that each opacity is divded by when hovering or highlighting from legend
130130
// TODO: invert this ratio so we don't have to do 1 / HIGHLIGHT_CONTRAST_RATIO every time
131+
/**
132+
* @deprecated
133+
*/
131134
export const HIGHLIGHT_CONTRAST_RATIO = 5;
135+
export const FADE_FACTOR = 0.2;
132136

133137
// legend tooltips
134138
export const TOOLTIP_DELAY = 350;

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
*/
1212
import { createRef } from 'react';
1313

14-
import { HIGHLIGHT_CONTRAST_RATIO } from '@spectrum-charts/constants';
14+
import { FADE_FACTOR } from '@spectrum-charts/constants';
1515
import { ChartHandle } from '@spectrum-charts/vega-spec-builder';
1616

1717
import { Chart } from '../Chart';
@@ -357,8 +357,8 @@ describe('Chart', () => {
357357
const bars = getAllMarksByGroupName(chart, 'bar0');
358358

359359
expect(bars[14]).toHaveAttribute('opacity', '1');
360-
expect(bars[13]).toHaveAttribute('opacity', `${1 / HIGHLIGHT_CONTRAST_RATIO}`);
361-
expect(bars[15]).toHaveAttribute('opacity', `${1 / HIGHLIGHT_CONTRAST_RATIO}`);
360+
expect(bars[13]).toHaveAttribute('opacity', `${FADE_FACTOR}`);
361+
expect(bars[15]).toHaveAttribute('opacity', `${FADE_FACTOR}`);
362362
});
363363
});
364364
});

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,10 @@
99
* OF ANY KIND, either express or implied. See the License for the specific language
1010
* governing permissions and limitations under the License.
1111
*/
12-
import React from 'react';
1312

1413
import userEvent from '@testing-library/user-event';
1514

16-
import { HIGHLIGHT_CONTRAST_RATIO } from '@spectrum-charts/constants';
15+
import { FADE_FACTOR } from '@spectrum-charts/constants';
1716

1817
import { Area } from '../../../components';
1918
import {
@@ -81,7 +80,7 @@ describe('Area', () => {
8180
const tooltip = await screen.findByTestId('rsc-tooltip');
8281
expect(tooltip).toBeInTheDocument();
8382
expect(within(tooltip).getByText('OS: Windows')).toBeInTheDocument();
84-
expect(areas[1].getAttribute('opacity')).toEqual((1 / HIGHLIGHT_CONTRAST_RATIO).toString());
83+
expect(areas[1].getAttribute('opacity')).toEqual(`${FADE_FACTOR}`);
8584

8685
const highlightRule = await findMarksByGroupName(chart, 'area0_rule', 'line');
8786
expect(highlightRule).toBeInTheDocument();

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

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,10 @@
99
* OF ANY KIND, either express or implied. See the License for the specific language
1010
* governing permissions and limitations under the License.
1111
*/
12-
import React from 'react';
1312

1413
import userEvent from '@testing-library/user-event';
1514

16-
import { HIGHLIGHT_CONTRAST_RATIO } from '@spectrum-charts/constants';
15+
import { FADE_FACTOR } from '@spectrum-charts/constants';
1716
import { spectrumColors } from '@spectrum-charts/themes';
1817

1918
import { ChartPopover } from '../../../components';
@@ -115,7 +114,7 @@ describe('ChartPopover', () => {
115114
expect(bars[0]).toHaveAttribute('stroke', spectrumColors.light['static-blue']);
116115
expect(bars[0]).toHaveAttribute('stroke-width', '2');
117116
// all other bars should be faded
118-
expect(allElementsHaveAttributeValue(bars.slice(1), 'opacity', 1 / HIGHLIGHT_CONTRAST_RATIO)).toBeTruthy();
117+
expect(allElementsHaveAttributeValue(bars.slice(1), 'opacity', FADE_FACTOR)).toBeTruthy();
119118
});
120119

121120
test('Popover should be corrrect size', async () => {
@@ -220,7 +219,7 @@ describe('ChartPopover', () => {
220219

221220
// validate the first line is still full opacity, but the other lines are faded
222221
expect(lines[0]).toHaveAttribute('opacity', '1');
223-
expect(allElementsHaveAttributeValue(lines.slice(1), 'opacity', 1 / HIGHLIGHT_CONTRAST_RATIO)).toBeTruthy();
222+
expect(allElementsHaveAttributeValue(lines.slice(1), 'opacity', FADE_FACTOR)).toBeTruthy();
224223
});
225224

226225
test('Dodged bar popover opens on mark click and closes when clicking outside', async () => {
@@ -243,7 +242,7 @@ describe('ChartPopover', () => {
243242
bars = getAllMarksByGroupName(chart, 'bar0');
244243

245244
// validate the highlight visuals are present
246-
expect(bars[0]).toHaveAttribute('opacity', `${1 / HIGHLIGHT_CONTRAST_RATIO}`);
245+
expect(bars[0]).toHaveAttribute('opacity', `${FADE_FACTOR}`);
247246
expect(bars[4]).toHaveAttribute('opacity', '1');
248247
expect(bars[4]).toHaveAttribute('stroke', spectrumColors.light['static-blue']);
249248
expect(bars[4]).toHaveAttribute('stroke-width', '2');
@@ -269,7 +268,7 @@ describe('ChartPopover', () => {
269268
bars = getAllMarksByGroupName(chart, 'bar0');
270269

271270
// validate the highlight visuals are present
272-
expect(bars[0]).toHaveAttribute('opacity', `${1 / HIGHLIGHT_CONTRAST_RATIO}`);
271+
expect(bars[0]).toHaveAttribute('opacity', `${FADE_FACTOR}`);
273272
expect(bars[4]).toHaveAttribute('opacity', '1');
274273

275274
const selectionRingMarks = getAllMarksByGroupName(chart, 'bar0_selectionRing');
@@ -319,7 +318,7 @@ describe('ChartPopover', () => {
319318
segments = getAllMarksByGroupName(chart, 'donut0');
320319

321320
// validate the highlight visuals are present
322-
expect(segments[0]).toHaveAttribute('opacity', `${1 / HIGHLIGHT_CONTRAST_RATIO}`);
321+
expect(segments[0]).toHaveAttribute('opacity', `${FADE_FACTOR}`);
323322
expect(segments[4]).toHaveAttribute('opacity', '1');
324323
expect(segments[4]).toHaveAttribute('stroke', spectrumColors.light['static-blue']);
325324
expect(segments[4]).toHaveAttribute('stroke-width', '2');

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

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,8 @@
99
* OF ANY KIND, either express or implied. See the License for the specific language
1010
* governing permissions and limitations under the License.
1111
*/
12-
import React from 'react';
1312

14-
import { HIGHLIGHT_CONTRAST_RATIO } from '@spectrum-charts/constants';
13+
import { FADE_FACTOR } from '@spectrum-charts/constants';
1514

1615
import { ChartTooltip } from '../../../components';
1716
import {
@@ -48,7 +47,7 @@ describe('ChartTooltip', () => {
4847
const tooltip = await screen.findByTestId('rsc-tooltip');
4948
expect(tooltip).toBeInTheDocument();
5049
expect(within(tooltip).getByText('Operating system: Windows')).toBeInTheDocument();
51-
expect(bars[1].getAttribute('opacity')).toEqual(`${1 / HIGHLIGHT_CONTRAST_RATIO}`);
50+
expect(bars[1].getAttribute('opacity')).toEqual(`${FADE_FACTOR}`);
5251

5352
// unhover and validate the highlights go away
5453
await unhoverNthElement(bars, 0);
@@ -99,7 +98,7 @@ describe('ChartTooltip', () => {
9998
expect(within(tooltip).getByText('Users: 3')).toBeInTheDocument();
10099

101100
// validate the highlight visuals are present
102-
expect(bars[0]).toHaveAttribute('opacity', `${1 / HIGHLIGHT_CONTRAST_RATIO}`);
101+
expect(bars[0]).toHaveAttribute('opacity', `${FADE_FACTOR}`);
103102
expect(bars[4]).toHaveAttribute('opacity', '1');
104103

105104
await unhoverNthElement(bars, 4);

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
* OF ANY KIND, either express or implied. See the License for the specific language
1010
* governing permissions and limitations under the License.
1111
*/
12-
import { HIGHLIGHT_CONTRAST_RATIO } from '@spectrum-charts/constants';
12+
import { FADE_FACTOR } from '@spectrum-charts/constants';
1313

1414
import {
1515
allElementsHaveAttributeValue,
@@ -37,7 +37,7 @@ describe('Basic', () => {
3737
expect(bars[2]).toHaveAttribute('opacity', '1');
3838
// all other bars
3939
expect(
40-
allElementsHaveAttributeValue([...bars.slice(0, 2), ...bars.slice(3)], 'opacity', 1 / HIGHLIGHT_CONTRAST_RATIO)
40+
allElementsHaveAttributeValue([...bars.slice(0, 2), ...bars.slice(3)], 'opacity', FADE_FACTOR)
4141
).toBe(true);
4242
});
4343
});
@@ -56,7 +56,7 @@ describe('Dimension', () => {
5656
// first three bars (same dimension)
5757
expect(allElementsHaveAttributeValue(bars.slice(0, 2), 'opacity', '1')).toBe(true);
5858
// all other bars
59-
expect(allElementsHaveAttributeValue(bars.slice(3), 'opacity', 1 / HIGHLIGHT_CONTRAST_RATIO)).toBe(true);
59+
expect(allElementsHaveAttributeValue(bars.slice(3), 'opacity', FADE_FACTOR)).toBe(true);
6060
});
6161
});
6262

@@ -78,7 +78,7 @@ describe('Series', () => {
7878
allElementsHaveAttributeValue(
7979
[...bars.slice(0, 1), ...bars.slice(3, 4), ...bars.slice(6, 7)],
8080
'opacity',
81-
1 / HIGHLIGHT_CONTRAST_RATIO
81+
FADE_FACTOR
8282
)
8383
).toBe(true);
8484
});
@@ -102,7 +102,7 @@ describe('Keys', () => {
102102
allElementsHaveAttributeValue(
103103
[...bars.slice(0, 1), ...bars.slice(3, 4), ...bars.slice(6, 7)],
104104
'opacity',
105-
1 / HIGHLIGHT_CONTRAST_RATIO
105+
FADE_FACTOR
106106
)
107107
).toBe(true);
108108
});
@@ -147,7 +147,7 @@ describe('ScatterChart', () => {
147147
// highlighted points
148148
expect(allElementsHaveAttributeValue(points.slice(0, 6), 'opacity', '1')).toBe(true);
149149
// all other points
150-
expect(allElementsHaveAttributeValue(points.slice(6), 'opacity', 1 / HIGHLIGHT_CONTRAST_RATIO)).toBe(true);
150+
expect(allElementsHaveAttributeValue(points.slice(6), 'opacity', FADE_FACTOR)).toBe(true);
151151
});
152152
});
153153

@@ -194,7 +194,7 @@ describe('AreaChart', () => {
194194
const highlightVerticalRules = queryAllMarksByGroupName(chart, 'area0_rule', 'line');
195195
expect(highlightVerticalRules).toHaveLength(0);
196196
expect(areas[0]).toHaveAttribute('opacity', '1');
197-
expect(allElementsHaveAttributeValue(areas.slice(1), 'opacity', (1 / HIGHLIGHT_CONTRAST_RATIO).toString())).toBe(
197+
expect(allElementsHaveAttributeValue(areas.slice(1), 'opacity', (FADE_FACTOR).toString())).toBe(
198198
true
199199
);
200200
});

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,10 @@
99
* OF ANY KIND, either express or implied. See the License for the specific language
1010
* governing permissions and limitations under the License.
1111
*/
12-
import React from 'react';
1312

1413
import userEvent from '@testing-library/user-event';
1514

16-
import { HIGHLIGHT_CONTRAST_RATIO, TOOLTIP_DELAY } from '@spectrum-charts/constants';
15+
import { FADE_FACTOR, TOOLTIP_DELAY } from '@spectrum-charts/constants';
1716

1817
import { Chart } from '../../../Chart';
1918
import { Legend } from '../../../components';
@@ -218,7 +217,7 @@ describe('Legend', () => {
218217
// first symbol should be highlighted
219218
let symbols = getAllLegendSymbols(chart);
220219
expect(symbols[0]).toHaveAttribute('opacity', '1');
221-
expect(symbols[1]).toHaveAttribute('opacity', `${1 / HIGHLIGHT_CONTRAST_RATIO}`);
220+
expect(symbols[1]).toHaveAttribute('opacity', `${FADE_FACTOR}`);
222221

223222
// shouldn't close the popover
224223
await userEvent.click(popover);

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,8 @@
99
* OF ANY KIND, either express or implied. See the License for the specific language
1010
* governing permissions and limitations under the License.
1111
*/
12-
import React from 'react';
1312

14-
import { HIGHLIGHT_CONTRAST_RATIO } from '@spectrum-charts/constants';
13+
import { FADE_FACTOR } from '@spectrum-charts/constants';
1514
import { spectrumColors } from '@spectrum-charts/themes';
1615

1716
import {
@@ -114,7 +113,7 @@ test('Hidden series should not highlight any marks', async () => {
114113
// hovering the second entry should lower the opacity of the first series
115114
await hoverNthElement(entries, 1);
116115
let bars = await findAllMarksByGroupName(chart, 'bar0');
117-
expect(bars[0]).toHaveAttribute('opacity', `${1 / HIGHLIGHT_CONTRAST_RATIO}`);
116+
expect(bars[0]).toHaveAttribute('opacity', `${FADE_FACTOR}`);
118117

119118
// hovering the third entry should not adjust the opcity of any of the bars since it is a hidden series
120119
await hoverNthElement(entries, 2);

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

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,8 @@
99
* OF ANY KIND, either express or implied. See the License for the specific language
1010
* governing permissions and limitations under the License.
1111
*/
12-
import React from 'react';
1312

14-
import { HIGHLIGHT_CONTRAST_RATIO } from '@spectrum-charts/constants';
13+
import { FADE_FACTOR } from '@spectrum-charts/constants';
1514

1615
import {
1716
allElementsHaveAttributeValue,
@@ -31,9 +30,9 @@ describe('Controlled', () => {
3130

3231
const bars = await findAllMarksByGroupName(chart, 'bar0');
3332
expect(bars.length).toEqual(9);
34-
expect(bars[0]).toHaveAttribute('opacity', `${1 / HIGHLIGHT_CONTRAST_RATIO}`);
33+
expect(bars[0]).toHaveAttribute('opacity', `${FADE_FACTOR}`);
3534
expect(bars[1]).toHaveAttribute('opacity', '1');
36-
expect(bars[2]).toHaveAttribute('opacity', `${1 / HIGHLIGHT_CONTRAST_RATIO}`);
35+
expect(bars[2]).toHaveAttribute('opacity', `${FADE_FACTOR}`);
3736
});
3837

3938
test('non highlighted series legend symbols should have opacity applied', async () => {
@@ -42,9 +41,9 @@ describe('Controlled', () => {
4241

4342
const legendSymbols = await findAllMarksByGroupName(chart, 'role-legend-symbol');
4443
expect(legendSymbols.length).toEqual(3);
45-
expect(legendSymbols[0]).toHaveAttribute('opacity', `${1 / HIGHLIGHT_CONTRAST_RATIO}`);
44+
expect(legendSymbols[0]).toHaveAttribute('opacity', `${FADE_FACTOR}`);
4645
expect(legendSymbols[1]).toHaveAttribute('opacity', '1');
47-
expect(legendSymbols[2]).toHaveAttribute('opacity', `${1 / HIGHLIGHT_CONTRAST_RATIO}`);
46+
expect(legendSymbols[2]).toHaveAttribute('opacity', `${FADE_FACTOR}`);
4847
});
4948

5049
test('non highlighted series legend labels should have opacity applied', async () => {
@@ -53,9 +52,9 @@ describe('Controlled', () => {
5352

5453
const legendLabels = await findAllMarksByGroupName(chart, 'role-legend-symbol');
5554
expect(legendLabels.length).toEqual(3);
56-
expect(legendLabels[0]).toHaveAttribute('opacity', `${1 / HIGHLIGHT_CONTRAST_RATIO}`);
55+
expect(legendLabels[0]).toHaveAttribute('opacity', `${FADE_FACTOR}`);
5756
expect(legendLabels[1]).toHaveAttribute('opacity', '1');
58-
expect(legendLabels[2]).toHaveAttribute('opacity', `${1 / HIGHLIGHT_CONTRAST_RATIO}`);
57+
expect(legendLabels[2]).toHaveAttribute('opacity', `${FADE_FACTOR}`);
5958
});
6059
});
6160

@@ -79,8 +78,8 @@ describe('Uncontrolled', () => {
7978

8079
bars = await findAllMarksByGroupName(chart, 'bar0');
8180
expect(bars[0]).toHaveAttribute('opacity', '1');
82-
expect(bars[1]).toHaveAttribute('opacity', `${1 / HIGHLIGHT_CONTRAST_RATIO}`);
83-
expect(bars[2]).toHaveAttribute('opacity', `${1 / HIGHLIGHT_CONTRAST_RATIO}`);
81+
expect(bars[1]).toHaveAttribute('opacity', `${FADE_FACTOR}`);
82+
expect(bars[2]).toHaveAttribute('opacity', `${FADE_FACTOR}`);
8483
});
8584

8685
test('hovering over legend items adds opacity to the non hovered legend symbols', async () => {
@@ -96,7 +95,7 @@ describe('Uncontrolled', () => {
9695

9796
legendSymbols = await findAllMarksByGroupName(chart, 'role-legend-symbol');
9897
expect(legendSymbols[0]).toHaveAttribute('opacity', '1');
99-
expect(allElementsHaveAttributeValue(legendSymbols.slice(1), 'opacity', 1 / HIGHLIGHT_CONTRAST_RATIO)).toBeTruthy();
98+
expect(allElementsHaveAttributeValue(legendSymbols.slice(1), 'opacity', FADE_FACTOR)).toBeTruthy();
10099
});
101100

102101
test('hovering over legend items adds opacity to the non hovered legend labels', async () => {
@@ -113,6 +112,6 @@ describe('Uncontrolled', () => {
113112
legendLabels = await findAllMarksByGroupName(chart, 'role-legend-symbol');
114113
expect(legendLabels.length).toEqual(3);
115114
expect(legendLabels[0]).toHaveAttribute('opacity', '1');
116-
expect(allElementsHaveAttributeValue(legendLabels.slice(1), 'opacity', 1 / HIGHLIGHT_CONTRAST_RATIO)).toBeTruthy();
115+
expect(allElementsHaveAttributeValue(legendLabels.slice(1), 'opacity', FADE_FACTOR)).toBeTruthy();
117116
});
118117
});

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
* OF ANY KIND, either express or implied. See the License for the specific language
1010
* governing permissions and limitations under the License.
1111
*/
12-
import { HIGHLIGHT_CONTRAST_RATIO } from '@spectrum-charts/constants';
12+
import { FADE_FACTOR } from '@spectrum-charts/constants';
1313

1414
import { Line } from '../../../components';
1515
import { workspaceTrendsData } from '../../../stories/data/data';
@@ -127,7 +127,6 @@ describe('Line', () => {
127127
});
128128

129129
test('Hovering over the entries on HistoricalCompare should highlight hovered series', async () => {
130-
const reducedOpacity = 1 / HIGHLIGHT_CONTRAST_RATIO;
131130
render(<HistoricalCompare {...HistoricalCompare.args} />);
132131
const chart = await findChart();
133132
expect(chart).toBeInTheDocument();
@@ -139,24 +138,24 @@ describe('Line', () => {
139138
// symbol opacity should be reduced for all but the first symbol
140139
let symbols = getAllLegendSymbols(chart);
141140
expect(symbols[0]).toHaveAttribute('opacity', '1');
142-
expect(allElementsHaveAttributeValue(symbols.slice(1), 'opacity', reducedOpacity)).toBeTruthy();
141+
expect(allElementsHaveAttributeValue(symbols.slice(1), 'opacity', FADE_FACTOR)).toBeTruthy();
143142

144143
// line opacity should be reduced for all but the first line
145144
let lines = await findAllMarksByGroupName(chart, 'line0');
146145
expect(lines[0]).toHaveAttribute('opacity', '1');
147-
expect(allElementsHaveAttributeValue(lines.slice(1), 'opacity', reducedOpacity)).toBeTruthy();
146+
expect(allElementsHaveAttributeValue(lines.slice(1), 'opacity', FADE_FACTOR)).toBeTruthy();
148147

149148
await unhoverNthElement(entries, 0);
150149
await hoverNthElement(entries, 3);
151150

152151
// symbol opacity should be reduced for all but the last symbol
153152
symbols = getAllLegendSymbols(chart);
154-
expect(allElementsHaveAttributeValue(symbols.slice(0, 3), 'opacity', reducedOpacity)).toBeTruthy();
153+
expect(allElementsHaveAttributeValue(symbols.slice(0, 3), 'opacity', FADE_FACTOR)).toBeTruthy();
155154
expect(symbols[3]).toHaveAttribute('opacity', '1');
156155

157156
// line opacity should be reduced for all but the last line
158157
lines = await findAllMarksByGroupName(chart, 'line0');
159-
expect(allElementsHaveAttributeValue(lines.slice(0, 3), 'opacity', reducedOpacity)).toBeTruthy();
158+
expect(allElementsHaveAttributeValue(lines.slice(0, 3), 'opacity', FADE_FACTOR)).toBeTruthy();
160159
expect(lines[3]).toHaveAttribute('opacity', '1');
161160
});
162161

0 commit comments

Comments
 (0)