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

PostPublishPanel: return focus to element that opened the panel #11543

Merged
merged 27 commits into from
Nov 9, 2018
Merged

PostPublishPanel: return focus to element that opened the panel #11543

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
3044063
Toggle disabled if post is published
oandregal Nov 7, 2018
6cca773
Toggle should be disabled if post is published
oandregal Nov 8, 2018
4bfb697
Return focus to Publish... button
oandregal Nov 6, 2018
0fac6ef
Sort dependencies
oandregal Nov 7, 2018
3155c59
Extract props
oandregal Nov 7, 2018
9228b68
Add toggleProps to button
oandregal Nov 8, 2018
557eca0
Inline text for toggle within the button component
oandregal Nov 8, 2018
4bbcac4
Always render the button component
oandregal Nov 8, 2018
22a421f
Use aria-disabled instead of disabled to avoid focus loss
oandregal Nov 8, 2018
35de337
Improve comments
oandregal Nov 8, 2018
c2b1a9f
Update snapshots
oandregal Nov 8, 2018
b12e4c3
Add deprecation notice in editor CHANGELOG
oandregal Nov 9, 2018
9ba9dd4
Add deprecation notice in plugin
oandregal Nov 9, 2018
6956da2
Add deprecation in component
oandregal Nov 9, 2018
e310d64
Add DotTip to PostPublishButton
oandregal Nov 9, 2018
72b8e99
Fix deprecation warning in tests
oandregal Nov 9, 2018
0e2cae7
Update PostPublishButton e2e tests to use aria-disabled instead of di…
oandregal Nov 9, 2018
2ffa78a
Update e2e test
oandregal Nov 9, 2018
57254cd
Tweak comment
tofumatt Nov 9, 2018
48439c0
Update version
nosolosw Nov 9, 2018
9341979
Update packages/editor/src/components/post-publish-panel/toggle.js
nosolosw Nov 9, 2018
0ef0206
Place component in a separate line
tofumatt Nov 9, 2018
94b251f
Update version number
nosolosw Nov 9, 2018
8f50b19
Add comment
oandregal Nov 9, 2018
ca93215
Update comment
oandregal Nov 9, 2018
99433b4
Tweak comment
tofumatt Nov 9, 2018
8b0d6bb
chore: Move deps to 4.5
tofumatt Nov 9, 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
4 changes: 4 additions & 0 deletions docs/reference/deprecated.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
Gutenberg's deprecation policy is intended to support backwards-compatibility for releases, when possible. The current deprecations are listed below and are grouped by _the version at which they will be removed completely_. If your plugin depends on these behaviors, you must update to the recommended alternative before the noted version.

## 5.1

- `wp.editor.PostPublishPanelToggle` has been deprecated in favor of `wp.editor.PostPublishButton`.

## 4.5.0
- `Dropdown.refresh()` has been deprecated as the contained `Popover` is now automatically refreshed.

