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.

Sign up for GitHub

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 Link: Add border and spacing support #63782

Open
wants to merge 8 commits into
base: trunk
Choose a base branch
from
Open

Social Link: Add border and spacing support #63782

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f76ea75
Add border, color, and spacing support in social icon
akasunil Jul 21, 2024
0d025ce
update developer docs
akasunil Jul 21, 2024
f7eaf6a
Merge branch 'trunk' of personal.github.com:WordPress/gutenberg into …
akasunil Aug 9, 2024
789b27b
Remove spacing styles from social links block
akasunil Aug 10, 2024
0861737
Merge branch 'trunk' of personal.github.com:WordPress/gutenberg into …
akasunil Aug 13, 2024
b252dd1
Skip color support for social link block
akasunil Aug 13, 2024
c6aa0b3
Make border support optional
akasunil Aug 26, 2024
0516af5
Merge branch 'trunk' of personal.github.com:WordPress/gutenberg into …
akasunil Aug 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/reference-guides/core-blocks.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -865,7 +865,7 @@ Display an icon linking to a social profile or site. ([Source](https://github.co
- **Name:** core/social-link
- **Category:** widgets
- **Parent:** core/social-links
- **Supports:** interactivity (clientNavigation), ~~html~~, ~~reusable~~
- **Supports:** interactivity (clientNavigation), spacing (margin, padding, units), ~~html~~, ~~reusable~~
- **Attributes:** label, rel, service, url

## Social Icons
Expand Down
21 changes: 21 additions & 0 deletions packages/block-library/src/social-link/block.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,27 @@
"html": false,
"interactivity": {
"clientNavigation": true
},
"spacing": {
"margin": true,
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure how useful allowing margin support on individual links is. My understanding of the social links block is that spacing should be handled via the parent Social Links block's block spacing support.

"padding": true,
"units": [ "px", "em", "rem", "vh", "vw" ],
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need custom definitions of available spacing units here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess not.

"__experimentalDefaultControls": {
"margin": false,
"padding": false
}
},
"__experimentalBorder": {
"radius": true,
"color": true,
"width": true,
"style": true,
"__experimentalDefaultControls": {
"radius": true,
"color": true,
"width": true,
"style": true
}
Copy link
Contributor

Choose a reason for hiding this comment

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

If the dimension controls aren't displayed by default, the border controls shouldn't be either.

}
},
"editorStyle": "wp-block-social-link-editor"
Expand Down
2 changes: 0 additions & 2 deletions packages/block-library/src/social-links/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,6 @@
// This needs specificity because themes usually override it with things like .widget-area a.
.wp-block-social-links .wp-block-social-link.wp-social-link {
display: inline-block;
margin: 0;
padding: 0;
Comment on lines -94 to -95
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the thinking behind removing these resets?

My understanding was they should be tweaked to lower the specificity so global styles could be applied. Was there an unexpected issue when doing so?

Copy link
Member Author

Choose a reason for hiding this comment

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

Should we move these style to social link block? Global style would be able to override that style. Another solution is we can add selector in social link block for spacing styles.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this might need some further thought.

The Social Link block contains a clickable button or link depending on whether the user is in the editor or frontend.

It would make sense to me that any padding added for a link would be on that inner button or link so it is taken into account for the clickable area. The same could be said for the border too.

This would likely mean, leaving the styles here untouched and skipping serialization for the border and padding support. Then manually applying the support styles on the appropriate inner element.


.wp-block-social-link-anchor {
&,
Expand Down
Loading