From 0762cf3174a5f290860c5dca62bc6df92c8acad0 Mon Sep 17 00:00:00 2001
From: Zach Barbre <52134863+ZachBarbre@users.noreply.github.com>
Date: Mon, 20 May 2024 14:41:21 -0400
Subject: [PATCH] Fix LegendThreshold (#1831)

* Fix rect color edge case in LegendThreshold

* Add tests, fix LegendThreshold default scale for negitive domain

* Add test for 0 case and fix LegendThreshold
---
 .../visx-legend/src/legends/Threshold.tsx     | 11 +--
 .../visx-legend/test/LegendThreshold.test.tsx | 73 +++++++++++++++++++
 2 files changed, 79 insertions(+), 5 deletions(-)

diff --git a/packages/visx-legend/src/legends/Threshold.tsx b/packages/visx-legend/src/legends/Threshold.tsx
index b1197f1a3..325424050 100644
--- a/packages/visx-legend/src/legends/Threshold.tsx
+++ b/packages/visx-legend/src/legends/Threshold.tsx
@@ -28,7 +28,6 @@ function defaultTransform<Scale extends AnyThresholdScale>({
 }: TransformProps): LabelFormatterFactory<Scale> {
   return ({ scale, labelFormat }) => {
     const scaleRange = scale.range();
-    const scaleDomain = scale.domain();
 
     type Datum = ScaleInput<Scale>;
 
@@ -43,7 +42,7 @@ function defaultTransform<Scale extends AnyThresholdScale>({
       if (d0 == null && typeof d1 === 'number') {
         // lower threshold e.g., [undefined, number]
         delimiter = labelLower || delimiter;
-        value = d1 - 1;
+        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
@@ -52,13 +51,13 @@ function defaultTransform<Scale extends AnyThresholdScale>({
       } 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 + Math.abs(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,
@@ -81,7 +80,9 @@ export default function Threshold<Scale extends AnyThresholdScale>({
   // 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..c704ac972 100644
--- a/packages/visx-legend/test/LegendThreshold.test.tsx
+++ b/packages/visx-legend/test/LegendThreshold.test.tsx
@@ -1,7 +1,80 @@
+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('<LegendThreshold />', () => {
   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(
+      <svg>
+        <LegendThreshold scale={thresholdScale} data-testid="thresholdLegend" />
+      </svg>,
+    );
+
+    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 with a negitive domain', () => {
+    const domain = [-3, -1];
+    const range = ['green', 'purple', 'blue'];
+    const thresholdScale1 = scaleThreshold({
+      domain,
+      range,
+    });
+
+    const { getAllByTestId } = render(
+      <svg>
+        <LegendThreshold scale={thresholdScale1} data-testid="thresholdLegend" />
+      </svg>,
+    );
+
+    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 with 0', () => {
+    const domain = [0, 1, 4];
+    const range = ['green', 'purple', 'blue', 'pink'];
+    const thresholdScale1 = scaleThreshold({
+      domain,
+      range,
+    });
+
+    const { getAllByTestId } = render(
+      <svg>
+        <LegendThreshold scale={thresholdScale1} data-testid="thresholdLegend" />
+      </svg>,
+    );
+
+    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}`);
+    });
+  });
 });