Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: new component ColorPicker #41990

Merged
merged 35 commits into from
May 12, 2023
Merged

Conversation

RedJue
Copy link
Member

@RedJue RedJue commented Apr 25, 2023

[中文版模板 / Chinese template]

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style update
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Internationalization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Workflow
  • Other (about what?)

🔗 Related issue link

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English New Component ColorPicker
🇨🇳 Chinese 新组件颜色选择器

☑️ Self-Check before Merge

⚠️ Please check all items below before requesting a reviewing. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

Copilot

🤖 Generated by Copilot at 45700f1

Summary

🎨🧪📝

This pull request adds a new ColorPicker component to the ant-design library, which allows users to select and edit colors in different formats. It includes the component implementation, tests, demos, and documentation files. The component uses the RcColorPicker component as a base, and extends it with some custom logic, styles, and subcomponents. The component also provides a hook to manage the color state, and exports some types for external use.

ColorPicker shines
A rainbow of props and tests
Spring of components

Walkthrough

  • Add the ColorPicker component and its documentation (link, link, link, link)
  • Implement the ExPanel component to extend the RcColorPicker panel with a ColorDataBar component (link, link)
  • Define the Color interface and the ColorFactory class to wrap the RcColor class and provide custom methods and properties (link, link)
  • Define the ColorFormatEnum type and the ColorFormat, PressetsItem, and ColorPickerBaseProps types for the props and internal components of the ColorPicker component (link, link)
  • Implement the ColorHexInput, ColorPlaceholder, and ColorSteppers components to render and edit the color values in different formats (link, link, link)
  • Add the basic usage demo of the ColorPicker component and its markdown file (link, link)
  • Add the test files for the ColorPicker component and its demos, using the testing library and image snapshots (link, link, link, link)
  • Add the style index file and the style hooks and tokens for the ColorPicker component and its subcomponents (link)

@github-actions
Copy link
Contributor

github-actions bot commented Apr 25, 2023

@codecov
Copy link

codecov bot commented Apr 25, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (c76d9fe) 100.00% compared to head (0012c27) 100.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           feature    #41990    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          619       638    +19     
  Lines        10570     10826   +256     
  Branches      2886      2943    +57     
==========================================
+ Hits         10570     10826   +256     
Impacted Files Coverage Δ
components/locale/en_US.ts 100.00% <ø> (ø)
components/locale/index.tsx 100.00% <ø> (ø)
components/locale/zh_CN.ts 100.00% <ø> (ø)
components/color-picker/ColorPicker.tsx 100.00% <100.00%> (ø)
components/color-picker/ColorPickerPanel.tsx 100.00% <100.00%> (ø)
components/color-picker/color.ts 100.00% <100.00%> (ø)
...onents/color-picker/components/ColorAlphaInput.tsx 100.00% <100.00%> (ø)
components/color-picker/components/ColorClear.tsx 100.00% <100.00%> (ø)
...mponents/color-picker/components/ColorHexInput.tsx 100.00% <100.00%> (ø)
...mponents/color-picker/components/ColorHsbInput.tsx 100.00% <100.00%> (ø)
... and 12 more

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@MadCcc
Copy link
Member

MadCcc commented Apr 26, 2023

image

  • Select 高度
  • HexInput 字号不对

@MadCcc
Copy link
Member

MadCcc commented Apr 26, 2023

image

  • 这里的 handle 被隐藏了

@MadCcc
Copy link
Member

MadCcc commented Apr 26, 2023

image

  • 这两个地方应该也要加个阴影
  • Input 没有对齐

@fuji221
Copy link

fuji221 commented Apr 26, 2023

image
1.hex 模式下,输入框内字号不对
2.在popover面板,任意操作几下后,popover会收起

@fuji221
Copy link

fuji221 commented Apr 26, 2023

image

  • 默认情况下,描边是灰色,同input
  • hover、click态同input

@fuji221
Copy link

fuji221 commented Apr 26, 2023

