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

Post publish panel: focus the post link #11489

Merged
merged 4 commits into from
Nov 6, 2018
Merged

Post publish panel: focus the post link #11489

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f711823
Extract variables for clarity
oandregal Nov 5, 2018
54b0b49
PostPublishPanelPostPublish: let the post link have the focus
oandregal Nov 5, 2018
a21a7b8
Add e2e test
oandregal Nov 5, 2018
a20f35e
Update snapshots
oandregal Nov 5, 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
12 changes: 7 additions & 5 deletions packages/editor/src/components/post-publish-panel/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,14 @@ export class PostPublishPanel extends Component {
PrePublishExtension,
...additionalProps
} = this.props;
const isPublishedOrScheduled = isPublished || ( isScheduled && isBeingScheduled );
const propsForPanel = omit( additionalProps, [ 'hasPublishAction', 'isDirty' ] );
const isPublishedOrScheduled = isPublished || ( isScheduled && isBeingScheduled );
const isPrePublish = ! isPublishedOrScheduled && ! isSaving;
Copy link
Member

Choose a reason for hiding this comment

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

I'm wondering if this isSaving check causes re-renders if an autosave fires after the panel is open? I saw some weird re-renders on slower connections (I'm working from the pub right now) and I wonder if a late auto-save after the publish panel is opened causes this to close the panel and then open it again?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure about how the autosave mechanism works, but I've just checked the editor store code and isSaving (that holds state.saving.request) only changes when the content has been edited. So, the spinner will be shown instead of the pre-publish panel when:

  • the post contents are changed while the panel is open
  • the panel was opened before the auto-save kicked in

Note that I couldn't reproduce this with the post-publish panel open.

For reference, before #11013 this wasn't probably happening because we used some internal state (submitted) to keep track of this, which had introduced a bug in scheduling posts.

Happy to revisit this in a separate PR if needed. Just assign it to me!

Copy link
Member Author

Choose a reason for hiding this comment

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

Forgot to mention: I've noticed that focus is retained at the component that had it after spinner is rendered.

const isPostPublish = isPublishedOrScheduled && ! isSaving;
return (
<div className="editor-post-publish-panel" { ...propsForPanel }>
<div className="editor-post-publish-panel__header">
{ isPublishedOrScheduled && ! isSaving ? (
{ isPostPublish ? (
<div className="editor-post-publish-panel__header-published">
{ isScheduled ? __( 'Scheduled' ) : __( 'Published' ) }
</div>
Expand All @@ -83,13 +85,13 @@ export class PostPublishPanel extends Component {
/>
</div>
<div className="editor-post-publish-panel__content">
{ ! isSaving && ! isPublishedOrScheduled && (
{ isPrePublish && (
<PostPublishPanelPrepublish>
{ PrePublishExtension && <PrePublishExtension /> }
</PostPublishPanelPrepublish>
) }
{ ! isSaving && isPublishedOrScheduled && (
<PostPublishPanelPostpublish>
{ isPostPublish && (
<PostPublishPanelPostpublish focusOnMount={ true } >
tofumatt marked this conversation as resolved.
Show resolved Hide resolved
{ PostPublishExtension && <PostPublishExtension /> }
</PostPublishPanelPostpublish>
) }
Expand Down
11 changes: 9 additions & 2 deletions packages/editor/src/components/post-publish-panel/postpublish.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { get } from 'lodash';
*/
import { PanelBody, Button, ClipboardButton, TextControl } from '@wordpress/components';
import { __, sprintf } from '@wordpress/i18n';
import { Component, Fragment } from '@wordpress/element';
import { Component, Fragment, createRef } from '@wordpress/element';
import { withSelect } from '@wordpress/data';

/**
Expand All @@ -24,6 +24,13 @@ class PostPublishPanelPostpublish extends Component {
};
this.onCopy = this.onCopy.bind( this );
this.onSelectInput = this.onSelectInput.bind( this );
this.postLink = createRef();
}

componentDidMount() {
if ( this.props.focusOnMount ) {
this.postLink.current.focus();
}
}

componentWillUnmount() {
Expand Down Expand Up @@ -59,7 +66,7 @@ class PostPublishPanelPostpublish extends Component {
return (
<div className="post-publish-panel__postpublish">
<PanelBody className="post-publish-panel__postpublish-header">
<a href={ post.link }>{ post.title || __( '(no title)' ) }</a> { postPublishNonLinkHeader }
<a ref={ this.postLink } href={ post.link }>{ post.title || __( '(no title)' ) }</a> { postPublishNonLinkHeader }
</PanelBody>
<PanelBody>
<p className="post-publish-panel__postpublish-subheader">
Expand Down
8 changes: 6 additions & 2 deletions packages/editor/src/components/post-publish-panel/test/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@ exports[`PostPublishPanel should render the post-publish panel if the post is pu
<div
className="editor-post-publish-panel__content"
>
<WithSelect(PostPublishPanelPostpublish) />
<WithSelect(PostPublishPanelPostpublish)
focusOnMount={true}
/>
</div>
<div
className="editor-post-publish-panel__footer"
Expand Down Expand Up @@ -54,7 +56,9 @@ exports[`PostPublishPanel should render the post-publish panel if the post is sc
<div
className="editor-post-publish-panel__content"
>
<WithSelect(PostPublishPanelPostpublish) />
<WithSelect(PostPublishPanelPostpublish)
focusOnMount={true}
/>
</div>
<div
className="editor-post-publish-panel__footer"
Expand Down
18 changes: 17 additions & 1 deletion test/e2e/specs/publish-panel.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
newPost,
openPublishPanel,
pressWithModifier,
publishPost,
} from '../support/utils';

describe( 'PostPublishPanel', () => {
Expand All @@ -23,7 +24,7 @@ describe( 'PostPublishPanel', () => {
}
} );

it( 'publish button should have focus', async () => {
it( 'PrePublish: publish button should have the focus', async () => {
await page.type( '.editor-post-title__input', 'E2E Test Post' );
await openPublishPanel();

Expand All @@ -33,6 +34,21 @@ describe( 'PostPublishPanel', () => {
expect( focusedElementClassList ).toContain( 'editor-post-publish-button' );
} );

it( 'PostPublish: post link should have the focus', async () => {
const postTitle = 'E2E Test Post';
await page.type( '.editor-post-title__input', postTitle );
await publishPost();

const focusedElementTag = await page.$eval( ':focus', ( focusedElement ) => {
return focusedElement.tagName.toLowerCase();
} );
const focusedElementText = await page.$eval( ':focus', ( focusedElement ) => {
return focusedElement.text;
} );
expect( focusedElementTag ).toBe( 'a' );
expect( focusedElementText ).toBe( postTitle );
} );

it( 'should retain focus within the panel', async () => {
await page.type( '.editor-post-title__input', 'E2E Test Post' );
await openPublishPanel();
Expand Down