Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Seedlet blocks: fix broken footer #4360

Merged
merged 1 commit into from
Aug 19, 2021
Merged

Conversation

MaggieCabrera
Copy link
Contributor

The footer for Seedlet blocks was broken. This solution leverages flex layout on the group block.

Before:

Screenshot 2021-08-03 at 17 12 07

After:

Screenshot 2021-08-03 at 17 09 38

@MaggieCabrera MaggieCabrera self-assigned this Aug 3, 2021
@MaggieCabrera MaggieCabrera requested a review from a team August 3, 2021 15:15
Copy link
Contributor

@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.

This fixes the issue in my testing, thanks! I have one comment but I am not sure it's worth holding up the fix because it may require a deeper rethinking of alignments.

font-family: var(--wp--preset--font-family--headings);
white-space: pre-wrap;
&[class*="wp-container-"] {
margin-left: auto !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

It's unfortunate that we are fighting our own alignment and padding rules here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, I thought the same, and I don't think it's the first time I've butted heads with this particular code. I'll have to think about it.

margin-left: auto !important;
margin-right: auto !important;
padding: 0;
column-gap: 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would be nice to be able to specify this gap value from theme.json, because it's a bit arbitrary right now (0.5em for any flex layouts).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah, GB should provide a setting for this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Apparently it's planned: WordPress/gutenberg#33359

@MaggieCabrera
Copy link
Contributor Author

I'm holding on to this one because the gap control looks like it may make it soon and I still want to check if we can improve the Blockbase rules that we are overriding here

@pbking
Copy link
Contributor

pbking commented Aug 12, 2021

I think this is the change introducing gap that you are talking about: WordPress/gutenberg#33812

Linking so I don't have to go hunting for it again. :P

@MaggieCabrera
Copy link
Contributor Author

I kind of thought that it was going to make it sooner, we could deploy this as is and then iterate, right now this PR adds the gap via CSS, we can always come back to remove that and add the new markup

@MaggieCabrera MaggieCabrera merged commit 73e02b3 into trunk Aug 19, 2021
@MaggieCabrera MaggieCabrera deleted the seedlet-blocks-footer-fix branch August 19, 2021 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants