-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Social links: add block gap support #35236
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Worked as advertised for me.
Where a single control only is available, we show it until there are more to choose from.
Thanks for reviewing @glendaviesnz !! I updated so that the block spacing control is shown by default since it's the only dimensions control. |
@ramonjd I just retested and the block gap displays by default now, and everything still works as expected in editor and front end. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for opting in this one, too, @ramonjd!
This is testing nicely for me. (Tested in the editor, on the front end, in global styles, and at the block level in theme.json) 👍
Not a blocker (we can always look at it in a follow-up), but I was wondering if it's worth us opting-in for the vertical margins, too, like we did in #34630 so that if someone wants to adjust the vertical margin that also uses the blockGap CSS variable, they can make a manual adjustment?
Good idea. Happy to add it to this or a super quick follow up. cc @jasmussen as a FYI |
Thanks! I don't feel too strongly about it, so either way works fine, I think 👍 |
Definitely a followup if need be. I'm only starting up my brain this morning, so not yet seeing the full picture. However high level, the principle of the container block handling layout of child items is a good north star. It speaks in favor of using gap, as done in this PR, to space out social icons. Similarly, it is the motivating factor of the blockGap property also spacing out blocks vertically as has been done. The idea being that you'll almost always want to space things out globally and in one place. Will there then be a need to override that spacing on individual blocks? Very probably, but less urgently so, and if we do it, probably one of those non-default spacing controls. Could be good to add if/when we are further with the reused Layout or Spacing panel. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working great!
💯 |
Description
#33991 has made block gap support available 🎉 and #34493 had prepared the groundwork for this block support to be applied to the Social Links Block 🎉.
This PR enables block spacing support for the Social Links Block.
Resolves #34526
How has this been tested?
Enable block spacing support in your theme.json under "settings", e.g.,
Insert a Social Links Block and, in the Dimensions Panel in the controls sidebar, adjust the block spacing value.
Note: the block spacing control should be displayed by default, meaning that you shouldn't have to toggle the ToolsPanel control to show it.
Check that the block spacing value is applied in the editor and for the saved post on the frontend.
In your theme.json, add the following to the "styles" object:
Check that the default value of
--wp--style--block-gap
is2em
and that it is applied to the Social Links Block container.Screenshots
Types of changes
Adding block spacing control to the Social Links block.
Checklist:
*.native.js
files for terms that need renaming or removal).