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

Jump to bottom

a11y: Add aria-label attributes and custom override control to core/social-link block #18651

Merged
merged 11 commits into from
Dec 27, 2019
Merged

a11y: Add aria-label attributes and custom override control to core/social-link block #18651

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
bb35aaf
add aria-label to social link block
0aveRyan Nov 21, 2019
aadde37
phpcs codesmells
0aveRyan Nov 21, 2019
fcee78e
phpcs docblock and one bad array key
0aveRyan Nov 21, 2019
c9ffe6b
social link brand spellings
0aveRyan Dec 3, 2019
05541ca
sidebar helper text
0aveRyan Dec 3, 2019
f3bda49
addressing feedback
0aveRyan Dec 26, 2019
d10d5ef
Merge branch 'master' into a11y/social-link-anchor-labels
0aveRyan Dec 26, 2019
982b894
linting
0aveRyan Dec 26, 2019
6d319a2
final linting
0aveRyan Dec 26, 2019
c20f894
label text removed in swap
0aveRyan Dec 26, 2019
cf861db
doh, final translation string fix
0aveRyan Dec 26, 2019
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
80 changes: 49 additions & 31 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 @@ -6,21 +6,24 @@ import classNames from 'classnames';
/**
Copy link
Contributor

Choose a reason for hiding this comment

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

I see a few strings here where we use Title Case. I think we've been recently moving to always using "Sentence case" in all of our strings. @karmatosed do you confirm?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Screen Shot 2019-12-26 at 10 03 00 AM

@youknowriad that doesn't appear to be the standard used for Panel Titles and that doesn't seem to be the standard outlined in the docs. Was there a discussion about that I missed? If that's changed, the docs need to be updated and issues opened to address the many instances of title case throughout the application.

Copy link
Contributor Author

@0aveRyan 0aveRyan Dec 27, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I've since dug and found the PRs and issue discussion around sentence case. A major departure like that should be discussed and memorialized in docs with clearer examples well before it's put into practice, to set clear expectations, avoid confusing situations like this and throwing up hurdles to contribution. Let me know if you'd like these strings updated to sentence case.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know to be honest, I'll let @karmatosed chime in when she comes back. I think we can land this as is for now.

* WordPress dependencies
*/
import { URLPopover } from '@wordpress/block-editor';
import { useState } from '@wordpress/element';
import { InspectorControls, URLPopover } from '@wordpress/block-editor';
import { Fragment, useState } from '@wordpress/element';
import {
Button,
IconButton,
PanelBody,
PanelRow,
TextControl,
} from '@wordpress/components';
import { __ } from '@wordpress/i18n';

/**
* Internal dependencies
*/
import { getIconBySite } from './social-list';
import { getIconBySite, getNameBySite } from './social-list';

const SocialLinkEdit = ( { attributes, setAttributes, isSelected } ) => {
const { url, site } = attributes;
const { url, site, label } = attributes;
const [ showURLPopover, setPopover ] = useState( false );
const classes = classNames(
'wp-social-link',
Expand All @@ -30,35 +33,50 @@ const SocialLinkEdit = ( { attributes, setAttributes, isSelected } ) => {

// Import icon.
const IconComponent = getIconBySite( site );
const SocialLinkName = getNameBySite( site );
0aveRyan marked this conversation as resolved.
Show resolved Hide resolved

return (
<Button
className={ classes }
onClick={ () => setPopover( true ) }
>
<IconComponent />
{ isSelected && showURLPopover && (
<URLPopover
onClose={ () => setPopover( false ) }
>
<form
className="block-editor-url-popover__link-editor"
onSubmit={ ( event ) => {
event.preventDefault();
setPopover( false );
} } >
<div className="editor-url-input block-editor-url-input">
<input type="text"
value={ url }
onChange={ ( event ) => setAttributes( { url: event.target.value } ) }
placeholder={ __( 'Enter Address' ) }
/>
</div>
<IconButton icon="editor-break" label={ __( 'Apply' ) } type="submit" />
</form>
</URLPopover>
) }
</Button>
<Fragment>
<InspectorControls>
<PanelBody title={ `${ SocialLinkName }` + __( ' Label' ) } initialOpen={ false }>
0aveRyan marked this conversation as resolved.
Show resolved Hide resolved
<PanelRow>
<TextControl
label={ __( 'Link Label' ) }
help={ __( 'Briefly describe the page you\'re linking to. This helps better identify the link for screen reader users.' ) }
0aveRyan marked this conversation as resolved.
Show resolved Hide resolved
value={ label }
onChange={ ( value ) => setAttributes( { label: value } ) }
/>
</PanelRow>
</PanelBody>
</InspectorControls>
<Button
className={ classes }
onClick={ () => setPopover( true ) }
>
<IconComponent />
{ isSelected && showURLPopover && (
<URLPopover
onClose={ () => setPopover( false ) }
>
<form
className="block-editor-url-popover__link-editor"
onSubmit={ ( event ) => {
event.preventDefault();
setPopover( false );
} } >
<div className="editor-url-input block-editor-url-input">
0aveRyan marked this conversation as resolved.
Show resolved Hide resolved
<input type="text"
value={ url }
onChange={ ( event ) => setAttributes( { url: event.target.value } ) }
placeholder={ __( 'Enter Address' ) }
/>
</div>
<IconButton icon="editor-break" label={ __( 'Apply' ) } type="submit" />
</form>
</URLPopover>
) }
</Button>
</Fragment>
);
};

Expand Down
3 changes: 3 additions & 0 deletions packages/block-library/src/social-link/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ export const sites = Object.keys( socialList ).map(
type: 'string',
default: site,
},
label: {
type: 'string',
},
},
},
};
Expand Down
Loading