Update the .is-dark-theme UI for Quote and Pullquote blocks #26510
Conversation
|
Size Change: +92 B (0%) Total Size: 1.2 MB
ℹ️ View Unchanged
|
|
The failed tests don't look like they are because of any CSS changes, so I'm going to merge this one. |
| @@ -4,12 +4,22 @@ | |||
| margin-bottom: 1.75em; | |||
| color: #555; | |||
|
|
|||
| .is-dark-theme & { | |||
| border-top: 4px solid $light-gray-placeholder; | |||
There was a problem hiding this comment.
Isn't this an internal UI variable? Shouldn't we be avoiding the use of those on front-end block styles?
There was a problem hiding this comment.
Good question. This is where the styles were defined for the editor, so adding this to account for the styles against a dark background worked here. I believe that themes need to opt-in for this though, right? Reading from: #16732
If a theme opts in, these are provided in the front-end as well, via the theme.css stylesheet.
There was a problem hiding this comment.
Yeah, themes need to opt-in to use this. I'm just not sure if this is the right approach. I mean, we're not using an SCSS variable for the non-dark-theme color, so why are we using one here? Or perhaps we should be using variables for both? It just seems inconsistent right now.
|
Something feels off to me in this PR as well. I thought the dark theme support was about adapting the UI to the theme and this PR adapts the actual block styles. Isn't this better done in the theme CSS itself? cc @jasmussen |
|
Yep this is off.
It is not meant for the frontend at all, and the
Another option is to do nothing — the visuals that style the Pullquote block to have light gray borders are stored in the |
|
To clarify, the theme author adds the following action to opt in to showing the UI that works on dark backgrounds: In my testing, doing this only outputs the
|
|
First, thanks for digging in!
My goal is to solve the issue with the citation placeholder text, and the borders which are both too difficult to see in the editor when using a dark theme. You can see in my screenshots in the description above. Doing nothing doesn't appear to be a good course of action here. I'd like to make sure these elements are visible while using a dark theme in the editor. I'm less concerned about the frontend because themes most likely style those elements for the frontend already. It's the editor that often suffers. I'm totally okay with leaving the difficult borders as being a styling element that requires the theme to address. But the citation placeholder text which is not readable should be fixed as that is part of the UI in the Editor which falls on us to fix. Shall we revert this change and target the citation placeholder text in another PR? If the |
|
Ah, that's helpful clarification.
The problem with this PR as it is, is that the frontend no longer matches the editor. The editor suggests your pullquote is going to have light borders and light text when you publish, but it doesn't. Same with the quote:
I created two alternate PRs that change things up so that the placeholder text is both legible, and the frontend and editor are visually in sync.
The confusion in this PR is probably compounded by the fact that it's a bit hard to understand what the features at play here do. It would be nice to revisit documentation around these to better explain what they do:
I'd personally prefer we move forward with #26684, as although it can be disruptive to themes that opt into opinionated editor styles, I suspect that's fewer and fewer. |
+1 |
Fixes #16732.
Updates the UI on Quote and Pullquote blocks for dark themes. This includes the borders and citations.
How has this been tested?
Tested locally.
Screenshots
Quote block before
Quote block after
Pullquote block before
Pullquote block after
Types of changes
CSS changes.
Checklist: