Social Link: Add contentOnly editing support #66622
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ndiego
added
[Type] Enhancement
|
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 If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Size Change: +212 B (+0.01%) Total Size: 1.81 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in bdd8687. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/11601729247
|
24 tasks
talldan
reviewed
Nov 8, 2024
| }, | ||
| "service": { | ||
| "type": "string" | ||
| }, | ||
| "label": { | ||
| "type": "string" | ||
| "type": "string", | ||
| "role": "content" | ||
| }, | ||
| "rel": { | ||
| "type": "string" |
There was a problem hiding this comment.
So the button block has rel as role: content, but the image block doesn't, which is a bit of a quandry. I think it maybe makes sense for it to have a content role, as it's part of the url which is also content. If the user can edit a url, they may want to also set rel and openInNewTab, so those have to be contentOnly too.
On the social link block, the rel can't be edited in contentOnly mode (it's in the inspector > advanced area), so perhaps it should be a follow-up issue to make that work in addition to adding the role.
talldan
reviewed
Nov 8, 2024
| @@ -141,6 +146,41 @@ const SocialLinkEdit = ( { | |||
|
|
|||
| return ( | |||
| <> | |||
| { isContentOnlyMode && showLabels && ( | |||
There was a problem hiding this comment.
When using the block normally (not in contentOnly), it's possible to always edit the text in the inspector, and the text is used for screen readers, so I wonder if it's worth considering making it always show in content only mode too. 🤔
There was a problem hiding this comment.
Just came here to ask this too. It seems like a good follow up to allow editing both text and links.
Here's "Design" mode when I click on a link inside the the social links block:
And in Write mode, the block styles are shown for all links:
talldan
reviewed
Nov 8, 2024
Comment on lines
+166
to
+179
| <TextControl | ||
| __next40pxDefaultSize | ||
| __nextHasNoMarginBottom | ||
| className="wp-block-social-link__toolbar_content_text" | ||
| label={ __( 'Text' ) } | ||
| help={ __( | ||
| 'Provide a text label or use the default.' | ||
| ) } | ||
| value={ label } | ||
| onChange={ ( value ) => | ||
| setAttributes( { label: value } ) | ||
| } | ||
| placeholder={ socialLinkName } | ||
| /> |
There was a problem hiding this comment.
I'd consider it out of scope for this PR, but in usage it feels like the text should be editable inline (as RichText) when showLabels is active.
It probably becomes more obvious in contentOnly mode. Maybe there are reasons it wasn't done like that 😄
talldan
reviewed
Nov 8, 2024
There was a problem hiding this comment.
This works well and code looks good. The comments are mostly subjective, but I'd be curious for your thoughts on them.
For some things, it might be good to get design feedback, like the 'Text' dropdown on the toolbar when in contentOnly mode (I left a comment about whether it should always be shown).
@richtabor is usually interested in these things 😄
talldan
approved these changes
Nov 27, 2024
talldan
deleted the
add/content-only-editing-support-to-social-link-block
branch
michalczaplinski
pushed a commit
that referenced
this pull request
Dec 5, 2024
Co-authored-by: ndiego <[email protected]> Co-authored-by: talldan <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This PR adds
contentOnlyediting support for the Social Link block, similar to the way we currently handle the editing mode in the Image block.Why?
When content only editing is enabled via
"templateLock": "contentOnly"any Social Link blocks within the container cannot be edited by the user. There are scenarios where the user should be able to at least edit theurlandlabelattributes, especially when the block is distributed in a pattern that can be used in a variety of contexts.How?
This PR sets
"role": "content"on theurlandlabelattributes inblock.json. Doing so automatically enables the<URLPopover>component, so nothing more is needed for editing the icon URL.The icon Text, on the other hand, is only editable in the block inspector, which is completely disabled in
contentOnlymode. Therefore, this PR applies the same approach used in the Image block, where a button/popover is added to the block toolbar that allows the user to edit thelabelattribute. This "Text" button is conditional based on whether "Show text" has been enabled on the parent Social Links block.Testing Instructions
Screenshots or screencast
contentOnlysupport)