feat(colorpicker): Add ColorPicker component with support for HEX, RGB and Alpha channel #2386
Conversation
|
Preview is ready. |
|
Visual Tests Report is ready. |
Estasie
changed the title
Estasie
changed the title
Button has a strange appearance. It should be squared evenly and also chess-pattern should be white/gray, not transparent, background should be visible through. |
|
Does this library has keyboard support? I can set the focus on the sliders but it seems not responding |
|
Why mixing RGB and HEX controls. If HEX selected then it should be a single field of hex value including alpha part: |
package.json
Outdated
| "i": "^0.3.7", | ||
| "lodash": "^4.17.21", | ||
| "npm": "^11.5.2", |
There was a problem hiding this comment.
Are these deps added by mistake?
| anchorElement={anchor} | ||
| onOpenChange={handleOpenChange} | ||
| > | ||
| <Flex direction={'column'} gap={2}> |
There was a problem hiding this comment.
Flex component should not be used inside the other components, it's for user-only component as it follows spacing variables which can be overwriten and we want our components to be fixed spacing
| <Popup | ||
| open={open} | ||
| className={b('popup')} | ||
| placement={'bottom-end'} |
There was a problem hiding this comment.
Only bottom-end value makes component look shifted when it rendered on the left side of the screen. Let's use 'bottom-start' | 'bottom-end' so the component could flip from side to side.
| return ( | ||
| <Card view={'outlined'} className={b('picker-wrapper')} ref={ref}> | ||
| <Flex alignItems={'center'} gap={2}> | ||
| <Button size={'s'} className={b('underlay')} onClick={onClick}> |
There was a problem hiding this comment.
Button has complex styles and instead of overriding them it's easier to use native button, reset and apply only needed styles. It will be much cleaner
I thought it would be more comfortable to use common instruments for setting opacity, percent just looks more human |
seems not, but react-colorful does |
Picker already has such instrument, the slider for opacity changing. Rest controls are specific for the format, and HEX should be a single input, so I can paste any valid color, inncluding alpha part and picker should accept it |
It's sad... |
package.json
Outdated
| "@gravity-ui/i18n": "^1.8.0", | ||
| "@gravity-ui/icons": "^2.13.0", | ||
| "@tanstack/react-virtual": "^3.13.9", | ||
| "@uiw/react-color": "^2.7.3", |
There was a problem hiding this comment.
Keyboard support has been added in 2.8.0. Let's update
|
|
||
| export interface ColorPickerProps { | ||
| withAlpha?: boolean; | ||
| mode?: Modes; |
There was a problem hiding this comment.
mode seems not very usefull if it's only affect inputs in popup. I assume we can remove it without affecting ux/dx
| mode?: Modes; | ||
| defaultColor?: HsvaColor; | ||
| onChange?: (color: HsvaColor) => void; | ||
| onClose?: () => void; |
There was a problem hiding this comment.
The component should have similar API to other inputs. Let's change API to the following:
interface ColorPickerProps {
// Same size as in the `TextInput`
size?: 's' | 'm' | 'l' | 'xl';
// if controlled value
value?: string;
// if uncontrolled value
defaultValue?: string;
// onChange naming is reserved for native event handlers
onUpdate?: (value: string) => void;
// if controlled open state
open?: boolean;
// if uncontrolled open state
defaultOpen?: boolean;
onOpenChange?: (open: boolean) => void;
// it should be false by default (common case)
withAlpha?: boolean;
}For controlled/uncontrolled state there is a hook useContolledState (see other components as an example)
There was a problem hiding this comment.
Added more props, also added 'onlyPicker' prop for button only render without value
| withAlpha?: boolean; | ||
| mode?: Modes; | ||
| defaultColor?: HsvaColor; | ||
| onChange?: (color: HsvaColor) => void; |
There was a problem hiding this comment.
What about accepting more common format as value, like simple HEX?
There was a problem hiding this comment.
I changed it to the hex, incoming as string and resolving it only for external usage, locally I use hsva for better precision
Estasie
requested review from
DakEnviy,
DaryaLari,
IsaevAlexandr,
NikitaCG,
Ruminat,
kseniya57 and
ogonkov
as code owners
No description provided.