Global Styles: Fix per block text color customization for cover block #34188
Conversation
Block level global styling for cover block text colors were being overridden by default colors. This is because, for global styles, we were setting styles on the top-level cover block element. The default text color, however, was being set on the child cover block inner container. Since the text content's nearest ancestor was the inner container, it was inheriting the inner containers text color declarations.
jeyip
added
[Type] Bug
|
Size Change: +282 B (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Notes:
- Alternatively, rather than making the default text color styling less specific like we do in this PR, we could make specificity of the global styles selector more specific. The reason I opted for this approach was because it seemed like it had the smallest surface area of changes to make.
- As far as background colors go, there are two places where we can set them on the cover block -- the block settings panel, and the block-level global styling panel. When no selection is made in block settings, background colors chosen in block-level global stylings should take effect. Unfortunately, they don't, and it's another bug that I'm looking into.
I think going with a less specific selector here is a good call; generally keeping selector specificity low for base styles seems like a good practice. |
Strike that comment, I forgot you can insert a cover block without an image. |
|
Tested with TT1-Blocks and it works as expected! ✨ Although, I've noticed that the Cover's background color doesn't work as well, and I guess we might want to fix that here as well. When we add the block, we are kinda forced to choose a background color (or an image, but we can ignore that here). Though, it's possible to clear the block's background color, which becomes hardcoded to black: gutenberg/packages/block-library/src/cover/style.scss Lines 46 to 48 in 292d9f2 If I read the associated issue and PR correctly, it's related to the Opacity setting ( In other words, if the block has a This is good and reasonable UX, but... The block starts with I think the easy solution would be to just check for the image too here: - 'has-background-dim': dimRatio !== 0,
+ url && 'has-background-dim': dimRatio !== 0,In my test, that change fixes the background color issue with Global Styles. |
After looking at the related issues you linked, I think you're right @Copons. Inline comments about the purpose of |
After testing, I see one potential caveat:
In practice, the problem seems like an edge case. My intuition wants to say that it's negligible, but I'm curious if others have strong opinions. Either way, I still think Jacopo spotted a legitimate oversight in the implementation of the
|
jeyip
force-pushed
the
fix/per-block-global-styling-for-cover-block
branch
from
5bf5095
to
bef5402
Compare
Hmm...I also noticed that if no background image and no global styles are applied to the cover block, the background color defaults to white, whereas previously, it defaulted to black. This makes me more concerned about backwards compatibility. If users have cleared any cover block background color selections, when we implement these changes, their cover blocks will change from black to white. |
|
I added logic that addresses all of the aforementioned issues in c9e7375. The default, black background color style for the cover block was conflicting with block-level global styles. I introduced a |
There was a problem hiding this comment.
As per #34150 (comment) the fix is actually not allowing the user to edit colors in the global styles sidebar for the cover block.
Not super clear what is the issue (but no worries I'll wait for the fix to read more about it 😄 ). |
|
Sure! I'll tag you both as reviewers. |
|
Closing in favor of #34293 |
Partially fixes #34150
Description
Block level global styling for cover block text colors were being overridden by default colors. This was because, for global styles, we were setting styles on the top-level cover block element. The default text color, however, was being set on a child, (on the cover block inner container).
Since the text content's nearest ancestor was the inner container, it was inheriting the inner container's text color declarations instead of global styles.
How has this been tested?
blockbaseormayland-blocksScreenshots
Types of changes
Bug fix
Checklist:
*.native.jsfiles for terms that need renaming or removal).