Expand Down
50 changes: 24 additions & 26 deletions packages/edit-post/src/components/header/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,34 +39,32 @@ function Header( {
tabIndex="-1"
>
<HeaderToolbar />
{ ! isPublishSidebarOpened && (
<div className="edit-post-header__settings">
<PostSavedState
forceIsDirty={ hasActiveMetaboxes }
forceIsSaving={ isSaving }
<div className="edit-post-header__settings">
{ ! isPublishSidebarOpened && ( <PostSavedState
forceIsDirty={ hasActiveMetaboxes }
forceIsSaving={ isSaving }
/> ) }
<PostPreviewButton />
<PostPublishButtonOrToggle
forceIsDirty={ hasActiveMetaboxes }
forceIsSaving={ isSaving }
/>
<div>
<IconButton
icon="admin-generic"
label={ __( 'Settings' ) }
onClick={ toggleGeneralSidebar }
isToggled={ isEditorSidebarOpened }
aria-expanded={ isEditorSidebarOpened }
shortcut={ shortcuts.toggleSidebar }
/>
<PostPreviewButton />
<PostPublishButtonOrToggle
forceIsDirty={ hasActiveMetaboxes }
forceIsSaving={ isSaving }
/>
<div>
<IconButton
icon="admin-generic"
label={ __( 'Settings' ) }
onClick={ toggleGeneralSidebar }
isToggled={ isEditorSidebarOpened }
aria-expanded={ isEditorSidebarOpened }
shortcut={ shortcuts.toggleSidebar }
/>
<DotTip tipId="core/editor.settings">
{ __( 'You’ll find more settings for your page and blocks in the sidebar. Click “Settings” to open it.' ) }
</DotTip>
</div>
<PinnedPlugins.Slot />
<MoreMenu />
<DotTip tipId="core/editor.settings">
{ __( 'You’ll find more settings for your page and blocks in the sidebar. Click “Settings” to open it.' ) }
</DotTip>
</div>
) }
<PinnedPlugins.Slot />
<MoreMenu />
</div>
</div>
);
}
Expand Down
68 changes: 34 additions & 34 deletions packages/edit-post/src/components/header/post-publish-button-or-toggle.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ import { get } from 'lodash';
/**
* WordPress dependencies.
*/
import { PostPublishPanelToggle, PostPublishButton } from '@wordpress/editor';
import { compose } from '@wordpress/compose';
import { withDispatch, withSelect } from '@wordpress/data';
import { PostPublishButton } from '@wordpress/editor';
import { withViewportMatch } from '@wordpress/viewport';

