Add configurable header styles to Blank Canvas Blocks #3439
Conversation
There was a problem hiding this comment.
This LGTM!
Just one comment that is more general — it seems we're leaning towards using the hyphenated CSS attribute name in our custom variables. When it's generated by theme.json / global styles mechanism proper, though, camel case is used.
Do we want to make these consistent, or is it better to leave them hyphenated so we know which variables we're creating via the "custom" section?
Good point! I think we should use camel case for consistency, we are already adding the |
| } | ||
| }, | ||
| "core/heading/h2": { | ||
| "typography": { | ||
| "fontSize": "var(--wp--preset--font-size--huge)", | ||
| "lineHeight": "var(--wp--custom--line-height--headings)" | ||
| "fontSize": "32px", |
There was a problem hiding this comment.
why are we not using variables for h1 and h2 like we are doing for the rest?
There was a problem hiding this comment.
Because those sizes exceeded what was configured as font size. I wasn't sure if it was a good idea to add two more sizes, which seem more focused on paragraph styles, just to style the larger header elements.
There was a problem hiding this comment.
I lean towards not adding more variables for headings, since we have the ability to change it directly in the global styles supported style properties of heading blocks.
Unless these font sizes would be used again outside the H1 + H2. Now that I think about it, they might for a block like site title... Hmm.
There was a problem hiding this comment.
That's more of a decision at the "theme" level then the "polyfill" level and those additional variables certainly could be added by that theme (or this one if we decide to style the block title in that way).
Don't code it 'till ya need it.
|
Good point y'all. Refactored for consistency. |
|
Once merge conflicts are resolved this LGTM! Thanks for the follow up on consistency in the key names. |
pbking
force-pushed
the
add/bcb-header-styles
branch
from
74c2971 to
a253021
Compare
Part of #3413 (for headings)
Added a custom headings configuration block including font-weight, -height, and -family and set the values to what they are in Blank Canvas.
Mapped font-family and font-weight to those properties to all h elements (h1-h6) in scss.
Refactored 'line-height' out of 'custom'
Set the header font size values in theme.json to the same size values in Blank Canvas
Before:
After:
