fix multi-color selection in `ColorRangeSelector` (#33989)

* fix multi-color selection in `ColorRangeSelector`

* unit test

frontend/src/metabase/core/components/ColorRangeSelector/ColorRangePopover.tsx

@@ -47,7 +47,9 @@ const ColorSelectorContent = forwardRef(function ColorRangeSelector(
   );
 
   const [value, setValue] = useState(() =>
-    getColorRange(color, colorMapping, isInverted),
+    color === "" // empty string is for multi-color selection
+      ? initialValue
+      : getColorRange(color, colorMapping, isInverted),
   );
 
   const handleColorSelect = useCallback(
@@ -61,13 +63,29 @@ const ColorSelectorContent = forwardRef(function ColorRangeSelector(
     [colorMapping, isInverted, onChange],
   );
 
+  const handleColorRangeSelect = useCallback(
+    (newColorRange: string[]) => {
+      const newValue = isInverted
+        ? [...newColorRange].reverse()
+        : newColorRange;
+
+      setColor("");
+      setValue(newValue);
+      onChange?.(newValue);
+    },
+    [isInverted, onChange],
+  );
+
   const handleToggleInvertedClick = useCallback(() => {
-    const newValue = getColorRange(color, colorMapping, !isInverted);
+    const newValue =
+      color === ""
+        ? [...value].reverse()
+        : getColorRange(color, colorMapping, !isInverted);
 
     setIsInverted(!isInverted);
     setValue(newValue);
     onChange?.(newValue);
-  }, [color, colorMapping, isInverted, onChange]);
+  }, [color, value, colorMapping, isInverted, onChange]);
 
   return (
     <PopoverRoot {...props} ref={ref}>
@@ -84,7 +102,8 @@ const ColorSelectorContent = forwardRef(function ColorRangeSelector(
       <ColorRangeToggle
         value={value}
         isQuantile={isQuantile}
-        onClick={handleToggleInvertedClick}
+        onToggleClick={handleToggleInvertedClick}
+        showToggleButton
       />
       {colorRanges.length > 0 && <PopoverDivider />}
       <PopoverColorRangeList>
@@ -93,7 +112,8 @@ const ColorSelectorContent = forwardRef(function ColorRangeSelector(
             key={index}
             value={range}
             isQuantile={isQuantile}
-            onClick={handleToggleInvertedClick}
+            onToggleClick={handleToggleInvertedClick}
+            onColorRangeSelect={handleColorRangeSelect}
           />
         ))}
       </PopoverColorRangeList>
@@ -126,7 +146,7 @@ const getDefaultColor = (
     } else {
       return selection;
     }
-  }, colors[0]);
+  }, "" as string);
 };
 
 const getDefaultColorMapping = (colors: string[]) => {

frontend/src/metabase/core/components/ColorRangeSelector/ColorRangeSelector.unit.spec.tsx

 import { render, screen } from "@testing-library/react";
 
 import { color } from "metabase/lib/colors";
+
 import ColorRangeSelector from "./ColorRangeSelector";
+import { getColorRangeLabel } from "./ColorRangeToggle";
 
 const DEFAULT_VALUE = [color("white"), color("brand")];
 const DEFAULT_COLORS = [color("brand"), color("summarize"), color("filter")];
 
+const WHITE_COLOR_RANGE = [color("error"), color("white"), color("success")];
+const WARNING_COLOR_RANGE = [
+  color("error"),
+  color("warning"),
+  color("success"),
+];
+
 function setup() {
   const onChange = jest.fn();
   render(
     <ColorRangeSelector
       value={DEFAULT_VALUE}
       colors={DEFAULT_COLORS}
+      colorRanges={[WHITE_COLOR_RANGE, WARNING_COLOR_RANGE]}
       onChange={onChange}
     />,
   );
@@ -32,4 +42,20 @@ describe("ColorRangeSelector", () => {
     screen.getByLabelText(color("filter")).click();
     expect(onChange).toHaveBeenCalled();
   });
+
+  it("should call `onChange` upon clicking a non-initial range", async () => {
+    const { onChange } = setup();
+
+    screen.getByRole("button").click();
+    expect(await screen.findByRole("tooltip")).toBeInTheDocument();
+
+    screen.getByLabelText(getColorRangeLabel(DEFAULT_VALUE)).click();
+    expect(onChange).not.toHaveBeenCalled();
+
+    screen.getByLabelText(getColorRangeLabel(WHITE_COLOR_RANGE)).click();
+    expect(onChange).toHaveBeenCalled();
+
+    screen.getByLabelText(getColorRangeLabel(WARNING_COLOR_RANGE)).click();
+    expect(onChange).toHaveBeenCalled();
+  });
 });

frontend/src/metabase/core/components/ColorRangeSelector/ColorRangeToggle.styled.tsx

@@ -2,13 +2,15 @@ import styled from "@emotion/styled";
 import ColorRange from "metabase/core/components/ColorRange";
 import Button from "metabase/core/components/Button";
 
+import type { ColorRangeProps } from "../ColorRange/ColorRange";
+
 export const ToggleRoot = styled.div`
   display: flex;
 `;
 
-export const ToggleColorRange = styled(ColorRange)`
+export const ToggleColorRange = styled(ColorRange)<ColorRangeProps>`
   flex: 1 1 auto;
-  cursor: default;
+  cursor: ${props => (props.onSelect ? "pointer" : " default")};
 `;
 
 export const ToggleButton = styled(Button)`

frontend/src/metabase/core/components/ColorRangeSelector/ColorRangeToggle.tsx

@@ -7,21 +7,36 @@ import {
 export interface ColorRangeToggleProps {
   value: string[];
   isQuantile?: boolean;
-  onClick?: () => void;
+  onToggleClick?: () => void;
+  onColorRangeSelect?: (newColorRange: string[]) => void;
+  showToggleButton?: boolean;
 }
 
 const ColorRangeToggle = ({
   value,
   isQuantile,
-  onClick,
+  onToggleClick,
+  onColorRangeSelect,
+  showToggleButton = false,
 }: ColorRangeToggleProps) => {
   return (
     <ToggleRoot>
-      <ToggleColorRange colors={value} isQuantile={isQuantile} />
-      <ToggleButton icon="compare" small onClick={onClick} />
+      <ToggleColorRange
+        colors={value}
+        isQuantile={isQuantile}
+        onSelect={onColorRangeSelect}
+        aria-label={getColorRangeLabel(value)}
+      />
+      {showToggleButton && (
+        <ToggleButton icon="compare" small onClick={onToggleClick} />
+      )}
     </ToggleRoot>
   );
 };
 
 // eslint-disable-next-line import/no-default-export -- deprecated usage
 export default ColorRangeToggle;
+
+export function getColorRangeLabel(value: string[]) {
+  return value.join("-");
+}