Skip to content
This repository has been archived by the owner on Nov 18, 2024. It is now read-only.

Remove the top margin from the footer template part #572

Closed
wants to merge 1 commit into from

Conversation

carolinan
Copy link
Contributor

Description
The purpose of this change is to remove the spacing between the footer template part and full width, color patterns placed above it,

Partial for #404

Screenshots
Before:
370705205-b17efa4d-f1bc-4019-b78a-1740ffe54b64

After
Partial screenshot just to show that the gap is gone:
image
(The size difference here is not related to the PR)

Testing Instructions
Create a new page.
Select "Landing page for Book".
Confirm that there is no gap between the footer and the last pattern.

Go to Appearance > Editor > Templates and select the Home template.
In the Design panel, select the News blog with featured posts grid, and save.
Confirm that there is no gap between the footer and the last pattern.

Copy link

github-actions bot commented Oct 14, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: carolinan <[email protected]>
Co-authored-by: juanfra <[email protected]>
Co-authored-by: beafialho <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link

Preview changes

You can preview these changes by following the link below:

I will update this comment with the latest preview links as you push more changes to this PR.
⚠️ Note: The preview sites are created using WordPress Playground. You can add content, edit settings, and test the themes as you would on a real site, but please note that changes are not saved between sessions.

Copy link
Member

@juanfra juanfra left a comment

Choose a reason for hiding this comment

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

Thanks Carolina.

I still see a gap for the first scenario.

Screen.Recording.2024-10-14.at.16.47.11.mov

For the second scenario, it works well with that particular home template, but with others it doesn't. Not sure if we want to address that in a separate PR

Screen.Recording.2024-10-14.at.16.49.12.mov

@carolinan
Copy link
Contributor Author

carolinan commented Oct 14, 2024

Can we remove the newsletter-signup pattern from this landing page instead? Is it not strange to have two signup buttons this close?

Edit:
Sorry for being unclear, I meant removing it from the "News blog with featured posts grid" pattern, not the landing page patterns. The landing page patterns only shows two "Subscribe" buttons if the user also updates the footer.

@juanfra
Copy link
Member

juanfra commented Oct 14, 2024

Can we remove the newsletter-signup pattern from this landing page instead? Is it not strange to have two signup buttons this close?

I wouldn't be opposed because it would solve this issue. I'll defer to @beafialho as she may want to keep the design as is for that section.

@beafialho
Copy link
Contributor

Can we remove the newsletter-signup pattern from this landing page instead? Is it not strange to have two signup buttons this close?

The original design of this template shows a different footer, so I suggest using the intended footer.

News blog with featured posts grid

@carolinan
Copy link
Contributor Author

I will create a new part for the footer-columns pattern. Can you check if there are any other templates that have the wrong footer?

@beafialho
Copy link
Contributor

I will create a new part for the footer-columns pattern. Can you check if there are any other templates that have the wrong footer?

Yes, from what I'm seeing the only other template that should also use this footer is "News blog with sidebar".

@carolinan
Copy link
Contributor Author

@beafialho I just want to make sure: The News blog with sidebar should be updated to use the footer with columns?

@beafialho
Copy link
Contributor

yes @carolinan, "News blog with sidebar" according to the design, should use the "Footer with columns". For clarification this is the intended appearance:

There's one thing I'd like clarification on, though: can we/do we want to set the headers and footers used in the Figma designs to each template? If that's the case, I can open up an issue to address the ones in News blog templates, because some of them are not using the headers and footers specified in the design.

News blog with sidebar

@carolinan
Copy link
Contributor Author

We can, I think we have time to do this before Monday evening (RC1)

@beafialho
Copy link
Contributor

We can, I think we have time to do this before Monday evening (RC1)

Thank you for clarifying. None of this is critical, it is fine-tuning. I have opened an issue here.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants