fix: color picker controlled value (#47816)

* fix: fix control value not show in dom

* chore: update title

* add test case

* change test title

* fix: cleared color should change after controlled value changed

* chore: code clean

* test: change test

* comment

* fix: should respect empty string

* test: add test case

* chore: update demo

---------

Co-authored-by: tanghui <yoyo837@hotmail.com>
Co-authored-by: afc163 <afc163@gmail.com>
Co-authored-by: lijianan <574980606@qq.com>

components/_util/getAllowClear.tsx

@@ -13,7 +13,7 @@ const getAllowClear = (allowClear: AllowClear): AllowClear => {
       clearIcon: <CloseCircleFilled />,
     };
   }
-  
+
   return mergedAllowClear;
 };
 

components/color-picker/ColorPicker.tsx

@@ -1,9 +1,5 @@
-import type { CSSProperties, FC } from 'react';
+import React, { useContext, useMemo, useRef } from 'react';
-import React, { useContext, useMemo, useRef, useState } from 'react';
+import type { HsbaColorType } from '@rc-component/color-picker';
-import type {
-  HsbaColorType,
-  ColorPickerProps as RcColorPickerProps,
-} from '@rc-component/color-picker';
 import classNames from 'classnames';
 import useMergedState from 'rc-util/lib/hooks/useMergedState';
 
@@ -15,7 +11,6 @@ import { ConfigContext } from '../config-provider/context';
 import DisabledContext from '../config-provider/DisabledContext';
 import useCSSVarCls from '../config-provider/hooks/useCSSVarCls';
 import useSize from '../config-provider/hooks/useSize';
-import type { SizeType } from '../config-provider/SizeContext';
 import { FormItemInputContext, NoFormStyle } from '../form/context';
 import type { PopoverProps } from '../popover';
 import Popover from '../popover';
@@ -23,50 +18,10 @@ import type { Color } from './color';
 import ColorPickerPanel from './ColorPickerPanel';
 import ColorTrigger from './components/ColorTrigger';
 import useColorState from './hooks/useColorState';
-import type {
+import type { ColorPickerBaseProps, ColorPickerProps, TriggerPlacement } from './interface';
-  ColorFormat,
-  ColorPickerBaseProps,
-  ColorValueType,
-  PresetsItem,
-  TriggerPlacement,
-  TriggerType,
-} from './interface';
 import useStyle from './style';
 import { genAlphaColor, generateColor, getAlphaColor } from './util';
 
-export type ColorPickerProps = Omit<
-  RcColorPickerProps,
-  'onChange' | 'value' | 'defaultValue' | 'panelRender' | 'disabledAlpha' | 'onChangeComplete'
-> & {
-  value?: ColorValueType;
-  defaultValue?: ColorValueType;
-  children?: React.ReactNode;
-  open?: boolean;
-  disabled?: boolean;
-  placement?: TriggerPlacement;
-  trigger?: TriggerType;
-  format?: keyof typeof ColorFormat;
-  defaultFormat?: keyof typeof ColorFormat;
-  allowClear?: boolean;
-  presets?: PresetsItem[];
-  arrow?: boolean | { pointAtCenter: boolean };
-  panelRender?: (
-    panel: React.ReactNode,
-    extra: { components: { Picker: FC; Presets: FC } },
-  ) => React.ReactNode;
-  showText?: boolean | ((color: Color) => React.ReactNode);
-  size?: SizeType;
-  styles?: { popup?: CSSProperties; popupOverlayInner?: CSSProperties };
-  rootClassName?: string;
-  disabledAlpha?: boolean;
-  [key: `data-${string}`]: string;
-  onOpenChange?: (open: boolean) => void;
-  onFormatChange?: (format: ColorFormat) => void;
-  onChange?: (value: Color, hex: string) => void;
-  onClear?: () => void;
-  onChangeComplete?: (value: Color) => void;
-} & Pick<PopoverProps, 'getPopupContainer' | 'autoAdjustOverflow' | 'destroyTooltipOnHide'>;
-
 type CompoundedComponent = React.FC<ColorPickerProps> & {
   _InternalPanelDoNotUseOrYouWillBeFired: typeof PurePanel;
 };
