From 8c01b034089c4da7d406692970966e6cdd36bff0 Mon Sep 17 00:00:00 2001 From: Mosaad <48773133+theMosaad@users.noreply.github.com> Date: Mon, 27 May 2024 16:19:37 +0300 Subject: [PATCH 1/4] Fix hue normalization --- packages/@react-stately/color/src/Color.ts | 21 ++++++++++++------- .../color/src/useColorWheelState.ts | 12 ++++------- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/packages/@react-stately/color/src/Color.ts b/packages/@react-stately/color/src/Color.ts index 25e909a2af5..beea5469890 100644 --- a/packages/@react-stately/color/src/Color.ts +++ b/packages/@react-stately/color/src/Color.ts @@ -49,6 +49,17 @@ export function getColorChannels(colorSpace: ColorSpace) { } } +/** + * Returns the hue value normalized to the range of 0 to 360. + */ +export function normalizeHue(hue: number) { + if (hue === 360) { + return hue; + } + + return ((hue % 360) + 360) % 360; +} + // Lightness threshold between orange and brown. const ORANGE_LIGHTNESS_THRESHOLD = 0.68; // Lightness threshold between pure yellow and "yellow green". @@ -145,7 +156,7 @@ abstract class Color implements IColor { } else if (c >= 0.15) { chroma = 'vibrant'; } - + if (l < 0.3) { lightness = 'very dark'; } else if (l < MAX_DARK_LIGHTNESS) { @@ -435,7 +446,7 @@ class HSBColor extends Color { let m: RegExpMatchArray | void; if ((m = value.match(HSB_REGEX))) { const [h, s, b, a] = (m[1] ?? m[2]).split(',').map(n => Number(n.trim().replace('%', ''))); - return new HSBColor(mod(h, 360), clamp(s, 0, 100), clamp(b, 0, 100), clamp(a ?? 1, 0, 1)); + return new HSBColor(normalizeHue(h), clamp(s, 0, 100), clamp(b, 0, 100), clamp(a ?? 1, 0, 1)); } } @@ -565,10 +576,6 @@ class HSBColor extends Color { // - hsla(X, X%, X%, X) const HSL_REGEX = /hsl\(([-+]?\d+(?:.\d+)?\s*,\s*[-+]?\d+(?:.\d+)?%\s*,\s*[-+]?\d+(?:.\d+)?%)\)|hsla\(([-+]?\d+(?:.\d+)?\s*,\s*[-+]?\d+(?:.\d+)?%\s*,\s*[-+]?\d+(?:.\d+)?%\s*,\s*[-+]?\d(.\d+)?)\)/; -function mod(n, m) { - return ((n % m) + m) % m; -} - class HSLColor extends Color { constructor(private hue: number, private saturation: number, private lightness: number, private alpha: number) { super(); @@ -578,7 +585,7 @@ class HSLColor extends Color { let m: RegExpMatchArray | void; if ((m = value.match(HSL_REGEX))) { const [h, s, l, a] = (m[1] ?? m[2]).split(',').map(n => Number(n.trim().replace('%', ''))); - return new HSLColor(mod(h, 360), clamp(s, 0, 100), clamp(l, 0, 100), clamp(a ?? 1, 0, 1)); + return new HSLColor(normalizeHue(h), clamp(s, 0, 100), clamp(l, 0, 100), clamp(a ?? 1, 0, 1)); } } diff --git a/packages/@react-stately/color/src/useColorWheelState.ts b/packages/@react-stately/color/src/useColorWheelState.ts index 5ddc1670f29..0220851ca70 100644 --- a/packages/@react-stately/color/src/useColorWheelState.ts +++ b/packages/@react-stately/color/src/useColorWheelState.ts @@ -11,7 +11,7 @@ */ import {Color, ColorWheelProps} from '@react-types/color'; -import {normalizeColor, parseColor} from './Color'; +import {normalizeColor, normalizeHue, parseColor} from './Color'; import {useControlledState} from '@react-stately/utils'; import {useMemo, useRef, useState} from 'react'; @@ -57,10 +57,6 @@ function roundToStep(value: number, step: number): number { return Math.round(value / step) * step; } -function mod(n: number, m: number) { - return ((n % m) + m) % m; -} - function roundDown(v: number) { let r = Math.floor(v); if (r === v) { @@ -124,7 +120,7 @@ export function useColorWheelState(props: ColorWheelProps): ColorWheelState { // Make sure you can always get back to 0. v = 0; } - v = roundToStep(mod(v, 360), step); + v = roundToStep(normalizeHue(v), step); if (hue !== v) { let color = value.withChannelValue('hue', v); setValue(color); @@ -154,7 +150,7 @@ export function useColorWheelState(props: ColorWheelProps): ColorWheelState { // Make sure you can always get back to 0. newValue = minValueX; } - setHue(roundToStep(mod(newValue, 360), s)); + setHue(roundToStep(normalizeHue(newValue), s)); }, decrement(stepSize = 1) { let s = Math.max(stepSize, step); @@ -163,7 +159,7 @@ export function useColorWheelState(props: ColorWheelProps): ColorWheelState { // |(previous step) - 0| < step size setHue(roundDown(360 / s) * s); } else { - setHue(roundToStep(mod(hue - s, 360), s)); + setHue(roundToStep(normalizeHue(hue - s), s)); } }, setDragging(isDragging) { From dad56c95a05387b1a8ae536ff3dd268014a2ab81 Mon Sep 17 00:00:00 2001 From: Mosaad <48773133+theMosaad@users.noreply.github.com> Date: Mon, 27 May 2024 16:32:46 +0300 Subject: [PATCH 2/4] add tests --- packages/@react-stately/color/test/Color.test.tsx | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/@react-stately/color/test/Color.test.tsx b/packages/@react-stately/color/test/Color.test.tsx index be3a04286d2..2ee7c5cd944 100644 --- a/packages/@react-stately/color/test/Color.test.tsx +++ b/packages/@react-stately/color/test/Color.test.tsx @@ -140,6 +140,12 @@ describe('Color', function () { expect(color.getChannelValue('alpha')).toBe(0); expect(color.toString('hsla')).toBe('hsla(320, 100%, 0%, 0)'); }); + + it('should keep 360 as hue value', function () { + let color = parseColor('hsl(360, 100%, 50%)'); + expect(color.getChannelValue('hue')).toBe(360); + expect(color.toString('hsl')).toBe('hsl(360, 100%, 50%)'); + }); }); it('withChannelValue', () => { @@ -180,6 +186,12 @@ describe('Color', function () { expect(color.getChannelValue('alpha')).toBe(0); expect(color.toString('hsba')).toBe('hsba(320, 100%, 0%, 0)'); }); + + it('should keep 360 as hue value', function () { + let color = parseColor('hsb(360, 100%, 50%)'); + expect(color.getChannelValue('hue')).toBe(360); + expect(color.toString('hsb')).toBe('hsb(360, 100%, 50%)'); + }); }); describe('conversions', () => { From a280e3405756aad33d3d524ba4a2fba26abe44ba Mon Sep 17 00:00:00 2001 From: Mosaad <48773133+theMosaad@users.noreply.github.com> Date: Mon, 27 May 2024 16:45:20 +0300 Subject: [PATCH 3/4] rename tests --- packages/@react-stately/color/test/Color.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/@react-stately/color/test/Color.test.tsx b/packages/@react-stately/color/test/Color.test.tsx index 2ee7c5cd944..d19223ba1e7 100644 --- a/packages/@react-stately/color/test/Color.test.tsx +++ b/packages/@react-stately/color/test/Color.test.tsx @@ -141,7 +141,7 @@ describe('Color', function () { expect(color.toString('hsla')).toBe('hsla(320, 100%, 0%, 0)'); }); - it('should keep 360 as hue value', function () { + it('should allow 360 as a hue value', function () { let color = parseColor('hsl(360, 100%, 50%)'); expect(color.getChannelValue('hue')).toBe(360); expect(color.toString('hsl')).toBe('hsl(360, 100%, 50%)'); @@ -187,7 +187,7 @@ describe('Color', function () { expect(color.toString('hsba')).toBe('hsba(320, 100%, 0%, 0)'); }); - it('should keep 360 as hue value', function () { + it('should allow 360 as a hue value', function () { let color = parseColor('hsb(360, 100%, 50%)'); expect(color.getChannelValue('hue')).toBe(360); expect(color.toString('hsb')).toBe('hsb(360, 100%, 50%)'); From 6308660a269a23d5c40789c0e3bbd54d19ad7374 Mon Sep 17 00:00:00 2001 From: Mosaad <48773133+theMosaad@users.noreply.github.com> Date: Tue, 28 May 2024 10:57:59 +0300 Subject: [PATCH 4/4] revert useColorWheelState.ts --- .../@react-stately/color/src/useColorWheelState.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/@react-stately/color/src/useColorWheelState.ts b/packages/@react-stately/color/src/useColorWheelState.ts index 0220851ca70..5ddc1670f29 100644 --- a/packages/@react-stately/color/src/useColorWheelState.ts +++ b/packages/@react-stately/color/src/useColorWheelState.ts @@ -11,7 +11,7 @@ */ import {Color, ColorWheelProps} from '@react-types/color'; -import {normalizeColor, normalizeHue, parseColor} from './Color'; +import {normalizeColor, parseColor} from './Color'; import {useControlledState} from '@react-stately/utils'; import {useMemo, useRef, useState} from 'react'; @@ -57,6 +57,10 @@ function roundToStep(value: number, step: number): number { return Math.round(value / step) * step; } +function mod(n: number, m: number) { + return ((n % m) + m) % m; +} + function roundDown(v: number) { let r = Math.floor(v); if (r === v) { @@ -120,7 +124,7 @@ export function useColorWheelState(props: ColorWheelProps): ColorWheelState { // Make sure you can always get back to 0. v = 0; } - v = roundToStep(normalizeHue(v), step); + v = roundToStep(mod(v, 360), step); if (hue !== v) { let color = value.withChannelValue('hue', v); setValue(color); @@ -150,7 +154,7 @@ export function useColorWheelState(props: ColorWheelProps): ColorWheelState { // Make sure you can always get back to 0. newValue = minValueX; } - setHue(roundToStep(normalizeHue(newValue), s)); + setHue(roundToStep(mod(newValue, 360), s)); }, decrement(stepSize = 1) { let s = Math.max(stepSize, step); @@ -159,7 +163,7 @@ export function useColorWheelState(props: ColorWheelProps): ColorWheelState { // |(previous step) - 0| < step size setHue(roundDown(360 / s) * s); } else { - setHue(roundToStep(normalizeHue(hue - s), s)); + setHue(roundToStep(mod(hue - s, 360), s)); } }, setDragging(isDragging) {