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.

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

Add an icon and visually hidden a11y text to links that open in a new tab #7883

Merged
merged 17 commits into from
Jul 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
7e86241
Use ExternalLink for link in editor toolbar for RichText block
talldan Jul 10, 2018
d49339e
Add prop for icon class name to ExternalLink
talldan Jul 10, 2018
7b1b42d
Allow Button component to render an ExternalLink
talldan Jul 10, 2018
2938366
Specify PostPreviewButton as opening external links
talldan Jul 10, 2018
e9686f6
Fix overly specific selector preventing override of icon styles
talldan Jul 10, 2018
90e7e57
Position dashicon for external links when rendered via Button
talldan Jul 10, 2018
1d61f51
Set isExternalLink true for post permalink and adjust styling
talldan Jul 10, 2018
0c01440
Fix incorrect default display property given to button
talldan Jul 11, 2018
80496ab
Use flex to center icon in button and add a tiny bit of margin to adjust
talldan Jul 11, 2018
33cd704
Add tests for isExternalLink prop in Button component
talldan Jul 11, 2018
241a84f
Pass component-button__icon class through to icon, and use it for scs…
talldan Jul 11, 2018
953beb1
Make post preview button render as a button instead of a link
talldan Jul 13, 2018
651eb5a
Revert "Pass component-button__icon class through to icon, and use it…
talldan Jul 16, 2018
b7dc451
Make the post permalink link an ExternalLink instead of a Button
talldan Jul 16, 2018
fdfcc2b
Rename member variable used as ref to reflect that it's not longer a …
talldan Jul 16, 2018
3eeb9f2
Modify ExternalLink component to forwards its ref
talldan Jul 16, 2018
e1a63e8
Adjust styling to work in a wider range of contexts
talldan Jul 16, 2018
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 edit-post/components/header/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
display: none;

@include break-small {
display: inline-block;
display: inline-flex;
}
}
}
Expand Down
10 changes: 5 additions & 5 deletions editor/components/post-permalink/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ import classnames from 'classnames';
import { withDispatch, withSelect } from '@wordpress/data';
import { Component } from '@wordpress/element';
import { __ } from '@wordpress/i18n';
import { ClipboardButton, Button } from '@wordpress/components';
import { compose } from '@wordpress/compose';
import { ClipboardButton, Button, ExternalLink } from '@wordpress/components';

