adobe / react-spectrum

A collection of libraries and tools that help you build adaptive, accessible, and robust user experiences.
https://react-spectrum.adobe.com
Apache License 2.0
12.9k stars 1.12k forks source link

ColorField feedback #2296

Closed matthewdeutsch closed 2 months ago

matthewdeutsch commented 3 years ago

๐Ÿ˜ฏ Current Behavior

https://codesandbox.io/s/react-spectrum-playground-forked-r3n4h?file=/src/App.js

๐Ÿค” Expected Behavior

๐Ÿ”ฆ Context

Adobe Teams building Color Pickers for their products

๐Ÿ’ป Examples

๐Ÿงข Your Company/Team

Adobe/AJO

holblin commented 3 years ago

Here is a better description of the problems or suggestions we have: https://codesandbox.io/s/react-spectrum-playground-forked-r3n4h?file=/src/App.js

holblin commented 3 years ago

Hi @matthewdeutsch, do you have any updates?

holblin commented 3 years ago

Hi,

My image in the playground doesn't work anymore. Here is the image I was targeting: https://spectrum.adobe.com/page/color-area/ (first image on the page which shows more than the color area)

@matthewdeutsch , I saw the issue was moved to Groomed, what are the next steps? Did you have some ETA or idea about the timeline?

dannify commented 3 years ago

I think we will take a look at the ts error for ColorField but the other items likely won't be addressed this quarter.

holblin commented 2 years ago

Hi @dannify, did you have some updates on the ColorField ts error?

Currently, we used null because it allows us to clear the field but we discover it also introduces a CSS issue (the focus CSS class is still active after blurring out of the field with the keyboard).

Since the CSS issue is the less evil of both bugs we will continue like that but I would love to have a supported way to empty out or keep empty (after user modification) the color field.

holblin commented 2 years ago

Hi @snowystinger , I checked the null argument on https://codesandbox.io/s/react-spectrum-playground-forked-ixe73z but when I tab I got an error: Cannot read properties of null (reading 'toString'). I try to update the version of the dependencies and I have the same issue.

Do you think there is some adjustment to do on top of https://github.com/adobe/react-spectrum/pull/2893 for the null state?

Also, did you catch the CSS issue after the keyboard blur? (I think it might be linked)

holblin commented 2 years ago

@dannify , did you know if these two points describe here are planned:

holblin commented 2 years ago

@matthewdeutsch , could you uncheck:

I don't think these points have been addressed yet.

No Rgb/Hsl conversion: Ability to use an HSL ColorField (or another color component) with an RGB value as input/output.

Keeping multiple inputs in sync: Keeping multiple color inputs in sync while they are in a different color space (RGB + HSL)

dannify commented 2 years ago

The updates to the type have not been released yet. Release to go out today/tomorrow.

dannify commented 2 years ago

Update: Release is out. The type issue should be resolved. RGB/HSL conversion is in a PR - https://github.com/adobe/react-spectrum/pull/2887 Keeping multiple inputs in sync isn't anything we would actually do in RSP as far as I am aware. This should be handled via using all of the components needed in a controlled state.

spsDrop commented 2 years ago

PR for hex rgba color support https://github.com/adobe/react-spectrum/pull/3228

snowystinger commented 2 months ago

I believe everything in this issue has been addressed. We can reopen if I'm mistaken. Thanks.