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 contentOnly editing support #66622

Merged
merged 1 commit into from
Nov 27, 2024
Merged

Social Link: Add contentOnly editing support #66622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 4 additions & 2 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 @@ -9,13 +9,15 @@
"textdomain": "default",
"attributes": {
"url": {
"type": "string"
"type": "string",
"role": "content"
},
"service": {
"type": "string"
},
"label": {
"type": "string"
"type": "string",
"role": "content"
},
"rel": {
"type": "string"
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

Expand Down
40 changes: 40 additions & 0 deletions packages/block-library/src/social-link/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,22 @@ import { DELETE, BACKSPACE } from '@wordpress/keycodes';
import { useDispatch } from '@wordpress/data';

import {
BlockControls,
InspectorControls,
URLPopover,
URLInput,
useBlockEditingMode,
useBlockProps,
store as blockEditorStore,
} from '@wordpress/block-editor';
import { useState } from '@wordpress/element';
import {
Button,
Dropdown,
PanelBody,
PanelRow,
TextControl,
ToolbarButton,
__experimentalInputControlSuffixWrapper as InputControlSuffixWrapper,
} from '@wordpress/components';
import { __ } from '@wordpress/i18n';
Expand Down Expand Up @@ -122,6 +126,7 @@ const SocialLinkEdit = ( {
// Use internal state instead of a ref to make sure that the component
// re-renders when the popover's anchor updates.
const [ popoverAnchor, setPopoverAnchor ] = useState( null );
const isContentOnlyMode = useBlockEditingMode() === 'contentOnly';

const IconComponent = getIconBySite( service );
const socialLinkName = getNameBySite( service );
Expand All @@ -141,6 +146,41 @@ const SocialLinkEdit = ( {

return (
<>
{ isContentOnlyMode && showLabels && (
Copy link
Contributor

@talldan talldan Nov 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

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

Copy link
Member

Choose a reason for hiding this comment

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

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:

Screenshot 2024-12-11 at 1 04 02 pm

And in Write mode, the block styles are shown for all links:

Screenshot 2024-12-11 at 1 03 52 pm

// Add an extra control to modify the label attribute when content only mode is active.
// With content only mode active, the inspector is hidden, so users need another way
// to edit this attribute.
<BlockControls group="other">
<Dropdown
popoverProps={ { position: 'bottom right' } }
renderToggle={ ( { isOpen, onToggle } ) => (
<ToolbarButton
onClick={ onToggle }
aria-haspopup="true"
aria-expanded={ isOpen }
>
{ __( 'Text' ) }
</ToolbarButton>
) }
renderContent={ () => (
<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 }
/>
Comment on lines +166 to +179
Copy link
Contributor

Choose a reason for hiding this comment

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

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 😄

) }
/>
</BlockControls>
) }
<InspectorControls>
<PanelBody title={ __( 'Settings' ) }>
<PanelRow>
Expand Down
5 changes: 5 additions & 0 deletions packages/block-library/src/social-link/editor.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,3 +39,8 @@
:root :where(.wp-block-social-links.is-style-logos-only .wp-social-link button) {
padding: 0;
}

.wp-block-social-link__toolbar_content_text {
// Corresponds to the size of the text control input in the block inspector.
width: 250px;
}
Loading