Hm I'm a bit perplexed by the failed ColorPickerValueShouldUpdateOnText test. It seems that the _textState value is only updated when the internal _valueState is not null. So during the first pass only the null _valueState is updated to the input value. On subsequent updates, the _valueState is no longer null and _textState is updated accordingly.

During the test here shouldUpdateBinding is false because _valueState.Value is null. colorChanged is true so only await _valueState.SetValueAsync(newColor); runs.

The obvious fix is to have it also call SetTextAsync() when colorChanged is true but I'm not sure if this behavior is intentional.

Does anyone know if this there a reason for this behavior? Glancing around the rest of the project I don't see this pattern anywhere else.

Does anyone know if this there a reason for this behavior? Glancing around the rest of the project I don't see this pattern anywhere else.

Honestly, no. This is pretty damn old code from ~5 years ago, and it was hard to migrate it from the anti-pattern setter/getter logic in parameters #10357. I mainly kept it how it was before, with all its quirks.

I’m mostly waiting for this #12556 so I can give it a proper refactor and not have to deal with all the Text ↔ Value synchronization.

upd: The Text parameter makes no sense to me here either, since you can already supply a hex string to the Value and it will automatically convert it to MudColor (if it's one way, due to the implicit operator). It’s also very straightforward to convert a MudColor back to a string.
The only thing Text can do that Value can’t is keep an invalid color (for example #ZZZZZZ). Value can never accept invalid colors because that restriction exists at the MudColor level. As a result, in the future it will either reset to null or the last valid value, depending on whatever logic we choose. However, I don’t think this is worth the current level of complexity to support that in future.

Ah thanks for the clarification, looking forward to seeing Text removed.

I also found that removing the default color for Value did not fully fix support for null values. Much of the component still expected a value in _valueState.Value in order to drive the various color picker inputs. So for this PR, I've made some larger changes to the ColorPicker to match as closely as possible the old behavior as seen on the live docs site.

I've added a new private getter called ValueOrDefault, which returns the first non-null MudColor from either the current value, the last value, or the default value. This means that when the picker is cleared, the color selection spectrum will remain on the last used value, which mimics the old behavior (and I personally think makes sense). I've also configured the default value to match the color previously used to initialize Value.

All inputs internal to the color picker now use this getter instead of accessing _valueState.Value directly. Externally, if the _valueState.Value is null then Value and ReadValue will still return null as expected.

This is a bit larger of a PR than I initially expected, so please let me know if there are any changes you'd like me to make.

It would be nice if you add bUnit test from #11775

<MudForm @ref="_formRef">
    <MudColorPicker @bind-Text="_colorValue" Placeholder="Select Color"
                    Required="true"
                    Editable="true" />
</MudForm>

<MudButton Color="Color.Primary" OnClick="Submit">Ok</MudButton>

@code {
    private MudForm _formRef = null!;

    private string _colorValue;

    private Task Submit()
    {
        return _formRef.Validate();
    }
}

and check if form is red.

Okay I've added two new tests and made a couple more tweaks.

One test called FormWithColorPicker() (adapted from FormWithDatePicker()), which tests that setting the color via the Value parameter correctly triggers form validation.

The other called Form_Should_Validate_ColorPicker_When_EditableInputCleared() which should cover the bug reported in #11775.

I've also updated the test SetNullColor_NothingChanged() to expect a null value when clearing the input, as that is the new desired behavior. I changed the ValueOrDefault to use the last valid color instead of the _baseColor because the latter goes through some kind of color transformation and is not always equal to the last input value.

@danielchalmers double checking if we are ok with these change that the default is not null value (see screenshots)

I'll try to test and review this today or tomorrow

When you load the page, the preview is transparent and does not match the selection in the color field.

Before

After

@danielchalmers retest. I added a test as well, which fails before that fix

Throttle no longer applies (tested on both server and wasm)

Rest of the examples seemed fine