Skatepark: add mixed media in container pattern #4403
Conversation
jffng
force-pushed
the
add/skatepark-mixed-media-container
branch
from
d4e5591 to
95871c7
Compare
| <!-- /wp:spacer --> | ||
|
|
||
| <!-- wp:image {"sizeSlug":"large","style":{"color":{"duotone":["#000","#BFF5A5"]}}} --> | ||
| <figure class="wp-block-image size-large"><img src="' . get_stylesheet_directory_uri() . '/assets/images/skateboard-sideways.jpg" " alt="' . esc_attr__( 'A skateboard laying on its side on top of concrete.', 'skatepark' ) . '"/></figure> |
There was a problem hiding this comment.
We should probably compress this.
| @@ -0,0 +1,6 @@ | |||
| .skatepark-mixed-media-in-container { | |||
| hr, p, h1, h2, h3, h4, h5, h6 { // Shrink default vertical margins for all the elements in this pattern | |||
There was a problem hiding this comment.
I'm not crazy about this but not sure of a better way to reduce and standardize the margins between headings, paragraphs, and separators for this pattern.
There was a problem hiding this comment.
I can't think of another way to target these elements... I think this is fine, especially as it's specific to this pattern.
There was a problem hiding this comment.
makes sense to me that we do it this way, since we are adding the margin globally from the theme css, it's not really the editor that's failing us here. In the end this is just an exception, right?
There was a problem hiding this comment.
Yeah they are exceptions. But if we reduce the vertical margins, I don't think these exceptions will be needed.
|
This is not specific to this PR but I guess we could fix it here. The text-indent block style for the paragraph is not looking nicely on mobile:
|
|
Should I hold off on reviewing this one until #4436 is merged, or is that already in this PR? |
jffng
force-pushed
the
add/skatepark-mixed-media-container
branch
from
5626fd4 to
085edb4
Compare
|
Sorry for the delay replying here. I don't think this should be held up by #4436, I think it can be reviewed from a design perspective.
It sounds like the way we handle the text indent is changing, so maybe let's leave the mobile styling for the PR that addresses it to avoid conflicts. |
|
The percents do scale nicely on mobile, though I don't know that I have a strong opinion on specifically using percents — my main concern is making sure it looks consistent across other patterns and blocks. Could we increase the spacing a tiny bit? Thinking: |
|
Thanks for the ✅ ! I compressed the image, so I'll bring this in. If it turns out we can remove the CSS that alters the vertical spacing, we'll do it in a follow up PR. |
Changes proposed in this Pull Request:
Adds the Mixed Media in Container block pattern:
Related issue(s):
Part of #4308