Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions assets/index.less
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,11 @@
border: @handler-border-size solid #fff;
border-radius: 50%;
box-shadow: 0 0 1px 1px rgba(0, 0, 0, 0.06);
&:focus-visible,
&:focus-within {
outline: 2px solid #1677ff;
outline-offset: 1px;
}
}
&-handler-sm {
width: @sm-handler-size;
Expand Down
18 changes: 18 additions & 0 deletions src/ColorPicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,15 @@ import useColorState from './hooks/useColorState';
import useComponent, { type Components } from './hooks/useComponent';
import type { BaseColorPickerProps, ColorGenInput } from './interface';

const defaultLocale: Required<BaseColorPickerProps>['locale'] = {
picker: 'Color picker',
pickerDescription: '2D slider',
hue: 'Hue',
alpha: 'Alpha',
saturation: 'Saturation',
brightness: 'Brightness',
};

const HUE_COLORS = [
{
color: 'rgb(255, 0, 0)',
Expand Down Expand Up @@ -67,8 +76,14 @@ const ColorPicker = forwardRef<HTMLDivElement, ColorPickerProps>(
disabledAlpha = false,
disabled = false,
components,
locale,
} = props;

const mergedLocale = useMemo(
() => ({ ...defaultLocale, ...locale }),
[locale],
);

// ========================== Components ==========================
const [Slider] = useComponent(components);

Expand Down Expand Up @@ -134,6 +149,7 @@ const ColorPicker = forwardRef<HTMLDivElement, ColorPickerProps>(
<Picker
onChange={handleChange}
{...sharedSliderProps}
locale={mergedLocale}
onChangeComplete={onChangeComplete}
/>
<div className={`${prefixCls}-slider-container`}>
Expand All @@ -151,6 +167,7 @@ const ColorPicker = forwardRef<HTMLDivElement, ColorPickerProps>(
value={colorValue.getHue()}
onChange={onHueChange}
onChangeComplete={onHueChangeComplete}
aria-label={mergedLocale.hue}
/>
{!disabledAlpha && (
<Slider
Expand All @@ -165,6 +182,7 @@ const ColorPicker = forwardRef<HTMLDivElement, ColorPickerProps>(
value={colorValue.a * 100}
onChange={onAlphaChange}
onChangeComplete={onAlphaChangeComplete}
aria-label={mergedLocale.alpha}
/>
)}
</div>
Expand Down
14 changes: 3 additions & 11 deletions src/components/ColorBlock.tsx
Original file line number Diff line number Diff line change
@@ -1,34 +1,26 @@
import { clsx } from 'clsx';
import React from 'react';

export type ColorBlockProps = {
export type ColorBlockProps = React.HTMLAttributes<HTMLDivElement> & {
color: string;
prefixCls?: string;
className?: string;
style?: React.CSSProperties;
/** Internal usage. Only used in antd ColorPicker semantic structure only */
innerClassName?: string;
/** Internal usage. Only used in antd ColorPicker semantic structure only */
innerStyle?: React.CSSProperties;
onClick?: React.MouseEventHandler<HTMLDivElement>;
};

const ColorBlock: React.FC<ColorBlockProps> = ({
color,
prefixCls,
className,
style,
innerClassName,
innerStyle,
onClick,
...props
}) => {
const colorBlockCls = `${prefixCls}-color-block`;
return (
<div
className={clsx(colorBlockCls, className)}
style={style}
onClick={onClick}
>
<div {...props} className={clsx(colorBlockCls, className)}>
<div
className={clsx(`${colorBlockCls}-inner`, innerClassName)}
style={{ background: color, ...innerStyle }}
Expand Down
96 changes: 92 additions & 4 deletions src/components/Handler.tsx
Original file line number Diff line number Diff line change
@@ -1,20 +1,108 @@
import { omit } from '@rc-component/util';
import { clsx } from 'clsx';
import React from 'react';

type HandlerSize = 'default' | 'small';

const Handler: React.FC<{
/** Keyboard keys (handled natively by `<input type="range">`) that mutate the value. */
const VALUE_KEYS = [
'ArrowLeft',
'ArrowRight',
'ArrowUp',
'ArrowDown',
'Home',
'End',
'PageUp',
'PageDown',
];

const isVerticalKey = (key: string) => key === 'ArrowUp' || key === 'ArrowDown';

// The range input is a keyboard / screen-reader proxy only — the visible thumb
// is the wrapping <div>. It must stay focusable and in the a11y tree, so it is
// hidden with `opacity` (not `display`/`visibility`). These styles are inlined
// rather than left to the stylesheet so consumers that don't ship our CSS
// (e.g. antd's own styling) still get a hidden input out of the box.
const RANGE_INPUT_STYLE: React.CSSProperties = {
position: 'absolute',
inset: 0,
width: '100%',
height: '100%',
margin: 0,
padding: 0,
opacity: 0,
pointerEvents: 'none',
};

interface HandlerAxis
extends Omit<
React.InputHTMLAttributes<HTMLInputElement>,
'size' | 'value' | 'onChange'
> {
value: number;
onChange: (value: number) => void;
onChangeComplete: (value: number) => void;
}

export interface HandlerProps {
size?: HandlerSize;
color?: string;
prefixCls?: string;
}> = ({ size = 'default', color, prefixCls }) => {
disabled?: boolean;
x: HandlerAxis;
y?: HandlerAxis;
}

const Handler: React.FC<HandlerProps> = ({
size = 'default',
color,
prefixCls,
disabled,
x,
y,
}) => {
// The browser ignores Up/Down on a horizontal range, so the vertical axis is
// handled here: clamp to its own [min, max] and emit through its callbacks.
const handleKeyDown = (event: React.KeyboardEvent<HTMLInputElement>) => {
if (!y || !isVerticalKey(event.key)) {
return;
}
event.preventDefault();
const step = Number(y.step ?? 1) || 1;
const min = Number(y.min ?? 0);
const max = Number(y.max ?? 100);
const delta = event.key === 'ArrowUp' ? step : -step;
y.onChange(Math.min(max, Math.max(min, y.value + delta)));
};

return (
<div
className={clsx(`${prefixCls}-handler`, {
[`${prefixCls}-handler-sm`]: size === 'small',
})}
style={{ backgroundColor: color }}
/>
style={{ position: 'relative', backgroundColor: color }}
>
<input
step={1}
{...omit(x, ['onChangeComplete'])}
type="range"
className={`${prefixCls}-handler-range`}
style={RANGE_INPUT_STYLE}
disabled={disabled}
onChange={event => x.onChange(Number(event.target.value))}
onKeyDown={y ? handleKeyDown : undefined}
onKeyUp={event => {
if (!VALUE_KEYS.includes(event.key)) {
return;
}
if (y && isVerticalKey(event.key)) {
y.onChangeComplete(y.value);
} else {
x.onChangeComplete(Number(event.currentTarget.value));
}
}}
/>
</div>
);
};
Comment on lines +56 to 107

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Since the vertical axis (y) is not a native range input, its value changes are handled entirely in JavaScript via onKeyDown. In controlled mode, rapid key presses or holding down the arrow keys can cause handleKeyDown and onKeyUp to read stale y.value props before the parent component has committed the state update and re-rendered.

To ensure smooth and correct keyboard navigation for the vertical axis, we should track the latest value in a mutable ref (lastValueRef) and update it synchronously on each key down/up event.

const Handler: React.FC<HandlerProps> = ({
  size = 'default',
  color,
  prefixCls,
  disabled,
  x,
  y,
}) => {
  const lastValueRef = React.useRef(y?.value);
  if (y && y.value !== lastValueRef.current) {
    lastValueRef.current = y.value;
  }

  // The browser ignores Up/Down on a horizontal range, so the vertical axis is
  // handled here: clamp to its own [min, max] and emit through its callbacks.
  const handleKeyDown = (event: React.KeyboardEvent<HTMLInputElement>) => {
    if (!y || !isVerticalKey(event.key)) {
      return;
    }
    event.preventDefault();
    const step = Number(y.step ?? 1) || 1;
    const min = Number(y.min ?? 0);
    const max = Number(y.max ?? 100);
    const delta = event.key === 'ArrowUp' ? step : -step;
    const nextValue = Math.min(max, Math.max(min, (lastValueRef.current ?? y.value) + delta));
    lastValueRef.current = nextValue;
    y.onChange(nextValue);
  };

  return (
    <div
      className={clsx(prefixCls + "-handler", {
        [prefixCls + "-handler-sm"]: size === "small",
      })}
      style={{ position: 'relative', backgroundColor: color }}
    >
      <input
        step={1}
        {...omit(x, ['onChangeComplete'])}
        type="range"
        className={prefixCls + "-handler-range"}
        style={RANGE_INPUT_STYLE}
        disabled={disabled}
        onChange={event => x.onChange(Number(event.target.value))}
        onKeyDown={y ? handleKeyDown : undefined}
        onKeyUp={event => {
          if (!VALUE_KEYS.includes(event.key)) {
            return;
          }
          if (y && isVerticalKey(event.key)) {
            y.onChangeComplete(lastValueRef.current ?? y.value);
          } else {
            x.onChangeComplete(Number(event.currentTarget.value));
          }
        }}
      />
    </div>
  );
};


Expand Down
38 changes: 36 additions & 2 deletions src/components/Picker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import type { FC } from 'react';
import React, { useRef } from 'react';
import useColorDrag from '../hooks/useColorDrag';
import type { BaseColorPickerProps, TransformOffset } from '../interface';
import { calcOffset, calculateColor } from '../util';
import { calcOffset, calculateColor, generateColor } from '../util';

import { useEvent } from '@rc-component/util';
import Handler from './Handler';
Expand All @@ -17,6 +17,7 @@ const Picker: FC<PickerProps> = ({
prefixCls,
onChangeComplete,
disabled,
locale,
}) => {
const pickerRef = useRef();
const transformRef = useRef();
Expand All @@ -43,6 +44,16 @@ const Picker: FC<PickerProps> = ({
disabledDrag: disabled,
});

// ===================== Keyboard (2-D handler) =====================
const hsb = color.toHsb();

// Build a new color from the current one with a single HSB channel changed.
const changeColor = (channel: 's' | 'b', percent: number) => {
const next = generateColor({ ...hsb, [channel]: percent / 100 });
colorRef.current = next;
onChange(next);
};
Comment on lines +47 to +55

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

In a controlled component, rapid keyboard interactions (like pressing or holding arrow keys) can trigger multiple state updates before the parent component re-renders and passes down the updated color prop. Because changeColor derives the next color from the potentially stale color prop, rapid changes to one channel (e.g., saturation) can revert changes made to the other channel (e.g., brightness) in rapid succession.

To prevent this race condition, we should keep colorRef.current in sync with the color prop during render, and derive the next color from colorRef.current instead of the color prop.

Suggested change
// ===================== Keyboard (2-D handler) =====================
const hsb = color.toHsb();
// Build a new color from the current one with a single HSB channel changed.
const changeColor = (channel: 's' | 'b', percent: number) => {
const next = generateColor({ ...hsb, [channel]: percent / 100 });
colorRef.current = next;
onChange(next);
};
// ===================== Keyboard (2-D handler) =====================
colorRef.current = color;
// Build a new color from the current one with a single HSB channel changed.
const changeColor = (channel: 's' | 'b', percent: number) => {
const currentHsb = colorRef.current.toHsb();
const next = generateColor({ ...currentHsb, [channel]: percent / 100 });
colorRef.current = next;
onChange(next);
};


return (
<div
ref={pickerRef}
Expand All @@ -52,7 +63,30 @@ const Picker: FC<PickerProps> = ({
>
<Palette prefixCls={prefixCls}>
<Transform x={offset.x} y={offset.y} ref={transformRef}>
<Handler color={color.toRgbString()} prefixCls={prefixCls} />
<Handler
color={color.toRgbString()}
prefixCls={prefixCls}
disabled={disabled}
x={{
'aria-label': locale.picker,
'aria-roledescription': locale.pickerDescription,
'aria-valuetext': `${locale.saturation} ${Math.round(
hsb.s * 100,
)}%, ${locale.brightness} ${Math.round(hsb.b * 100)}%`,
min: 0,
max: 100,
value: hsb.s * 100,
onChange: percent => changeColor('s', percent),
onChangeComplete: () => onChangeComplete?.(colorRef.current),
}}
y={{
min: 0,
max: 100,
value: hsb.b * 100,
onChange: percent => changeColor('b', percent),
onChangeComplete: () => onChangeComplete?.(colorRef.current),
}}
/>
</Transform>
<div
className={`${prefixCls}-saturation`}
Expand Down
14 changes: 14 additions & 0 deletions src/components/Slider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export interface BaseSliderProps {
onChangeComplete: (value: number) => void;
type: HsbaColorType;
color: Color;
'aria-label'?: string;
}

const Slider: React.FC<BaseSliderProps> = props => {
Expand All @@ -33,6 +34,10 @@ const Slider: React.FC<BaseSliderProps> = props => {
onChangeComplete,
color,
type,
min,
max,
value,
'aria-label': ariaLabel,
} = props;

const sliderRef = useRef<HTMLDivElement>(null);
Expand Down Expand Up @@ -103,6 +108,15 @@ const Slider: React.FC<BaseSliderProps> = props => {
size="small"
color={handleColor.toHexString()}
prefixCls={prefixCls}
disabled={disabled}
x={{
'aria-label': ariaLabel,
min,
max,
value,
onChange,
onChangeComplete,
}}
/>
</Transform>
<Gradient colors={gradientList} type={type} prefixCls={prefixCls} />
Expand Down
8 changes: 8 additions & 0 deletions src/interface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ export interface BaseColorPickerProps {
color?: Color;
prefixCls?: string;
disabled?: boolean;
locale?: {
picker?: string;
pickerDescription?: string;
hue?: string;
alpha?: string;
saturation?: string;
brightness?: string;
};
onChange?: (
color: Color,
info?: { type?: HsbaColorType; value?: number },
Expand Down
Loading