add color support to separator block

[senadir]

tackles a point from #16483

it adds color support to the Separator block.

worth mentioning, the color won't be honored if the theme already defined a color, this is the case for the Dots, not the line, I can extend it to honor the theme color in the case of the line HR but it will require printing direct CSS or that the theme style is defined it using !important.

so, this is what is happening

• line HR color is overriding the theme style (unless the theme uses !important)
• the Dots are respecting the theme style (since they use currentColor)

you will see this case in 2019 in which they define a color for the dots

my suggestion is to unify the experience, but this will cause a slight problem.

the Dots are defined using :before, in order to unify the experience, we have to do this inside React rather then using :before but it will break compantility with anything that is already targeting the :before, like 2019

[youknowriad]

you will see this case in 2019 in which they define a color for the dots

@kjellr Do you think this should be fixed in 2019 or maybe force the styling using !important?

[kjellr]

Do you think this should be fixed in 2019 or maybe force the styling using !important?

Twenty Nineteen is likely not the only theme doing this. Since Gutenberg was providing a color here, I'm sure other themes are overriding it too. Even so, I'd prefer not using !important — the "right" way to fix this would be in Twenty Nineteen (and any other themes that do this).

So yes, we should open a Trac ticket for Twenty Nineteen once this PR is merged in.

[youknowriad]

Thanks @kjellr It seems that this PR requires a dev note.

[youknowriad]

Code-wise this PR is in a good state, can we get some design thoughts.

[kjellr]

Hey @senadir! Thanks for the PR.

I think this is missing an update to separator/theme.scss in order to get his working correctly on the front end. By default, that provides a light gray bottom border to themes that include theme support for wp-block-styles. I'm seeing that gray show up alongside the specified color on the front end for the Gutenberg Starter theme:

Editor:

Front end:

[mapk]

I just tested this and it's looking good. Thanks for creating the PR @senadir!

✅ The colors show in the style variations beautifully.

---

I also experienced the oddity on the frontend that @kjellr pointed out. Here it is zoomed in.

[senadir]

native behavior of HR is to provide color via border-color, since we don't have color classes for borders, I'm using color & background (Dots & line respectively).
the theme.scss is also defining a border so I'm deleting it.

frontend

editor

[kjellr]

the theme.scss is also defining a border so I'm deleting it.

I don't think this works as intended — we do want to provide a default front-end color for themes that opt in to wp-block-styles. We only need to delete/override this border when a background is applied. Otherwise, it clears the default color entirely, meaning that the "Default" and "Wide Line" variations are invisible by default, (unless a theme provides styles for them).

Here's what I see testing these latest changes in the Gutenberg Starter Theme (which provides no editor styles, and just uses wp-block-styles to pull in separator styles on the front end):

master in front end and the editor:

This branch, in front end and the editor:

As you can see, the first two separators are invisible now. If I specify a color for each one in the editor, they'll show up again:

Additionally, it looks like this PR shortens the height of the Default (short) separator style to 1px. I tend to prefer the previous 2px height, and I think we should stick to that unless we have solid reasoning for changing it.

Thank you!

[senadir]

this commit should fix this, changes are not scoped to the style
(Gutenberg starter theme/master frontend)

(2019 frontend)

(2019 editor style is still inconsistent between the frontend & the preview because it uses height for the frontend & border for the editor)

@kjellr Gutenberg starter theme behavior seems the most optimal, but I fear 2019 behavior is common
do you think we should force remove the border inside the editor for cases like 2019? or leave it for theme owners to fix it?

[kjellr]

I left a quick inline note about one of the updates. Other than that, this is looking pretty good.

I'm really hesitant to resort to !important rules here, so I'm still thinking the best way to do this would be to fix that Twenty Nineteen problem in the theme itself. We'll also add a dev note when this is merged, so that theme authors know to check their themes.

I'm definitely happy to hear perspective from other theme authors though!

[senadir]

@kjellr do you think this is ready to merge now?

[kjellr]

👍 This is good to go. I have a patch ready for Twenty Nineteen, and will link back here once I open the Trac ticket.

[kjellr]

The trac ticket for Twenty Nineteen is available here: https://core.trac.wordpress.org/ticket/47811

[aduth]

I see there was some discussion about the use of !important here in the pull request comments, and separately a fix addressing a TwentyTwenty-specific incompatibility at Trac#47811.

Which leaves the question: Do we actually need it? Can it be removed?

See also: #19539

Alternatively, I wonder:

• Only including this specific style rule if .has-text-color and/or .has-background, representative of the fact that the default should only need to be reset when the user chooses to apply a custom color (correct me if I'm mistaken)
• Can it be addressed by assuring that the specificity of this selector would always "defeat" a theme's styling? Especially considering we would probably want this reset only in the case of applying when a custom color is used, i.e. adding .has-text-color and/or .has-background as additional modifier classes:

.wp-block-separator.is-style-dots.has-background,
.wp-block-separator.is-style-dots.has-text-color {
	background: none;
}

[joyously]

What if a different method for generating dots was used?
The current method is a lot of CSS, that is a hassle for themes to override. I was going for the minimal style of making dots with multiple background images using radial gradients, but the current style using !important wouldn't let me.
I also couldn't use a dotted border because all those other styles I would have to override.

So what if the core style is the radial gradient instead of an important no background?

[mtias]

It'd be worth exploring a simpler approach.