Skip to content
This repository has been archived by the owner on Feb 17, 2025. It is now read-only.

Skatepark: Add space below the Featured Image on the single post page #5061

Merged
merged 1 commit into from
Nov 24, 2021
Merged

Skatepark: Add space below the Featured Image on the single post page #5061

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion skatepark/block-templates/single.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

<!-- wp:group {"align":"full"} -->
<div class="wp-block-group alignfull">
<!-- wp:post-featured-image {"align":"full"} /--></div>
<!-- wp:post-featured-image {"align":"full", "style":{"spacing":{"margin":{"bottom":"calc(2*var(--wp--style--block-gap) + 3*var(--wp--custom--gap--horizontal))"}}}} /--></div>
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we should be using var() or calc() in block markup; It doesn't work in the FSE when you load that block template. It makes the block markup more verbose, but I'm of the opinion that we should be verbose there, even if it means more manual changes when things like "block gap" needs to change.

Copy link
Contributor

Choose a reason for hiding this comment

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

About this, looks like there's a proposal so that those values are not empty on the block sidebar

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@MaggieCabrera Perhaps this PR could be moved to Approved but not mergable?

Copy link
Contributor

Choose a reason for hiding this comment

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

no, I think we can merge this anyway, since only the UI is affected by the future change and not the actual looks of the site

<!-- /wp:group -->

<!-- wp:post-content {"layout":{"inherit":true}} /-->
Expand Down