Skip to content
This repository has been archived by the owner on Jan 26, 2022. It is now read-only.

Try a static spacer underneath the header #327

Merged
merged 3 commits into from
Jan 10, 2022
Merged

Conversation

kjellr
Copy link
Collaborator

@kjellr kjellr commented Jan 10, 2022

Alternative to #325 and #319.
Closes #272.

This PR adds a static spacer block under the header to replace the unsupported margin value that was there previously. Ideally, this should be a dynamic value, but this is the best we can do for the moment since that's not landing in 5.9.

Using a static value produces more space than intended on mobile, but overall I think the usability enhancement makes it worthwhile for the initial release. This also makes for a more efficient transition to #325, once dynamic spacer units will be supported in a future minor release. I (somewhat regretfully) think this is the best path forward for the moment.


Editor:

Screen Shot 2022-01-10 at 11 36 08 AM

Frontend:

(Front-end desktop views are identical to trunk.)

Before After
Screen Shot 2022-01-10 at 11 33 34 AM Screen Shot 2022-01-10 at 11 33 45 AM
Before After
Screen Shot 2022-01-10 at 11 33 20 AM Screen Shot 2022-01-10 at 11 29 13 AM

@kjellr kjellr added the [Type] Bug Something isn't working label Jan 10, 2022
@kjellr kjellr requested a review from jffng January 10, 2022 16:39
@kjellr kjellr self-assigned this Jan 10, 2022
@kjellr kjellr added this to the RC 2 milestone Jan 10, 2022
<div class="wp-block-group"><!-- wp:group {"align":"full","style":{"elements":{"link":{"color":{"text":"var:preset|color|background"}}},"spacing":{"padding":{"top":"0px","bottom":"0px"},"margin":{"bottom":"var(--wp--custom--spacing--large, 8rem)"}}},"backgroundColor":"foreground","textColor":"background","layout":{"inherit":true}} -->
<div class="wp-block-group alignfull has-background-color has-foreground-background-color has-text-color has-background has-link-color" style="margin-bottom:var(--wp--custom--spacing--large, 8rem);padding-top:0px;padding-bottom:0px"><!-- wp:template-part {"slug":"header","tagName":"header","align":"wide"} /-->
<div class="wp-block-group"><!-- wp:group {"align":"full","style":{"elements":{"link":{"color":{"text":"var:preset|color|background"}}},"spacing":{"padding":{"top":"0px","bottom":"var(--wp--custom--spacing--large, 8rem)"}}},"backgroundColor":"foreground","textColor":"background","layout":{"inherit":true}} -->
<div class="wp-block-group alignfull has-background-color has-foreground-background-color has-text-color has-background has-link-color" style="padding-top:0px;padding-bottom:var(--wp--custom--spacing--large, 8rem)"><!-- wp:template-part {"slug":"header","tagName":"header","align":"wide"} /-->
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You'll notice that I'm adding some bottom padding here. The bird image is supposed to have some padding underneath it, but it was removed erroneously at some point. 🤷‍♂️

The padding is still present in the pattern, as it should be. So I'm just adding it back in to the template too.

Before After
Screen Shot 2022-01-10 at 11 33 20 AM Screen Shot 2022-01-10 at 11 29 13 AM

Copy link
Collaborator

@jffng jffng left a comment

Choose a reason for hiding this comment

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

I agree the spacer improves the usability, so I also prefer this approach to #325.

LGTM.

@kjellr kjellr merged commit f0346e1 into trunk Jan 10, 2022
@kjellr kjellr deleted the use/spacer-under-the-header branch January 10, 2022 16:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
[Type] Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Empty gaps seen in the site editor.
2 participants