From 40a83c8c9d2f6a4335be8f53b66b44452147d29a Mon Sep 17 00:00:00 2001 From: Zach Barbre Date: Wed, 8 May 2024 18:06:26 -0400 Subject: [PATCH 1/3] Fix rect color edge case in LegendThreshold --- packages/visx-legend/src/legends/Threshold.tsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/visx-legend/src/legends/Threshold.tsx b/packages/visx-legend/src/legends/Threshold.tsx index b1197f1a3..afc0c75b5 100644 --- a/packages/visx-legend/src/legends/Threshold.tsx +++ b/packages/visx-legend/src/legends/Threshold.tsx @@ -28,7 +28,6 @@ function defaultTransform({ }: TransformProps): LabelFormatterFactory { return ({ scale, labelFormat }) => { const scaleRange = scale.range(); - const scaleDomain = scale.domain(); type Datum = ScaleInput; @@ -43,7 +42,7 @@ function defaultTransform({ if (d0 == null && typeof d1 === 'number') { // lower threshold e.g., [undefined, number] delimiter = labelLower || delimiter; - value = d1 - 1; + value = d1 - (2 * d1 - 1); // guarantees a value smaller than the lower threshold text = `${delimiter}${formatZero(labelFormat(d1, i))}`; } else if (d0 != null && d1 != null) { // threshold step @@ -52,13 +51,13 @@ function defaultTransform({ } else if (typeof d0 === 'number' && d1 == null) { // upper threshold e.g., [number, undefined] delimiter = labelUpper || delimiter; - value = d0 + scaleDomain[1]; // x0,x1 are from the domain, so the domain is numeric if d0 is + value = d0 + (2 * d0 + 1); // // guarantees a value larger than the upper threshold text = `${delimiter}${formatZero(labelFormat(d0, i))}`; } return { extent: [d0, d1], - value: scale(value || d), + value: scale(value), text, datum: d, index: i, From 73d031e75a475b094dd2d87b3115a511a6d49d69 Mon Sep 17 00:00:00 2001 From: Zach Barbre Date: Mon, 13 May 2024 23:05:37 -0400 Subject: [PATCH 2/3] Add tests, fix LegendThreshold default scale for negitive domain --- .../visx-legend/src/legends/Threshold.tsx | 8 +-- .../visx-legend/test/LegendThreshold.test.tsx | 50 +++++++++++++++++++ 2 files changed, 55 insertions(+), 3 deletions(-) diff --git a/packages/visx-legend/src/legends/Threshold.tsx b/packages/visx-legend/src/legends/Threshold.tsx index afc0c75b5..2b1b73990 100644 --- a/packages/visx-legend/src/legends/Threshold.tsx +++ b/packages/visx-legend/src/legends/Threshold.tsx @@ -42,7 +42,7 @@ function defaultTransform({ if (d0 == null && typeof d1 === 'number') { // lower threshold e.g., [undefined, number] delimiter = labelLower || delimiter; - value = d1 - (2 * d1 - 1); // guarantees a value smaller than the lower threshold + value = d1 - 2 * Math.abs(d1); // guarantees a value smaller than the lower threshold text = `${delimiter}${formatZero(labelFormat(d1, i))}`; } else if (d0 != null && d1 != null) { // threshold step @@ -51,7 +51,7 @@ function defaultTransform({ } else if (typeof d0 === 'number' && d1 == null) { // upper threshold e.g., [number, undefined] delimiter = labelUpper || delimiter; - value = d0 + (2 * d0 + 1); // // guarantees a value larger than the upper threshold + value = d0 + 2 * Math.abs(d0); // // guarantees a value larger than the upper threshold text = `${delimiter}${formatZero(labelFormat(d0, i))}`; } @@ -80,7 +80,9 @@ export default function Threshold({ // https://github.com/d3/d3-scale#threshold_domain // therefore if a domain is not specified we transform the range into input values // because it should contain more elements - const domain = inputDomain || scale.range().map((output) => scale.invertExtent(output)[0]); + + const domain = + inputDomain || scale.range().map((output) => scale.invertExtent(output)[0] as Range); const labelTransform = inputLabelTransform || diff --git a/packages/visx-legend/test/LegendThreshold.test.tsx b/packages/visx-legend/test/LegendThreshold.test.tsx index 4e419f850..6c952fc66 100644 --- a/packages/visx-legend/test/LegendThreshold.test.tsx +++ b/packages/visx-legend/test/LegendThreshold.test.tsx @@ -1,7 +1,57 @@ +import React from 'react'; +import { render } from '@testing-library/react'; +import '@testing-library/jest-dom'; +import { scaleThreshold } from '@visx/scale'; import { LegendThreshold } from '../src'; describe('', () => { test('it should be defined', () => { expect(LegendThreshold).toBeDefined(); }); + + it('should render LegendShape with the correct color', () => { + const domain = [1, 2, 9]; + const range = ['green', 'purple', 'blue', 'pink']; + const thresholdScale = scaleThreshold({ + domain, + range, + }); + + const { getAllByTestId } = render( + + + , + ); + + const thresholdLegend = getAllByTestId('thresholdLegend'); + + range.forEach((color, index) => { + const legendItem = thresholdLegend[index]; + const legendShape = legendItem?.querySelector('.visx-legend-shape'); + expect(legendShape?.querySelector('div')).toHaveStyle(`background: ${color}`); + }); + }); + + it('should render LegendShape with the correct color in a negitive domain', () => { + const domain = [-3, -1]; + const range = ['green', 'purple', 'blue']; + const thresholdScale1 = scaleThreshold({ + domain, + range, + }); + + const { getAllByTestId } = render( + + + , + ); + + const thresholdLegend = getAllByTestId('thresholdLegend'); + + range.forEach((color, index) => { + const legendItem = thresholdLegend[index]; + const legendShape = legendItem?.querySelector('.visx-legend-shape'); + expect(legendShape?.querySelector('div')).toHaveStyle(`background: ${color}`); + }); + }); }); From 7575c7a32ab34f3110d580705248d4e969b3fc0e Mon Sep 17 00:00:00 2001 From: Zach Barbre Date: Tue, 14 May 2024 10:09:27 -0400 Subject: [PATCH 3/3] Add test for 0 case and fix LegendThreshold --- .../visx-legend/src/legends/Threshold.tsx | 4 +-- .../visx-legend/test/LegendThreshold.test.tsx | 25 ++++++++++++++++++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/packages/visx-legend/src/legends/Threshold.tsx b/packages/visx-legend/src/legends/Threshold.tsx index 2b1b73990..325424050 100644 --- a/packages/visx-legend/src/legends/Threshold.tsx +++ b/packages/visx-legend/src/legends/Threshold.tsx @@ -42,7 +42,7 @@ function defaultTransform({ if (d0 == null && typeof d1 === 'number') { // lower threshold e.g., [undefined, number] delimiter = labelLower || delimiter; - value = d1 - 2 * Math.abs(d1); // guarantees a value smaller than the lower threshold + value = d1 - Math.abs(2 * d1 - 1); // guarantees a value smaller than the lower threshold text = `${delimiter}${formatZero(labelFormat(d1, i))}`; } else if (d0 != null && d1 != null) { // threshold step @@ -51,7 +51,7 @@ function defaultTransform({ } else if (typeof d0 === 'number' && d1 == null) { // upper threshold e.g., [number, undefined] delimiter = labelUpper || delimiter; - value = d0 + 2 * Math.abs(d0); // // guarantees a value larger than the upper threshold + value = d0 + Math.abs(2 * d0 + 1); // // guarantees a value larger than the upper threshold text = `${delimiter}${formatZero(labelFormat(d0, i))}`; } diff --git a/packages/visx-legend/test/LegendThreshold.test.tsx b/packages/visx-legend/test/LegendThreshold.test.tsx index 6c952fc66..c704ac972 100644 --- a/packages/visx-legend/test/LegendThreshold.test.tsx +++ b/packages/visx-legend/test/LegendThreshold.test.tsx @@ -32,7 +32,7 @@ describe('', () => { }); }); - it('should render LegendShape with the correct color in a negitive domain', () => { + it('should render LegendShape with the correct color with a negitive domain', () => { const domain = [-3, -1]; const range = ['green', 'purple', 'blue']; const thresholdScale1 = scaleThreshold({ @@ -54,4 +54,27 @@ describe('', () => { expect(legendShape?.querySelector('div')).toHaveStyle(`background: ${color}`); }); }); + + it('should render LegendShape with the correct color with 0', () => { + const domain = [0, 1, 4]; + const range = ['green', 'purple', 'blue', 'pink']; + const thresholdScale1 = scaleThreshold({ + domain, + range, + }); + + const { getAllByTestId } = render( + + + , + ); + + const thresholdLegend = getAllByTestId('thresholdLegend'); + + range.forEach((color, index) => { + const legendItem = thresholdLegend[index]; + const legendShape = legendItem?.querySelector('.visx-legend-shape'); + expect(legendShape?.querySelector('div')).toHaveStyle(`background: ${color}`); + }); + }); });