-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Make sure Social icons links aren't empty and improve UI clarity #60047
Merged
+36
−33
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
334fca2
Make sure social links are not empty.
afercia f9696e8
Improve labels and descriptions.
afercia 302f22c
Move popover outside of button to make the Apply button tooltip work.
afercia 8248dfe
socialLinkLabel fallbacks to the social media name so no need for und…
afercia 682aa2d
Link text setting panel open by default.
afercia 6b08f40
Remove the term media from social media.
afercia f637158
Change panel title to Settings.
afercia b3608e1
Improve labels.
afercia cecdb6f
Prevent empty social links on front end.
afercia 07c17f9
Fallback to social name in the editing UI.
afercia File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,7 +23,7 @@ import { | |
PanelRow, | ||
TextControl, | ||
} from '@wordpress/components'; | ||
import { __, sprintf } from '@wordpress/i18n'; | ||
import { __ } from '@wordpress/i18n'; | ||
import { keyboardReturn } from '@wordpress/icons'; | ||
|
||
/** | ||
|
@@ -58,7 +58,9 @@ const SocialLinkURLPopover = ( { | |
onChange={ ( nextURL ) => | ||
setAttributes( { url: nextURL } ) | ||
} | ||
placeholder={ __( 'Enter address' ) } | ||
placeholder={ __( 'Enter social link' ) } | ||
label={ __( 'Enter social link' ) } | ||
hideLabelFromVision | ||
disableSuggestions | ||
onKeyDown={ ( event ) => { | ||
if ( | ||
|
@@ -91,7 +93,7 @@ const SocialLinkEdit = ( { | |
setAttributes, | ||
clientId, | ||
} ) => { | ||
const { url, service, label, rel } = attributes; | ||
const { url, service, label = '', rel } = attributes; | ||
const { | ||
showLabels, | ||
iconColor, | ||
|
@@ -113,7 +115,12 @@ const SocialLinkEdit = ( { | |
|
||
const IconComponent = getIconBySite( service ); | ||
const socialLinkName = getNameBySite( service ); | ||
const socialLinkLabel = label ?? socialLinkName; | ||
// The initial label (ie. the link text) is an empty string. | ||
// We want to prevent empty links so that the link text always fallbacks to | ||
// the social name, even when users enter and save an empty string or only | ||
// spaces. The PHP render callback fallbacks to the social name as well. | ||
const socialLinkText = label.trim() === '' ? socialLinkName : label; | ||
|
||
const blockProps = useBlockProps( { | ||
className: classes, | ||
style: { | ||
|
@@ -125,25 +132,19 @@ const SocialLinkEdit = ( { | |
return ( | ||
<> | ||
<InspectorControls> | ||
<PanelBody | ||
title={ sprintf( | ||
/* translators: %s: name of the social service. */ | ||
__( '%s label' ), | ||
socialLinkName | ||
) } | ||
initialOpen={ false } | ||
> | ||
<PanelBody title={ __( 'Settings' ) }> | ||
<PanelRow> | ||
<TextControl | ||
__nextHasNoMarginBottom | ||
label={ __( 'Link label' ) } | ||
label={ __( 'Link text' ) } | ||
help={ __( | ||
'Briefly describe the link to help screen reader users.' | ||
'The link text is visible when enabled from the parent Social Icons block.' | ||
) } | ||
value={ label || '' } | ||
value={ label } | ||
onChange={ ( value ) => | ||
setAttributes( { label: value || undefined } ) | ||
setAttributes( { label: value } ) | ||
} | ||
placeholder={ socialLinkName } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A nice addition! |
||
/> | ||
</PanelRow> | ||
</PanelBody> | ||
|
@@ -168,18 +169,18 @@ const SocialLinkEdit = ( { | |
'screen-reader-text': ! showLabels, | ||
} ) } | ||
> | ||
{ socialLinkLabel } | ||
{ socialLinkText } | ||
</span> | ||
{ isSelected && showURLPopover && ( | ||
<SocialLinkURLPopover | ||
url={ url } | ||
setAttributes={ setAttributes } | ||
setPopover={ setPopover } | ||
popoverAnchor={ popoverAnchor } | ||
clientId={ clientId } | ||
/> | ||
) } | ||
</Button> | ||
{ isSelected && showURLPopover && ( | ||
<SocialLinkURLPopover | ||
url={ url } | ||
setAttributes={ setAttributes } | ||
setPopover={ setPopover } | ||
popoverAnchor={ popoverAnchor } | ||
clientId={ clientId } | ||
/> | ||
) } | ||
</li> | ||
</> | ||
); | ||
|
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
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
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
Oops, something went wrong.
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.
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.
We don't use "Link text" as a control label elsewhere. Perhaps we should just use "Text" and allow for the
help
prop to communicate what it is.LinkControl uses "Text" and the navigation link block uses "Label" (which needs to be changed to "Text", to map to the LinkControl). I suspect we can do the same here for consistency.
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.
We should. See also #59993
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.
I disagree. In LinkControl, it's duplicative to have "Link text" and "Link URL" when you're opening/editing a link.
But what we should do here is the same controls as the Navigation Item blocks (like Custom Link), where you have the same "Text" and "Link" fields, in the same order.