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

Cover Block: Unnecessary default styles for H2 heading #17704

Closed
CreativeDive opened this issue Oct 2, 2019 · 6 comments · Fixed by #17815
Closed

Cover Block: Unnecessary default styles for H2 heading #17704

CreativeDive opened this issue Oct 2, 2019 · 6 comments · Fixed by #17815
Assignees
Labels
[Block] Cover Affects the Cover Block - used to display content laid over a background image Needs Testing Needs further testing to be confirmed. [Status] In Progress Tracking issues with work in progress

Comments

@CreativeDive
Copy link
Contributor

Hey,

the cover block comes with the following unnecessary default styles:

.wp-block-cover-image .wp-block-cover-image-text, .wp-block-cover-image .wp-block-cover-text, .wp-block-cover-image h2, .wp-block-cover .wp-block-cover-image-text, .wp-block-cover .wp-block-cover-text, .wp-block-cover h2 { color: #fff; font-size: 2em; line-height: 1.25; z-index: 1; margin-bottom: 0; max-width: 610px; padding: 14px; text-align: center; }

This default styles are only matched to h2 but not h1, h3, h4, h5, h6. I'm interested why the h2 needs a default style?

  1. This is very inconsistent.
  2. The default h2 style from cover block breaks the theme style for h2 headings and override it.

Please remove this unnecessary h2 default style from cover block. This formatting should be added by the theme only.

@youknowriad
Copy link
Contributor

I guess this is probably a leftover from the initial implementation of the Cover block (without nesting). is that right? cc @jorgefilipecosta

@youknowriad youknowriad added [Block] Cover Affects the Cover Block - used to display content laid over a background image Needs Testing Needs further testing to be confirmed. labels Oct 2, 2019
@CreativeDive
Copy link
Contributor Author

Bug 1: H2 Heading is centered, but not selected as centered.
Bug 2: Color is white, but not defined as white.
Bug 3: Margin bottom overrides H2 styles from a theme.
Bug 4: Font Size overrides H2 styles from a theme.
Bug 5: line Height overrides H2 styles from a theme.

@jorgefilipecosta
Copy link
Member

jorgefilipecosta commented Oct 4, 2019

I guess this is probably a leftover from the initial implementation of the Cover block (without nesting). is that right?

Hi @youknowriad exactly these styles exist because of a previous version of a cover block. But they are not a leftover they were intentionally left because without them old cover blocks would not work.

Cover block suffered lots of changes I guess we need a better way to organize its styles and clearly indicate what is a deprecation and what is not. And we will need a way to relying on the previous markup still keep styles on working when in the presence of the old markup but not affect the new markup.
I will look into this issue and try to organize the styles in a better way.

@CreativeDive
Copy link
Contributor Author

CreativeDive commented Oct 4, 2019

@jorgefilipecosta I can not imagine that this old style can be organized in such a way that it does not always override the theme style. Supporting older versions unnecessarily inflates the code.

The theme comes with a style like:

h2 {
    font-size 20px;
    color: #000;
}

Any other CSS assignment like the following will override this theme style.

.xyz h2 {
    font-size 16px;
    color: #fff;
}

Is there a way? I think this old cover block h2 css code is useless :-)

@jorgefilipecosta
Copy link
Member

Hi @CreativeDive I created a PR that solves this issue:
#17815

@jorgefilipecosta I can not imagine that this old style can be organized in such a way that it does not always override the theme style

I relied on some differences between the old markup and the new.

Supporting older versions unnecessarily inflates the code.

Unfortunately, we can not remove CSS for older versions otherwise during an upgrade old blocks would stop working or their design would be changed.

@CreativeDive
Copy link
Contributor Author

@jorgefilipecosta Good job! I will test it after the release of Gutenberg 6.7.

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 Needs Testing Needs further testing to be confirmed. [Status] In Progress Tracking issues with work in progress
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants