Unset inherited backgrounds on social icons. #37940
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jasmussen
added
the
[Block] Social
|
Size Change: +37 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
This was referenced Jan 13, 2022
oandregal
reviewed
Jan 21, 2022
| @@ -6,6 +6,11 @@ | |||
| // Some themes give all <ul> default margin instead of padding. | |||
| margin-left: 0; | |||
|
|
|||
| // Unset background colors that can be inherited from Global Styles with extra specificity. | |||
| &.wp-block-social-links { | |||
| background: none; | |||
There was a problem hiding this comment.
Can we lower this to be just a single .wp-block-social-links selector like the rules above? I don't think that it makes a big difference tbh but it's nice in case anyone does want to target it directly.
There was a problem hiding this comment.
I'm fairly sure that when I tried, that didn't have enough specificity. Let me just verify that.
There was a problem hiding this comment.
On investigation, in the editor it gets specificity to override this rule:
.editor-styles-wrapper ol, .editor-styles-wrapper ul {
background-color: var(--wp--preset--color--vivid-green-cyan);
}
On the frontend, it need only override this rule:
ol, ul {
background-color: var(--wp--preset--color--vivid-green-cyan);
}
I can change it so we have a low-specificity reset in style.scss, and higher specificity in editor.scss, sound good?
As a separate or alternative, we could look at how to reduce how editor styles are output, perhaps :where(.editor-styles-wrapper) ul, etc.?
oandregal
approved these changes
Jan 21, 2022
jasmussen
force-pushed
the
try/unset-social-icons-inherited-bg
branch
from
00406fc to
14e4306
Compare
|
I rebased this one, and per feedback moved the extra specific rule to editor.scss only, so the frontend can maintain low specificity. Let me know if this is preferred over the single rule. Frontend: Editor:
In both of the above screenshots, a green background is inherited, but unset. |
|
This sounds good: a theme will need to wrap its styles with |
|
Cool, I'll land this, and I'll take a look at some adjacent Navigation block issues when I have a moment! |
|
This is great to have been merged. Thanks! However, I do think it's important to point out that the underlying issue also impacts every third-party block that uses list markup. For example, I will now need to apply this "patch" to my Social Sharing Block., which is a fork of the core Social Icons block. Not a huge deal, but important for block developers to be aware of. |
|
The original issue is intentionally not closed. As I've noted I personally think the underlying issue is sufficiently addressed, it'd be good to keep it open until we get a feel for this solution, and whether it lets us reach some consensus or not. It'll also let other ideas keep trickling in 👍 👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
If you set the List block to have a specific background color in global styles, the
ulblock is targetted, making that color bleed into the Social Icons block:As an alternative approach to #37528, this PR unsets any inherited background colors in the social icons block, stopping the cascade:
How has this been tested?
Change the background color of the List block in global styles. Then insert a Social Icons block, it should still have a transparent background.
Checklist:
*.native.jsfiles for terms that need renaming or removal).