Allow three character hex color definitions in Olivero settings

Hi @Eli-T,

I have attached the patch to have the 3 character hex-color. Please verify and add comments.

Hi @deborahblessy,

Thanks for helping with the issue.

There is already an issue fork and merge request in this issue that would conflict with the changes you propose. If you would like to contribute to this issue I would suggest you co-ordinate with @andy-blum in the #Olivero channel in Drupal slack to avoid working at cross purposes.

@deborahblessy

As Eli said, thank you for contributing!

I took a quick look at your patch and wanted point out that while {3,6} does match the 3-character syntax for hex codes, the regex as you've written it will also match to invalid 4- and 5-character strings. Additionally, the HTMLColorInput that we create in color-picker.js will only accept a full 6-character string, so we have to also flesh out any 3-character strings before we try to set that element's value.

Since your patch passes the tests but would cause problems in real situations, we probably have a need for some additional tests. I'll open a followup issue for those.

Found a bug:

if you manually input 0df into the field, it thinks it works. The JS looks good, etc. But when you navigate to the homescreen the color is all black

One nit, one question 🤓

I agree with @andy-blum, that we should follow the same regex in the config here as there is in the config.

LGTM! 🚀

Hiding the patch file that was posted incorrectly. Thanks!

Added a maintainability suggestion to the MR.

Well, I guess the plan is to use the config validation instead of having it defined in the validation method? So this will end up being the canonical source in the future.

Agreed with @borisson_: the regex will eventually be moved out of PHP/form logic and into *.schema.yml.

Scratch "eventually": very soon actually: his issue was opened specifically because it blocks 📌 [PP-1] Add validation constraints to olivero.settings Postponed which was doing exactly that!

Once this lands, we can update #3395555 and update it to use #config_target (which only became usable after that MR was originally created, see the CR at https://www.drupal.org/node/3373502 → ).

To me this is borderline for backport; on one hand it's tagged as feature request but on the other you could perhaps consider it a bug. For now committing to 11.x (10.3.x) only, if anyone feels strongly that it should be backported then feel free to reopen.

Committed 441307e and pushed to 11.x. Thanks!

Automatically closed - issue fixed for 2 weeks with no activity.