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

Fix: Heading and paragraph colors not applied inside the cover block #17728

Conversation

jorgefilipecosta
Copy link
Member

Description

Fixes: #17706

The cover block had a rule to make paragraphs and headings inherit the color. This rule had higher specificity then what most themes use in their color classes so it made the colors selected by users not applicable inside the cover block.

This PR fixes the problem by not applying color rules if the class has-text-color is present.
Meanwhile, I discovered that the heading block is not adding this class when it should. I'm creating a parallel PR to address this issue.

How has this been tested?

I used the 2020 theme.
I added a cover block.
I added a paragraph inside it.
I selected the color "accent" in the color palette (first one).
I checked the post on the front end and verified the color was applied as expected (in master the color was white).

@jorgefilipecosta jorgefilipecosta added [Type] Bug An existing feature does not function as intended [Block] Cover Affects the Cover Block - used to display content laid over a background image labels Oct 2, 2019
@@ -130,7 +134,9 @@
h5,
h6,
.wp-block-subhead {
color: inherit;
&:not(.has-text-color) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really important, but shouldn't this just be .wp-block-subhead:not(.has-text-color)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so we also want to have selector p:not(.has-text-color) and h1:not(.has-text-color) etc... Having the selectors nested in this way ensures we apply the rule to all the selectors (not just .wp-block-subhead) that don't have a text color.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, whoops. I misread the SCSS. 😛

@jorgefilipecosta jorgefilipecosta force-pushed the fix/heading-and-paragraph-colors-not-applied-inside-cover branch from 31c48e2 to 3a51e18 Compare October 3, 2019 09:46
@jorgefilipecosta jorgefilipecosta force-pushed the fix/heading-and-paragraph-colors-not-applied-inside-cover branch from 3a51e18 to bcf23ad Compare October 3, 2019 15:49

h2:not(.has-text-color),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to this #17704, why do we need to style h2? I guess this is not needed anymore right? (all the h2 references in this file). For example, you'll notice that it's impossible to "left align" and h2 inside a Cover.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, @youknowriad, I answered this subject at #17704 (comment).
I think we should address it in a different PR.

@jorgefilipecosta jorgefilipecosta force-pushed the fix/heading-and-paragraph-colors-not-applied-inside-cover branch from bcf23ad to 20003cc Compare October 4, 2019 13:13
@jorgefilipecosta jorgefilipecosta merged commit 3d7e813 into master Oct 4, 2019
@jorgefilipecosta jorgefilipecosta deleted the fix/heading-and-paragraph-colors-not-applied-inside-cover branch October 4, 2019 13:51
@youknowriad youknowriad added this to the Gutenberg 6.7 milestone Oct 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cover Block: Custom Text Color not working
3 participants