image
拖动条、查看颜色的位置都需要加内阴影(#000000,20%)
image

@fuji221
Copy link

fuji221 commented Apr 26, 2023

image
间距为12px

@@ -2,9 +2,9 @@ import classNames from 'classnames';
import ResizeObserver from 'rc-resize-observer';
import omit from 'rc-util/lib/omit';
import React, { createRef, forwardRef, useContext } from 'react';
import throttleByAnimationFrame from '../_util/throttleByAnimationFrame';
Copy link
Member

Choose a reason for hiding this comment

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

这个文件需要回滚一下

Comment on lines 54 to 62
<Select.Option value={ColorFormatEnum.hex}>
{ColorFormatEnum.hex.toLocaleUpperCase()}
</Select.Option>
<Select.Option value={ColorFormatEnum.hsb}>
{ColorFormatEnum.hsb.toLocaleUpperCase()}
</Select.Option>
<Select.Option value={ColorFormatEnum.rgb}>
{ColorFormatEnum.rgb.toLocaleUpperCase()}
</Select.Option>
Copy link
Member

Choose a reason for hiding this comment

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

用 options

@@ -0,0 +1,6 @@
/* eslint-disable import/prefer-default-export */
export enum ColorFormatEnum {
Copy link
Member

Choose a reason for hiding this comment

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

这个写个字面量放 interface 里就行

@MadCcc
Copy link
Member

MadCcc commented Apr 27, 2023

  • 鼠标拖动到 popup 外之后松开会直接让 popup 关闭

@MadCcc
Copy link
Member

MadCcc commented Apr 27, 2023

image

  • 这个内阴影还没加

@kiner-tang
Copy link
Member

image
HSB 和 RGB 的默认值是不是展示错了,怎么都展示成了 HEX 了?


export interface ColorPickerProps
extends Omit<RcColorPickerProps, 'onChange' | 'arrow' | 'value' | 'defaultValue' | 'children'> {
value?: Color | string;
Copy link
Member

Choose a reason for hiding this comment

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

这个类型好多地方都有在用,是不是可以弄一个新类型聚合一下呢?
比如:type ColorVal = Color | string,命名上可以再斟酌一下,不过感觉有个别名,在其他地方使用的时候会方便一点

Copy link
Member Author

Choose a reason for hiding this comment

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

可以考虑加个通用类型

import React from 'react';
import ColorPicker from '../ColorPicker';

describe('ColorPicker', () => {
Copy link
Member

Choose a reason for hiding this comment

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

测试用来还没加完吗?

Copy link
Member Author

Choose a reason for hiding this comment

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

还没,还在UI走查阶段,有空我补一下

@RedJue
Copy link
Member Author

RedJue commented Apr 27, 2023

image HSB 和 RGB 的默认值是不是展示错了,怎么都展示成了 HEX 了?

demo 之前重构改的忘记修了,已经修复了


const ColorHexInput: FC<ColorHexInputProps> = ({ prefixCls, value, onChange }) => {
const ColorHexInputPrefixCls = `${prefixCls}-hex-input`;
const [hexValue, setHexValue] = useState(toHex(value?.toHexString() || ''));
Copy link
Member

Choose a reason for hiding this comment

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

如果 value 有可能为空的话,是不是可以单独定义一个变量处理兜底逻辑,这样就不用后面每次用 value 的时候都用可选链,担心太多可选链会增加包的体积,例如:

const val = useMemo(() => value ? value.toHexString() : '', [value])

这样后面直接用 val 就好了?

Copy link
Member Author

Choose a reason for hiding this comment

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

可以优化一下,可选链多了确实不好

@li-jia-nan
Copy link
Member

闲夕大佬这么晚还在推代码

@RedJue
Copy link
Member Author

RedJue commented May 10, 2023

辛苦了大佬🙏

@li-jia-nan
Copy link
Member

看样子可以上 5.5 的车了🎉

@fuji221
Copy link

fuji221 commented May 10, 2023

image
popover在上方时间距有问题

@MadCcc
Copy link
Member

MadCcc commented May 11, 2023

argos 没有截图,还得看看

@RedJue
Copy link
Member Author

RedJue commented May 11, 2023

image popover在上方时间距有问题

已经修复了,可以再看看

@fuji221
Copy link

fuji221 commented May 12, 2023

没问题了

components/color-picker/index.zh-CN.md Outdated Show resolved Hide resolved
@MadCcc MadCcc merged commit b95d567 into ant-design:feature May 12, 2023
50 of 51 checks passed
@afc163
Copy link
Member

afc163 commented May 13, 2023

图片

弹出动画方向有点不对,上面这个是从中间弹开的,应该从箭头方向弹开。

@afc163
Copy link
Member

afc163 commented May 13, 2023

图片

操作点 hover 时没有出现鼠标手型。

@afc163
Copy link
Member

afc163 commented May 13, 2023

@afc163
Copy link
Member

afc163 commented May 13, 2023

图片

@afc163
Copy link
Member

afc163 commented May 13, 2023

import type { Color } from 'antd/es/color-picker';

用 es 目录。

@afc163
Copy link
Member

afc163 commented May 13, 2023

图片
  • 这个图案是重复的,感觉 16X16 + repeat 就够了,不需要这么大。
图片

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants