-
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
PostPublishPanel: return focus to element that opened the panel #11543
Changes from 25 commits
3044063
6cca773
4bfb697
0fac6ef
3155c59
9228b68
557eca0
4bbcac4
22a421f
35de337
c2b1a9f
b12e4c3
9ba9dd4
6956da2
e310d64
72b8e99
0e2cae7
2ffa78a
57254cd
48439c0
9341979
0ef0206
94b251f
8f50b19
ca93215
99433b4
8b0d6bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -39,34 +39,39 @@ function Header( { | |
tabIndex="-1" | ||
> | ||
<HeaderToolbar /> | ||
{ ! isPublishSidebarOpened && ( | ||
<div className="edit-post-header__settings"> | ||
<div className="edit-post-header__settings"> | ||
{ ! isPublishSidebarOpened && ( | ||
// This button isn't completely hidden by the publish sidebar. | ||
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. 👍 This is an awesome, super-helpful comment. Ace! |
||
// We can't hide the whole toolbar when the publish sidebar is open because | ||
// we want to prevent mounting/unmounting the PostPublishButtonOrToggle DOM node. | ||
// We track that DOM node to return focus to the PostPublishButtonOrToggle | ||
// when the publish sidebar has been closed. | ||
<PostSavedState | ||
forceIsDirty={ hasActiveMetaboxes } | ||
forceIsSaving={ isSaving } | ||
/> | ||
<PostPreviewButton /> | ||
<PostPublishButtonOrToggle | ||
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 } | ||
/> | ||
<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> | ||
); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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( { | ||
|
@@ -24,54 +24,55 @@ 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): | ||
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. 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 example, for an author with the contributor role). Some languages can have | ||
* long translations for "Submit for review", so given the lack of UI real estate available | ||
* we decided to take into account the viewport in that case. See | ||
* https://github.com/WordPress/gutenberg/issues/10475 | ||
* | ||
* 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( | ||
|
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} | ||
/> | ||
`; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -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 || | ||
|
@@ -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'; | ||
|
@@ -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', | ||
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. Should we update this to be 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. Or perhaps a better alternative would be to remove the 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.
EDIT: oops, I meant |
||
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 } | ||
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. For what reason did we need to stop passing the 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. 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. Good catch. #13194 should fix it. The |
||
isBusy={ isSaving && isPublished } | ||
{ ...componentProps } | ||
> | ||
<PublishButtonLabel forceIsSaving={ forceIsSaving } /> | ||
{ componentChildren } | ||
<DotTip tipId="core/editor.publish"> | ||
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. The |
||
{ __( 'Finished writing? That’s great, let’s get this published right now. Just click “Publish” and you’re good to go.' ) } | ||
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. 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> | ||
); | ||
} | ||
|
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.
Best to keep this consistent with the rest of the file: