useColorField: Controlled version does not allow free input

[dbrxnds]

Provide a general summary of the issue here

When using the controlled version of useColorField it will jump to a value when you are not done typing yet. Strangely this does not seem to happen when you start without a #

🤔 Expected Behavior?

Allowing you to freely type

😯 Current Behavior

Examples:

• Type: #334 -> It will jump to #333344
• Type: #125 -> it will jump to #112255 Doing either of these without a # will allow you to keep typing

Similiarly, when there is a complete value and you only backspace the last character it will keep jumping to complete values.

💁 Possible Solution

No response

🔦 Context

We have a custom ColorPicker component that makes use of useColorField together with useColorSlider and useColorArea to allow users to type as well as drag to the color they want. This requires us to keep the state synced between the 3 parts.

🖥️ Steps to Reproduce

https://react-spectrum.adobe.com/react-aria/useColorField.html#controlled

Version

3.0.0-beta.27

What browsers are you seeing the problem on?

Firefox, Chrome

If other, please specify.

No response

What operating system are you using?

MacOS Sonoma, Linux Mint, Windows 11

🧢 Your Company/Team

No response

🕷 Tracking Issue

No response

[LFDanLu]

Thanks for catching this, looks like we actually should be omitting onChange and value from the useColorField hook since useColorFieldState will manage these for us: https://codesandbox.io/s/stoic-feather-qtw7hh?file=/src/App.js. @snowystinger do you remember any particular reasons why we still have those props supported in the useColorField hook?

EDIT: Theoretically we should be overwriting the props.onChange here instead of merging it with the onChange defined in the hook

[dbrxnds]

Thanks for the response @LFDanLu, I only noticed just now. Is there a workaround for now that you know of?

[LFDanLu]

Yep, for your own version of the ColorField, simply don't pass value and onChange to the useColorField hooks since useColorFieldState will handle that for you, this sandbox here demonstrates the setup: https://codesandbox.io/s/stoic-feather-qtw7hh?file=/src/App.js

[dbrxnds]

Ah, my head was a little elsewhere when I read your first comment. This works, thank you! @LFDanLu

[sookmax]

@LFDanLu @snowystinger Guys, so do we want to remove value and onChange from AriaColorFieldProps, or do we keep the props and silently overwrite them (if passed) with the internal value and onChange in useColorField?

[LFDanLu]

@sookmax IMO we should remove value and onChange from those types since it maybe confusing to show they are supported props but then not actually do anything. I guess we could potentially get rid of value only and update it to the hooks to also call the user provided onChange so that users can still have a way to action when the field changes.

Open to opinions, bit of a special case here.

[sookmax]

@LFDanLu Thanks for the comment!

Is it possible to use useColorField without accompanying useColorFieldState? I'm asking this because if useColorField and useColorFieldState are meant to be used together all the time, then wouldn't it be a little confusing for users where to pass onChange to (if both accept onChange simultaneously)?

[LFDanLu]

It is technically possible to use useColorField and provide your own version of a ColorFieldState to it but the two were designed to be used in tandem. I agree it can be confusing, we hoped users would simply pass all their props to both hooks and everything would work as is (this confusion with gluing the hooks together has been a big factor in the creation of the React Aria Components).

[sookmax]

@LFDanLu I see. Thanks for the explanation!

I guess we could potentially get rid of value only and update it to the hooks to also call the user provided onChange so that users can still have a way to action when the field changes.

But yet again, it still feels a little redundant to allow the user to pass onChange to useColorField since:

1. the user can always provide onChange to useColorFieldState if they choose to use useColorField with useColorFieldState.
2. the user is the one responsible for providing state.setInputValue()—which is called inside the internal onChange in useColorField—if they choose to provide their own state implementation.

Maybe I'm missing something here, but I don't really see the use case of passing onChange to useColorField.

[snowystinger]

@LFDanLu's assessment is correct, we shouldn't be merging the props like we currently are in the link. Instead, it should follow how NumberField works.

ColorField is a beta component, along with its hooks, it probably just hasn't had as much scrutiny as other components, which is how it slipped through.

As for the types, the original goal was, as @LFDanLu says, that all props could be passed "as-is" to our hooks, instead of needing to pull out certain ones, requiring more knowledge of individual hooks. If we omit the value and onChange, it would affect every component which is an input. It would be the same change in NumberField, Switch, Checkbox, and RadioGroup, to name a few. Meanwhile, TextField is only an aria hook, there is no stately (for "historical reasons"), so that'd be the odd one out.

All that to say, I think it's ok for the hooks to accept all the props, though I can see why it'd be confusing. The hook requires the state object, and the state object must include setInputValue.

[cssinate]

I just wanted to add an additional strange behavior to the list on the original post. If you look at https://react-spectrum.adobe.com/react-aria/useColorField.html#controlled and type in a 5-character string and then blur, the printed value shows the bad string, but the field shows the last known good string. I just thought this might be an extra thing to test for whenever somebody is looking at the fix for this.

[cssinate]

I want to add two more things to this discussion:

1. It may be worth it to add a prop indicating whether or not transparent values are acceptable
2. Where useColorField is providing isInvalid, validationErrors, and validationDetails, I might recommend something like (this is a bad name) lastKnownGoodColor. If a user types in 5 characters like faceee, then this gives form validation an opportunity to say "I'll use the last known valid value which is face (#FFAACCEE)" (or fac (#FFAACC) if 1 is introduced and allowTransparentColors is false)