@@ -109,7 +64,7 @@ const ColorPicker: CompoundedComponent = (props) => {
   const contextDisabled = useContext(DisabledContext);
   const mergedDisabled = disabled ?? contextDisabled;
 
-  const [colorValue, setColorValue] = useColorState('', {
+  const [colorValue, setColorValue, prevValue] = useColorState('', {
     value,
     defaultValue,
   });
@@ -124,8 +79,6 @@ const ColorPicker: CompoundedComponent = (props) => {
     onChange: onFormatChange,
   });
 
-  const [colorCleared, setColorCleared] = useState(!value && !defaultValue);
-
   const prefixCls = getPrefixCls('color-picker', customizePrefixCls);
 
   const isAlphaColor = useMemo(() => getAlphaColor(colorValue) < 100, [colorValue]);
@@ -167,14 +120,16 @@ const ColorPicker: CompoundedComponent = (props) => {
 
   const handleChange = (data: Color, type?: HsbaColorType, pickColor?: boolean) => {
     let color: Color = generateColor(data);
+
+    // If color is cleared, reset alpha to 100
     const isNull = value === null || (!value && defaultValue === null);
-    if (colorCleared || isNull) {
+    if (prevValue.current?.cleared || isNull) {
-      setColorCleared(false);
       // ignore alpha slider
       if (getAlphaColor(colorValue) === 0 && type !== 'alpha') {
         color = genAlphaColor(color);
       }
     }
+
     // ignore alpha color
     if (disabledAlpha && isAlphaColor) {
       color = genAlphaColor(color);
@@ -192,7 +147,6 @@ const ColorPicker: CompoundedComponent = (props) => {
   };
 
   const handleClear = () => {
-    setColorCleared(true);
     onClear?.();
   };
 
@@ -221,7 +175,6 @@ const ColorPicker: CompoundedComponent = (props) => {
     prefixCls,
     color: colorValue,
     allowClear,
-    colorCleared,
     disabled: mergedDisabled,
     disabledAlpha,
     presets,
@@ -262,13 +215,12 @@ const ColorPicker: CompoundedComponent = (props) => {
           open={popupOpen}
           className={mergedCls}
           style={mergedStyle}
-          color={value ? generateColor(value) : colorValue}
           prefixCls={prefixCls}
           disabled={mergedDisabled}
-          colorCleared={colorCleared}
           showText={showText}
           format={formatValue}
           {...rest}
+          color={colorValue}
         />
       )}
     </Popover>,

components/color-picker/__tests__/index.test.tsx

@@ -1,4 +1,4 @@
-import React, { useMemo, useState } from 'react';
+import React, { useEffect, useMemo, useState } from 'react';
 import { createEvent, fireEvent, render } from '@testing-library/react';
 import { spyElementPrototypes } from 'rc-util/lib/test/domHook';
 
@@ -11,8 +11,9 @@ import ConfigProvider from '../../config-provider';
 import Form from '../../form';
 import theme from '../../theme';
 import type { Color } from '../color';
-import type { ColorPickerProps } from '../ColorPicker';
 import ColorPicker from '../ColorPicker';
+import type { ColorPickerProps, ColorValueType } from '../interface';
+import { generateColor } from '../util';
 
 function doMouseMove(
   container: HTMLElement,
@@ -607,4 +608,94 @@ describe('ColorPicker', () => {
     const { container } = render(<ColorPicker />);
     expect(container.querySelector('.ant-color-picker-clear')).toBeTruthy();
   });
+
+  ['', null].forEach((value) => {
+    it(`When controlled and without an initial value, then changing the controlled value to valid color should be reflected correctly on the DOM. [${String(
+      value,
+    )}]`, async () => {
+      const Demo = () => {
+        const [color, setColor] = useState<ColorValueType>(value);
+        useEffect(() => {
+          setColor(generateColor('red'));
+        }, []);
+        return <ColorPicker value={color} />;
+      };
+      const { container } = render(<Demo />);
+      await waitFakeTimer();
+      expect(container.querySelector('.ant-color-picker-color-block-inner')).toHaveStyle({
+        background: 'rgb(255, 0, 0)',
+      });
+    });
+
+    it(`When controlled and has an initial value, then changing the controlled value to cleared color should be reflected correctly on the DOM. [${String(
+      value,
+    )}]`, async () => {
+      const Demo = () => {
+        const [color, setColor] = useState<ColorValueType>(generateColor('red'));
+        useEffect(() => {
+          setColor(value);
+        }, []);
+        return <ColorPicker value={color} />;
+      };
+      const { container } = render(<Demo />);
+      await waitFakeTimer();
+      expect(container.querySelector('.ant-color-picker-clear')).toBeTruthy();
+    });
+  });
+
+  it('Controlled string value should work with allowClear correctly', async () => {
+    const Demo = (props: any) => {
+      const [color, setColor] = useState<ColorValueType>(generateColor('red'));
+
+      useEffect(() => {
+        if (typeof props.value !== 'undefined') {
+          setColor(props.value);
+        }
+      }, [props.value]);
+
+      return (
+        <ColorPicker value={color} onChange={(e) => setColor(e.toHexString())} open allowClear />
+      );
+    };
+    const { container, rerender } = render(<Demo />);
+    await waitFakeTimer();
+    expect(
+      container.querySelector('.ant-color-picker-trigger .ant-color-picker-clear'),
+    ).toBeFalsy();
+    fireEvent.click(container.querySelector('.ant-color-picker-clear')!);
+    expect(
+      container.querySelector('.ant-color-picker-trigger .ant-color-picker-clear'),
+    ).toBeTruthy();
+    rerender(<Demo value="#1677ff" />);
+    expect(
+      container.querySelector('.ant-color-picker-trigger .ant-color-picker-clear'),
+    ).toBeFalsy();
+  });
+
+  it('Controlled value should work with allowClear correctly', async () => {
+    const Demo = (props: any) => {
+      const [color, setColor] = useState<ColorValueType>(generateColor('red'));
+
+      useEffect(() => {
+        if (typeof props.value !== 'undefined') {
+          setColor(props.value);
+        }
+      }, [props.value]);
+
+      return <ColorPicker value={color} onChange={(e) => setColor(e)} open allowClear />;
+    };
+    const { container, rerender } = render(<Demo />);
+    await waitFakeTimer();
+    expect(
+      container.querySelector('.ant-color-picker-trigger .ant-color-picker-clear'),
+    ).toBeFalsy();
+    fireEvent.click(container.querySelector('.ant-color-picker-clear')!);
+    expect(
+      container.querySelector('.ant-color-picker-trigger .ant-color-picker-clear'),
+    ).toBeTruthy();
+    rerender(<Demo value="#1677ff" />);
+    expect(
+      container.querySelector('.ant-color-picker-trigger .ant-color-picker-clear'),
+    ).toBeFalsy();
+  });
 });

components/color-picker/color.ts

@@ -11,16 +11,21 @@ export interface Color
   extends Pick<
     RcColor,
     'toHsb' | 'toHsbString' | 'toHex' | 'toHexString' | 'toRgb' | 'toRgbString'
-  > {}
+  > {
+  cleared: boolean | 'controlled';
+}
 
-export class ColorFactory {
+export class ColorFactory implements Color {
   /** Original Color object */
   private metaColor: RcColor;
 
+  public cleared: boolean = false;
+
   constructor(color: ColorGenInput<Color>) {
     this.metaColor = new RcColor(color as ColorGenInput);
     if (!color) {
       this.metaColor.setAlpha(0);
+      this.cleared = true;
     }
   }
 

components/color-picker/components/ColorClear.tsx

@@ -4,17 +4,18 @@ import type { Color } from '../color';
 import type { ColorPickerBaseProps } from '../interface';
 import { generateColor } from '../util';
 
-interface ColorClearProps extends Pick<ColorPickerBaseProps, 'prefixCls' | 'colorCleared'> {
+interface ColorClearProps extends Pick<ColorPickerBaseProps, 'prefixCls'> {
   value?: Color;
   onChange?: (value: Color) => void;
 }
 
-const ColorClear: FC<ColorClearProps> = ({ prefixCls, value, colorCleared, onChange }) => {
+const ColorClear: FC<ColorClearProps> = ({ prefixCls, value, onChange }) => {
   const handleClick = () => {
-    if (value && !colorCleared) {
+    if (value && !value.cleared) {
       const hsba = value.toHsb();
       hsba.a = 0;
       const genColor = generateColor(hsba);
+      genColor.cleared = true;
       onChange?.(genColor);
     }
   };

components/color-picker/components/ColorTrigger.tsx

@@ -2,14 +2,13 @@ import { ColorBlock } from '@rc-component/color-picker';
 import classNames from 'classnames';
 import type { CSSProperties, MouseEventHandler } from 'react';
 import React, { forwardRef, useMemo } from 'react';
-import type { ColorPickerProps } from '../ColorPicker';
+import type { ColorPickerProps, ColorPickerBaseProps } from '../interface';
-import type { ColorPickerBaseProps } from '../interface';
 import { getAlphaColor } from '../util';
 import ColorClear from './ColorClear';
 
-interface colorTriggerProps
+export interface ColorTriggerProps
-  extends Pick<ColorPickerBaseProps, 'prefixCls' | 'colorCleared' | 'disabled' | 'format'> {
+  extends Pick<ColorPickerBaseProps, 'prefixCls' | 'disabled' | 'format'> {
-  color: Exclude<ColorPickerBaseProps['color'], undefined>;
+  color: NonNullable<ColorPickerBaseProps['color']>;
   open?: boolean;
   showText?: ColorPickerProps['showText'];
   className?: string;
@@ -19,19 +18,18 @@ interface colorTriggerProps
   onMouseLeave?: MouseEventHandler<HTMLDivElement>;
 }
 
-const ColorTrigger = forwardRef<HTMLDivElement, colorTriggerProps>((props, ref) => {
+const ColorTrigger = forwardRef<HTMLDivElement, ColorTriggerProps>((props, ref) => {
-  const { color, prefixCls, open, colorCleared, disabled, format, className, showText, ...rest } =
+  const { color, prefixCls, open, disabled, format, className, showText, ...rest } = props;
-    props;
   const colorTriggerPrefixCls = `${prefixCls}-trigger`;
 
   const containerNode = useMemo<React.ReactNode>(
     () =>
-      colorCleared ? (
+      color.cleared ? (
         <ColorClear prefixCls={prefixCls} />
       ) : (
         <ColorBlock prefixCls={prefixCls} color={color.toRgbString()} />
       ),
-    [color, colorCleared, prefixCls],
+    [color, prefixCls],
   );
 
   const genColorString = () => {

components/color-picker/components/PanelPicker.tsx

@@ -7,11 +7,12 @@ import { PanelPickerContext } from '../context';
 import type { ColorPickerBaseProps } from '../interface';
 import ColorClear from './ColorClear';
 import ColorInput from './ColorInput';
+import { generateColor } from '../util';
 
 export interface PanelPickerProps
   extends Pick<
     ColorPickerBaseProps,
-    'prefixCls' | 'colorCleared' | 'allowClear' | 'disabledAlpha' | 'onChangeComplete'
+    'prefixCls' | 'allowClear' | 'disabledAlpha' | 'onChangeComplete'
   > {
   value?: Color;
   onChange?: (value?: Color, type?: HsbaColorType, pickColor?: boolean) => void;
@@ -21,7 +22,6 @@ export interface PanelPickerProps
 const PanelPicker: FC = () => {
   const {
     prefixCls,
-    colorCleared,
     allowClear,
     value,
     disabledAlpha,
@@ -36,7 +36,6 @@ const PanelPicker: FC = () => {
         <ColorClear
           prefixCls={prefixCls}
           value={value}
-          colorCleared={colorCleared}
           onChange={(clearColor) => {
             onChange?.(clearColor);
             onClear?.();
@@ -48,8 +47,12 @@ const PanelPicker: FC = () => {
         prefixCls={prefixCls}
         value={value?.toHsb()}
         disabledAlpha={disabledAlpha}
-        onChange={(colorValue, type) => onChange?.(colorValue, type, true)}
+        onChange={(colorValue, type) => {
-        onChangeComplete={onChangeComplete}
+          onChange?.(generateColor(colorValue), type, true);
+        }}
+        onChangeComplete={(colorValue) => {
+          onChangeComplete?.(generateColor(colorValue));
+        }}
       />
       <ColorInput
         value={value}

components/color-picker/demo/allowClear.tsx

@@ -1,4 +1,16 @@
 import React from 'react';
 import { ColorPicker } from 'antd';
 
-export default () => <ColorPicker defaultValue="#1677ff" allowClear />;
+export default () => {
+  const [color, setColor] = React.useState<string>('#1677ff');
+
+  return (
+    <ColorPicker
+      value={color}
+      allowClear
+      onChange={(c) => {
+        setColor(c.toHexString());
+      }}
+    />
+  );
+};

components/color-picker/hooks/useColorState.ts

@@ -1,4 +1,4 @@
-import { useEffect, useState } from 'react';
+import { useEffect, useRef, useState } from 'react';
 
 import type { Color } from '../color';
 import type { ColorValueType } from '../interface';
@@ -11,9 +11,10 @@ function hasValue(value?: ColorValueType) {
 const useColorState = (
   defaultStateValue: ColorValueType,
   option: { defaultValue?: ColorValueType; value?: ColorValueType },
-): readonly [Color, React.Dispatch<React.SetStateAction<Color>>] => {
+) => {
   const { defaultValue, value } = option;
-  const [colorValue, setColorValue] = useState<Color>(() => {
+  const prevColor = useRef<Color>(generateColor(''));
+  const [colorValue, _setColorValue] = useState<Color>(() => {
     let mergedState: ColorValueType | undefined;
     if (hasValue(value)) {
       mergedState = value;
@@ -22,16 +23,27 @@ const useColorState = (
     } else {
       mergedState = defaultStateValue;
     }
-    return generateColor(mergedState || '');
+    const color = generateColor(mergedState || '');
+    prevColor.current = color;
+    return color;
   });
 
+  const setColorValue: typeof _setColorValue = (color: Color) => {
+    _setColorValue(color);
+    prevColor.current = color;
+  };
+
   useEffect(() => {
-    if (value) {
+    if (hasValue(value)) {
-      setColorValue(generateColor(value));
+      const newColor = generateColor(value || '');
+      if (prevColor.current.cleared === true) {
+        newColor.cleared = 'controlled';
+      }
+      setColorValue(newColor);
     }
   }, [value]);
 
-  return [colorValue, setColorValue] as const;
+  return [colorValue, setColorValue, prevColor] as const;
 };
 
 export default useColorState;

components/color-picker/index.ts

@@ -1,6 +1,6 @@
 import ColorPicker from './ColorPicker';
 
-export type { ColorPickerProps } from './ColorPicker';
+export type { ColorPickerProps } from './interface';
 export type { Color } from './color';
 
 export default ColorPicker;

components/color-picker/interface.ts

@@ -1,6 +1,8 @@
-import type { ReactNode } from 'react';
+import type { CSSProperties, FC, ReactNode } from 'react';
-import type { ColorPickerProps } from './ColorPicker';
 import type { Color } from './color';
+import type { ColorPickerProps as RcColorPickerProps } from '@rc-component/color-picker';
+import type { SizeType } from '../config-provider/SizeContext';
+import type { PopoverProps } from '../popover';
 
 export enum ColorFormat {
   hex = 'hex',
@@ -32,7 +34,6 @@ export interface ColorPickerBaseProps {
   prefixCls: string;
   format?: keyof typeof ColorFormat;
   allowClear?: boolean;
-  colorCleared?: boolean;
   disabled?: boolean;
   disabledAlpha?: boolean;
   presets?: PresetsItem[];
@@ -42,3 +43,36 @@ export interface ColorPickerBaseProps {
 }
 
 export type ColorValueType = Color | string | null;
+
+export type ColorPickerProps = Omit<
+  RcColorPickerProps,
+  'onChange' | 'value' | 'defaultValue' | 'panelRender' | 'disabledAlpha' | 'onChangeComplete'
+> & {
+  value?: ColorValueType;
+  defaultValue?: ColorValueType;
+  children?: React.ReactNode;
+  open?: boolean;
+  disabled?: boolean;
+  placement?: TriggerPlacement;
+  trigger?: TriggerType;
+  format?: keyof typeof ColorFormat;
+  defaultFormat?: keyof typeof ColorFormat;
+  allowClear?: boolean;
+  presets?: PresetsItem[];
+  arrow?: boolean | { pointAtCenter: boolean };
+  panelRender?: (
+    panel: React.ReactNode,
+    extra: { components: { Picker: FC; Presets: FC } },
+  ) => React.ReactNode;
+  showText?: boolean | ((color: Color) => React.ReactNode);
+  size?: SizeType;
+  styles?: { popup?: CSSProperties; popupOverlayInner?: CSSProperties };
+  rootClassName?: string;
+  disabledAlpha?: boolean;
+  [key: `data-${string}`]: string;
+  onOpenChange?: (open: boolean) => void;
+  onFormatChange?: (format: ColorFormat) => void;
+  onChange?: (value: Color, hex: string) => void;
+  onClear?: () => void;
+  onChangeComplete?: (value: Color) => void;
+} & Pick<PopoverProps, 'getPopupContainer' | 'autoAdjustOverflow' | 'destroyTooltipOnHide'>;