Fix RGB channels drifting when only alpha is changed

src/utils/convert.ts

@@ -181,10 +181,12 @@ export const rgbaToHsva = ({ r, g, b, a }: RgbaColor): HsvaColor => {
         : 4 + (r - g) / delta
     : 0;
 
+  // Do not round the result: rounding the internal HSVA representation loses precision,
+  // making `fromHsva(toHsva(color))` drift (e.g. RGB channels change when only alpha does)
   return {
-    h: round(60 * (hh < 0 ? hh + 6 : hh)),
-    s: round(max ? (delta / max) * 100 : 0),
-    v: round((max / 255) * 100),
+    h: 60 * (hh < 0 ? hh + 6 : hh),
+    s: max ? (delta / max) * 100 : 0,
+    v: (max / 255) * 100,
     a,
   };
 };

tests/components.test.js

@@ -311,6 +311,49 @@ it("Changes alpha channel value after an interaction", async () => {
   expect(handleChange).toHaveReturnedWith({ h: 100, s: 0, l: 0, a: 1 });
 });
 
+it("Doesn't change RGB channels when only alpha is changed", async () => {
+  const handleChange = jest.fn((rgba) => rgba);
+  const initialValue = { r: 200, g: 120, b: 35, a: 0.5 };
+
+  const result = render(<RgbaColorPicker color={initialValue} onChange={handleChange} />);
+  const alpha = result.container.querySelector(
+    ".react-colorful__alpha .react-colorful__interactive"
+  );
+
+  fireEvent(alpha, new FakeMouseEvent("mousedown", { pageX: 0, pageY: 0 }));
+  fireEvent(alpha, new FakeMouseEvent("mousemove", { pageX: 105, pageY: 0 }));
+
+  expect(handleChange).toHaveLastReturnedWith({ r: 200, g: 120, b: 35, a: 1 });
+});
+
+it("Doesn't change RGB string channels when only alpha is changed", async () => {
+  const handleChange = jest.fn((rgbaString) => rgbaString);
+
+  const result = render(
+    <RgbaStringColorPicker color="rgba(200, 120, 35, 1)" onChange={handleChange} />
+  );
+  const alpha = result.container.querySelector(
+    ".react-colorful__alpha .react-colorful__interactive"
+  );
+
+  fireEvent(alpha, new FakeMouseEvent("mousedown", { pageX: 55, pageY: 0 }));
+
+  expect(handleChange).toHaveLastReturnedWith("rgba(200, 120, 35, 0.5)");
+});
+
+it("Doesn't change HEX color channels when only alpha is changed", async () => {
+  const handleChange = jest.fn((hex) => hex);
+
+  const result = render(<HexAlphaColorPicker color="#c87823" onChange={handleChange} />);
+  const alpha = result.container.querySelector(
+    ".react-colorful__alpha .react-colorful__interactive"
+  );
+
+  fireEvent(alpha, new FakeMouseEvent("mousedown", { pageX: 55, pageY: 0 }));
+
+  expect(handleChange).toHaveLastReturnedWith("#c8782380");
+});
+
 it("Uses #rrggbbaa format if alpha channel value is less than 1", async () => {
   const handleChange = jest.fn((hex) => hex);
   const result = render(<HexAlphaColorPicker color="#112233" onChange={handleChange} />);

tests/utils.test.js

@@ -35,14 +35,14 @@ it("Converts HEX to HSVA", () => {
   expect(hexToHsva("#ffff00")).toMatchObject({ h: 60, s: 100, v: 100, a: 1 });
   expect(hexToHsva("#ff0000")).toMatchObject({ h: 0, s: 100, v: 100, a: 1 });
   expect(hexToHsva("#000000")).toMatchObject({ h: 0, s: 0, v: 0, a: 1 });
-  expect(hexToHsva("#c62182")).toMatchObject({ h: 325, s: 83, v: 78, a: 1 });
+  expect(roundHsva(hexToHsva("#c62182"))).toMatchObject({ h: 325, s: 83, v: 78, a: 1 });
 });
 
 it("Converts shorthand HEX to HSVA", () => {
   expect(hexToHsva("#FFF")).toMatchObject({ h: 0, s: 0, v: 100, a: 1 });
   expect(hexToHsva("#FF0")).toMatchObject({ h: 60, s: 100, v: 100, a: 1 });
   expect(hexToHsva("#F00")).toMatchObject({ h: 0, s: 100, v: 100, a: 1 });
-  expect(hexToHsva("#ABC")).toMatchObject({ h: 210, s: 17, v: 80, a: 1 });
+  expect(roundHsva(hexToHsva("#ABC"))).toMatchObject({ h: 210, s: 17, v: 80, a: 1 });
 });
 
 it("Converts HSVA to HEX", () => {
@@ -100,6 +100,16 @@ it("Converts HSVA to RGBA", () => {
   test({ h: 0, s: 100, v: 100, a: 0.567 }, { r: 255, g: 0, b: 0, a: 0.57 });
 });
 
+it("Converts RGBA to HSVA and back without losing precision", () => {
+  // `rgbaToHsva` must not round the result, otherwise the RGB values drift
+  // after a roundtrip (e.g. when the user changes only the alpha channel)
+  const test = (rgba) => expect(hsvaToRgba(rgbaToHsva(rgba))).toMatchObject(rgba);
+  test({ r: 200, g: 120, b: 35, a: 0.5 }); // https://github.com/omgovich/react-colorful/issues/163
+  test({ r: 239, g: 239, b: 239, a: 1 }); // grayscale (delta = 0)
+  test({ r: 3, g: 22, b: 252, a: 0.75 });
+  test({ r: 255, g: 254, b: 253, a: 1 });
+});
+
 it("Converts RGBA to HSVA", () => {
   expect(rgbaToHsva({ r: 255, g: 255, b: 255, a: 1 })).toMatchObject({ h: 0, s: 0, v: 100, a: 1 });
   expect(rgbaToHsva({ r: 0, g: 255, b: 0, a: 1 })).toMatchObject({ h: 120, s: 100, v: 100, a: 1 });
@@ -110,7 +120,12 @@ it("Converts RGB string to HSVA", () => {
   expect(rgbStringToHsva("rgb(255, 255, 255)")).toMatchObject({ h: 0, s: 0, v: 100, a: 1 });
   expect(rgbStringToHsva("rgb(0,0,0)")).toMatchObject({ h: 0, s: 0, v: 0, a: 1 });
   expect(rgbStringToHsva("rgb(100% 100% 100%)")).toMatchObject({ h: 0, s: 0, v: 100, a: 1 });
-  expect(rgbStringToHsva("rgb(50% 45.9% 25%)")).toMatchObject({ h: 50, s: 50, v: 50, a: 1 });
+  expect(roundHsva(rgbStringToHsva("rgb(50% 45.9% 25%)"))).toMatchObject({
+    h: 50,
+    s: 50,
+    v: 50,
+    a: 1,
+  });
 });
 
 it("Converts HSVA to RGB string", () => {
@@ -124,7 +139,7 @@ it("Converts HSVA to RGBA string", () => {
 });
 
 it("Converts RGBA string to HSVA", () => {
-  let test = (input, output) => expect(rgbaStringToHsva(input)).toMatchObject(output);
+  let test = (input, output) => expect(roundHsva(rgbaStringToHsva(input))).toMatchObject(output);
   test("rgba(61, 88, 102, 0.5)", { h: 200, s: 40, v: 40, a: 0.5 });
   test("rgba(23.9% 34.5% 40% / 99%)", { h: 200, s: 40, v: 40, a: 0.99 });
 });