Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Digits in custom color slugs #19285

Open
vg-robin opened this issue Dec 20, 2019 · 2 comments
Open

Digits in custom color slugs #19285

vg-robin opened this issue Dec 20, 2019 · 2 comments
Labels
[Type] Developer Documentation Documentation for developers

Comments

@vg-robin
Copy link

vg-robin commented Dec 20, 2019

Custom colors are defined with slug, color, and name properties. These are registered via add_theme_support('editor-color-palette', $cols).
When dynamically creating class names, getColorClassName() uses Lodash's kebabCase() function. If a color's slug includes a digit, such as "grey1", kebabCase() adds a hyphen before it, to become "grey-1", meaning the slug is different in server-side and client-side contexts.

Steps to reproduce the behavior:

  1. In PHP, do something like:
$col = ['slug'=>'grey1', 'name'=>'Grey 1', 'color'=>'#555555'];
add_theme_support( 'editor-color-palette', [$col]);
  1. Do something server-side with that color. Eg. in my case, colors are defined in the child theme, and code in the parent theme runs through the map and dynamically creates style declarations like:
$css[] = 'body .has-'.$col['slug'].'-color { color: '.$col['color'].'; }';

Similar could be used in dynamic block rendering.
3. Use the "Grey 1" color in the block editor. This is sent through getColorClassName() so will end up with the class "has-grey-1-color".
4. The styles won't match.

Not sure what the best solution is:

  1. generate an error when registering the color in the first place (a breaking change)
  2. use an exact same kebabCase process server-side, so the slug stored server-side is correct
  3. use an alternative to kebabCase when creating the slug. Ideally I think the slug should just be validated as a valid class name, and - if it is - not do any processing at all.
  4. Or just a note in the "Theme Support" doc page. It does mention the kabab-casing, but I think the number thing should at least be mentioned there?
    I suppose one could make the same argument about underscores, which would get changed to hyphens, but that's a bit more expected I think.

I know it seems like an edge case, but it's also very easy to miss, and in a fairly fundamental area for theme developers, so if there's an easy fix...

lodash/lodash#2843

@talldan talldan added [Type] Developer Documentation Documentation for developers Good First Issue An issue that's suitable for someone looking to contribute for the first time labels Feb 4, 2020
@talldan
Copy link
Contributor

talldan commented Feb 4, 2020

Thanks for creating this issue @vg-robin.

I could imagine any change to the way classnames are generated would break backwards compatibility for any color classnames already in use in plugins or core.

A documentation update sounds like a reasonable option. What do you think? I've labelled the issue as a documentation issue in the meantime.

@oandregal oandregal removed the Good First Issue An issue that's suitable for someone looking to contribute for the first time label Jun 4, 2020
@mburridge
Copy link
Contributor

Does anything need doing here? If so, what?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Developer Documentation Documentation for developers
Projects
None yet
Development

No branches or pull requests

4 participants