/**
* Internal Dependencies
Expand Down Expand Up @@ -48,7 +48,7 @@ class PostPermalink extends Component {
componentDidUpdate( prevProps, prevState ) {
// If we've just stopped editing the permalink, focus on the new permalink.
if ( prevState.isEditingPermalink && ! this.state.isEditingPermalink ) {
this.permalinkButton.focus();
this.linkElement.focus();
}
}

Expand Down Expand Up @@ -79,15 +79,15 @@ class PostPermalink extends Component {
<span className="editor-post-permalink__label">{ __( 'Permalink:' ) }</span>

{ ! isEditingPermalink &&
<Button
<ExternalLink
className="editor-post-permalink__link"
href={ ! isPublished ? postLink : samplePermalink }
target="_blank"
ref={ ( permalinkButton ) => this.permalinkButton = permalinkButton }
ref={ ( linkElement ) => this.linkElement = linkElement }
>
{ decodeURI( samplePermalink ) }
&lrm;
</Button>
</ExternalLink>
}

{ isEditingPermalink &&
Expand Down
12 changes: 7 additions & 5 deletions editor/components/post-preview-button/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,12 +63,16 @@ export class PostPreviewButton extends Component {
* @param {MouseEvent} event Click event from preview button click.
*/
openPreviewWindow( event ) {
const { isAutosaveable, previewLink } = this.props;
const { isAutosaveable, previewLink, currentPostLink } = this.props;

// If there are no changes to autosave, we cannot perform the save, but
// if there is an existing preview link (e.g. previous published post
// autosave), it should be reused as the popup destination.
if ( ! isAutosaveable && ! previewLink ) {
if ( ! isAutosaveable && ! previewLink && currentPostLink ) {
this.previewWindow = window.open(
currentPostLink,
this.getWindowTarget()
);
return;
}

Expand Down Expand Up @@ -115,15 +119,13 @@ export class PostPreviewButton extends Component {
}

render() {
const { currentPostLink, isSaveable } = this.props;
const { isSaveable } = this.props;

return (
<Button
className="editor-post-preview"
isLarge
href={ currentPostLink }
onClick={ this.openPreviewWindow }
target={ this.getWindowTarget() }
disabled={ ! isSaveable }
>
{ _x( 'Preview', 'imperative verb' ) }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`PostPreviewButton render() should match the snapshot 1`] = `
<Button
className="editor-post-preview"
disabled={false}
isLarge={true}
onClick={[Function]}
>
Preview
<WithSafeTimeout(WithSelect(WithDispatch(DotTip)))
id="core/editor.preview"
>
Click ‘Preview’ to load a preview of this page, so you can make sure you’re happy with your blocks.
</WithSafeTimeout(WithSelect(WithDispatch(DotTip)))>
</Button>
`;
17 changes: 10 additions & 7 deletions editor/components/post-preview-button/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,10 @@ describe( 'PostPreviewButton', () => {
wrapper.simulate( 'click', { preventDefault } );

if ( expectedPreviewURL ) {
expect( preventDefault ).toHaveBeenCalled();
if ( expectedPreviewURL !== props.currentPostLink ) {
expect( preventDefault ).toHaveBeenCalled();
}

expect( window.open ).toHaveBeenCalledWith( expectedPreviewURL, 'wp-preview-1' );
} else {
expect( preventDefault ).not.toHaveBeenCalled();
Expand All @@ -113,11 +116,13 @@ describe( 'PostPreviewButton', () => {
return wrapper;
}

it( 'should do nothing if neither autosaveable nor preview link available', () => {
it( 'should open the currentPostLink if not autosaveable nor preview link available', () => {
const currentPostLink = 'https://wordpress.org/?p=1';
assertForPreview( {
isAutosaveable: false,
previewLink: undefined,
}, null, false );
currentPostLink,
}, currentPostLink, false );
} );

it( 'should save for autosaveable post with preview link', () => {
Expand All @@ -143,17 +148,15 @@ describe( 'PostPreviewButton', () => {
} );

describe( 'render()', () => {
it( 'should render a link', () => {
it( 'should match the snapshot', () => {
const wrapper = shallow(
<PostPreviewButton
postId={ 1 }
isSaveable
currentPostLink="https://wordpress.org/?p=1" />
);

expect( wrapper.prop( 'href' ) ).toBe( 'https://wordpress.org/?p=1' );
expect( wrapper.prop( 'disabled' ) ).toBe( false );
expect( wrapper.prop( 'target' ) ).toBe( 'wp-preview-1' );
expect( wrapper ).toMatchSnapshot();
} );

it( 'should be disabled if post is not saveable', () => {
Expand Down
6 changes: 3 additions & 3 deletions editor/components/rich-text/format-toolbar/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
Toolbar,
withSpokenMessages,
Popover,
ExternalLink,
} from '@wordpress/components';
import { ESCAPE, LEFT, RIGHT, UP, DOWN, BACKSPACE, ENTER, displayShortcut } from '@wordpress/keycodes';
import { prependHTTP } from '@wordpress/url';
Expand Down Expand Up @@ -255,13 +256,12 @@ class FormatToolbar extends Component {
onKeyPress={ stopKeyPropagation }
>
<div className="editor-format-toolbar__link-modal-line">
<a
<ExternalLink
className="editor-format-toolbar__link-value"
href={ formats.link.value }
target="_blank"
>
{ formats.link.value && filterURLForDisplay( decodeURI( formats.link.value ) ) }
</a>
</ExternalLink>
<IconButton icon="edit" label={ __( 'Edit' ) } onClick={ this.editLink } />
<IconButton
className="editor-format-toolbar__link-settings-toggle"
Expand Down
5 changes: 5 additions & 0 deletions packages/components/src/button/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ import classnames from 'classnames';
*/
import { createElement, forwardRef } from '@wordpress/element';

/**
* Internal dependencies
*/
import './style.scss';

export function Button( props, ref ) {
const {
href,
Expand Down
9 changes: 5 additions & 4 deletions packages/components/src/external-link/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@ import { compact, uniq } from 'lodash';
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { forwardRef } from '@wordpress/element';

/**
* Internal dependencies
*/
import Dashicon from '../dashicon';

function ExternalLink( { href, children, className, rel = '', ...additionalProps } ) {
export function ExternalLink( { href, children, className, rel = '', ...additionalProps }, ref ) {
rel = uniq( compact( [
...rel.split( ' ' ),
'external',
Expand All @@ -23,17 +24,17 @@ function ExternalLink( { href, children, className, rel = '', ...additionalProps
] ) ).join( ' ' );
const classes = classnames( 'components-external-link', className );
return (
<a { ...additionalProps } className={ classes } href={ href } target="_blank" rel={ rel }>
<a { ...additionalProps } className={ classes } href={ href } target="_blank" rel={ rel } ref={ ref }>
{ children }
<span className="screen-reader-text">
{
/* translators: accessibility text */
__( '(opens in a new window)' )
}
</span>
<Dashicon icon="external" />
<Dashicon icon="external" className="components-external-link__icon" />
</a>
);
}

export default ExternalLink;
export default forwardRef( ExternalLink );
14 changes: 6 additions & 8 deletions packages/components/src/external-link/style.scss
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
.components-external-link {
.dashicon {
width: 1.4em;
height: 1.4em;
margin: 0 .2em;
vertical-align: top;
}
}
.components-external-link__icon {
width: 1.4em;
height: 1.4em;
margin: -0.2em 0.1em 0;
vertical-align: middle;
}