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

Social links: add block gap support #35236

Merged
merged 2 commits into from
Oct 5, 2021
Merged

Conversation

ramonjd
Copy link
Member

@ramonjd ramonjd commented Sep 30, 2021

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

		"spacing": {
			"blockGap": true,
                         ...

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:

		"blocks": {
			"core/social-links": {
				"spacing": {
					"blockGap": "2em"
				}
			}
		}

Check that the default value of --wp--style--block-gap is 2em and that it is applied to the Social Links Block container.

Screenshots

kitty-spacing

Types of changes

Adding block spacing control to the Social Links block.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • I've tested my changes with keyboard and screen readers.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

@ramonjd ramonjd added the [Block] Social Affects the Social Block - used to display Social Media accounts label Sep 30, 2021
@ramonjd ramonjd requested a review from mkaz as a code owner September 30, 2021 05:51
@ramonjd ramonjd self-assigned this Sep 30, 2021
Copy link
Contributor

@glendaviesnz glendaviesnz left a 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.
@ramonjd
Copy link
Member Author

ramonjd commented Oct 4, 2021

Thanks for reviewing @glendaviesnz !!

I updated so that the block spacing control is shown by default since it's the only dimensions control.

@glendaviesnz
Copy link
Contributor

@ramonjd I just retested and the block gap displays by default now, and everything still works as expected in editor and front end.

Copy link
Contributor

@andrewserong andrewserong left a 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?

image

@ramonjd
Copy link
Member Author

ramonjd commented Oct 5, 2021

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

@andrewserong
Copy link
Contributor

Happy to add it to this or a super quick follow up

Thanks! I don't feel too strongly about it, so either way works fine, I think 👍

@jasmussen
Copy link
Contributor

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.

@apeatling apeatling self-requested a review October 5, 2021 20:29
Copy link
Contributor

@apeatling apeatling left a comment

Choose a reason for hiding this comment

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

Working great!

@ramonjd ramonjd merged commit 67eab29 into trunk Oct 5, 2021
@ramonjd ramonjd deleted the add/social-links-block-gap-support branch October 5, 2021 23:30
@github-actions github-actions bot added this to the Gutenberg 11.7 milestone Oct 5, 2021
@jasmussen
Copy link
Contributor

💯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Social Affects the Social Block - used to display Social Media accounts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Social Icons: Support gap via theme.json
5 participants