export function PostPublishButtonOrToggle( {
Expand All @@ -24,54 +24,54 @@ export function PostPublishButtonOrToggle( {
isScheduled,
togglePublishSidebar,
} ) {
const button = (
<PostPublishButton
forceIsDirty={ forceIsDirty }
forceIsSaving={ forceIsSaving }
/>
);
const toggle = (
<PostPublishPanelToggle
isOpen={ isPublishSidebarOpened }
onToggle={ togglePublishSidebar }
forceIsSaving={ forceIsSaving }
forceIsDirty={ forceIsDirty }
/>
);
const IS_TOGGLE = 'toggle';
const IS_BUTTON = 'button';
let component;

/**
* We want to show a BUTTON when the post status is at the _final stage_
* Conditions to show a BUTTON (publish directly) or a TOGGLE (open publish sidebar):
Copy link
Member Author

Choose a reason for hiding this comment

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

This big comment absorbs the previous two different existing comments.

*
* 1) We want to show a BUTTON when the post status is at the _final stage_
* for a particular role (see https://codex.wordpress.org/Post_Status):
*
* - is published
* - is scheduled to be published
* - is pending and can't be published (but only for viewports >= medium)
* - is pending and can't be published (but only for viewports >= medium).
* Originally, we considered showing a button for pending posts that couldn't be published
* (for ex, for a contributor role). Some languages can have really long translations
* for "Submit for review", so given the lack of UI real state available
* we decided to take into account the viewport in that particular case.
*
* 2) Then, in small viewports, we'll show a TOGGLE.
*
* Originally we considered showing a button for pending posts
* that couldn't be published (for ex, for a contributor role).
* Some languages can have really long translations for "Submit for review",
* so given the lack of UI real state we decided to take into account the viewport
* in that particular case.
* 3) Finally, we'll use the publish sidebar status to decide:
*
* - if it is enabled, we show a TOGGLE
* - if it is disabled, we show a BUTTON
*/
if (
isPublished ||
( isScheduled && isBeingScheduled ) ||
( isPending && ! hasPublishAction && ! isLessThanMediumViewport )
) {
return button;
component = IS_BUTTON;
} else if ( isLessThanMediumViewport ) {
component = IS_TOGGLE;
} else if ( isPublishSidebarEnabled ) {
component = IS_TOGGLE;
} else {
component = IS_BUTTON;
}

/**
* Then, we take other things into account:
*
* - Show TOGGLE if it is small viewport.
* - Otherwise, use publish sidebar status to decide - TOGGLE if enabled, BUTTON if not.
*/
if ( isLessThanMediumViewport ) {
return toggle;
}

return isPublishSidebarEnabled ? toggle : button;
return (
<PostPublishButton
forceIsDirty={ forceIsDirty }
forceIsSaving={ forceIsSaving }
isOpen={ isPublishSidebarOpened }
isToggle={ component === IS_TOGGLE }
onToggle={ togglePublishSidebar }
/>
);
}

export default compose(
Expand Down
36 changes: 30 additions & 6 deletions packages/edit-post/src/components/header/test/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,37 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`PostPublishButtonOrToggle should render a button when post is not (1), (2), (3), the viewport is >= medium, and the publish sidebar is disabled 1`] = `<WithSelect(WithDispatch(PostPublishButton)) />`;
exports[`PostPublishButtonOrToggle should render a button when post is not (1), (2), (3), the viewport is >= medium, and the publish sidebar is disabled 1`] = `
<WithSelect(WithDispatch(PostPublishButton))
isToggle={false}
/>
`;

exports[`PostPublishButtonOrToggle should render a button when the post is pending and cannot be published but the viewport is >= medium (3) 1`] = `<WithSelect(WithDispatch(PostPublishButton)) />`;
exports[`PostPublishButtonOrToggle should render a button when the post is pending and cannot be published but the viewport is >= medium (3) 1`] = `
<WithSelect(WithDispatch(PostPublishButton))
isToggle={false}
/>
`;

exports[`PostPublishButtonOrToggle should render a button when the post is published (1) 1`] = `<WithSelect(WithDispatch(PostPublishButton)) />`;
exports[`PostPublishButtonOrToggle should render a button when the post is published (1) 1`] = `
<WithSelect(WithDispatch(PostPublishButton))
isToggle={false}
/>
`;

exports[`PostPublishButtonOrToggle should render a button when the post is scheduled (2) 1`] = `<WithSelect(WithDispatch(PostPublishButton)) />`;
exports[`PostPublishButtonOrToggle should render a button when the post is scheduled (2) 1`] = `
<WithSelect(WithDispatch(PostPublishButton))
isToggle={false}
/>
`;

exports[`PostPublishButtonOrToggle should render a toggle when post is not (1), (2), (3), the viewport is >= medium, and the publish sidebar is enabled 1`] = `<WithSelect(PostPublishPanelToggle) />`;
exports[`PostPublishButtonOrToggle should render a toggle when post is not (1), (2), (3), the viewport is >= medium, and the publish sidebar is enabled 1`] = `
<WithSelect(WithDispatch(PostPublishButton))
isToggle={true}
/>
`;

exports[`PostPublishButtonOrToggle should render a toggle when post is not published or scheduled and the viewport is < medium 1`] = `<WithSelect(PostPublishPanelToggle) />`;
exports[`PostPublishButtonOrToggle should render a toggle when post is not published or scheduled and the viewport is < medium 1`] = `
<WithSelect(WithDispatch(PostPublishButton))
isToggle={true}
/>
`;
4 changes: 4 additions & 0 deletions packages/editor/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
## 6.2.1 (Unreleased)

### Deprecations

- `wp.editor.PostPublishPanelToggle` has been deprecated in favor of `wp.editor.PostPublishButton`.

### Polish

- Reactive block styles.
Expand Down
65 changes: 49 additions & 16 deletions packages/editor/src/components/post-publish-button/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import { Button } from '@wordpress/components';
import { Component, createRef } from '@wordpress/element';
import { withSelect, withDispatch } from '@wordpress/data';
import { compose } from '@wordpress/compose';
import { __ } from '@wordpress/i18n';
import { DotTip } from '@wordpress/nux';

/**
* Internal dependencies
Expand All @@ -28,19 +30,22 @@ export class PostPublishButton extends Component {

render() {
const {
isSaving,
onStatusChange,
onSave,
forceIsDirty,
forceIsSaving,
hasPublishAction,
isBeingScheduled,
visibility,
isPublishable,
isSaveable,
isOpen,
isPostSavingLocked,
isPublishable,
isPublished,
hasPublishAction,
isSaveable,
isSaving,
isToggle,
onSave,
onStatusChange,
onSubmit = noop,
forceIsDirty,
forceIsSaving,
onToggle,
visibility,
} = this.props;
const isButtonDisabled =
isSaving ||
Expand All @@ -49,6 +54,13 @@ export class PostPublishButton extends Component {
isPostSavingLocked ||
( ! isPublishable && ! forceIsDirty );

const isToggleDisabled =
isPublished ||
isSaving ||
forceIsSaving ||
! isSaveable ||
( ! isPublishable && ! forceIsDirty );

let publishStatus;
if ( ! hasPublishAction ) {
publishStatus = 'pending';
Expand All @@ -66,17 +78,38 @@ export class PostPublishButton extends Component {
onSave();
};

const buttonProps = {
'aria-disabled': isButtonDisabled,
className: 'editor-post-publish-button',
isBusy: isSaving && isPublished,
isLarge: true,
isPrimary: true,
onClick,
};

const toggleProps = {
'aria-disabled': isToggleDisabled,
'aria-expanded': isOpen,
className: 'editor-post-publish-panel__toggle',
Copy link
Member Author

@oandregal oandregal Nov 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Should we update this to be editor-post-publish-toggle instead? We can maintain both the existing and the new class until the toggle component is removed as to preserve compatibility. I can prepare a follow-up PR to this one with the changes if we wanted to do it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Or perhaps a better alternative would be to remove the editor-post-publish-panel__toggle class once the toggle component is removed.

Copy link
Member

@tofumatt tofumatt Nov 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

editor-post-publish-panel__toggle seems like the right name.

EDIT: oops, I meant editor-post-publish-button__toggle

isBusy: isSaving && isPublished,
isPrimary: true,
onClick: onToggle,
};

const toggleChildren = isBeingScheduled ? __( 'Schedule…' ) : __( 'Publish…' );
const buttonChildren = <PublishButtonLabel forceIsSaving={ forceIsSaving } />;

const componentProps = isToggle ? toggleProps : buttonProps;
const componentChildren = isToggle ? toggleChildren : buttonChildren;
return (
<Button
ref={ this.buttonNode }
className="editor-post-publish-button"
isPrimary
isLarge
onClick={ onClick }
disabled={ isButtonDisabled }
Copy link
Member

Choose a reason for hiding this comment

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

For what reason did we need to stop passing the disabled prop? While this and many other buttons in the header appear as disabled, they don't act as disabled. You can immediately press "Publish" on a new post and the publish panel is (wrongly?) shown.

As it relates to focus return, it's not clear to me how it's relevant, since the panel would have presumably never been opened if it were disabled.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. #13194 should fix it.

The disabled property didn't play well with the withFocusReturn HOC, IIRC. I can dig up the exact details if necessary.

isBusy={ isSaving && isPublished }
{ ...componentProps }
>
<PublishButtonLabel forceIsSaving={ forceIsSaving } />
{ componentChildren }
<DotTip tipId="core/editor.publish">
Copy link
Member Author

Choose a reason for hiding this comment

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

The DotTip component was only visible for the toggle. This PR changes it and shows the tip no matter the button publish directly or opens the publish sidebar.

{ __( 'Finished writing? That’s great, let’s get this published right now. Just click “Publish” and you’re good to go.' ) }
Copy link
Member

Choose a reason for hiding this comment

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

I find "Just" to be a weasel word and would like to remove it, but I know that's a bit out-of-scope for the PR. Just saying' 😆

</DotTip>
</Button>
);
}
Expand Down
Loading