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

Swap position of the Pre-publish checks buttons. #65317

Merged
merged 2 commits into from
Sep 19, 2024
Merged

Swap position of the Pre-publish checks buttons. #65317

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
39f5a7c
Swap position of the Pre-publish checks buttons.
afercia Sep 13, 2024
411656b
Adjust test.
afercia Sep 13, 2024
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
19 changes: 1 addition & 18 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 @@ -2,7 +2,7 @@
* WordPress dependencies
*/
import { Button } from '@wordpress/components';
import { Component, createRef } from '@wordpress/element';
import { Component } from '@wordpress/element';
import { withSelect, withDispatch } from '@wordpress/data';
import { compose } from '@wordpress/compose';

Expand All @@ -17,7 +17,6 @@ const noop = () => {};
export class PostPublishButton extends Component {
constructor( props ) {
super( props );
this.buttonNode = createRef();

this.createOnClick = this.createOnClick.bind( this );
this.closeEntitiesSavedStates =
Expand All @@ -28,21 +27,6 @@ export class PostPublishButton extends Component {
};
}

componentDidMount() {
if ( this.props.focusOnMount ) {
// This timeout is necessary to make sure the `useEffect` hook of
// `useFocusReturn` gets the correct element (the button that opens the
// PostPublishPanel) otherwise it will get this button.
this.timeoutID = setTimeout( () => {
this.buttonNode.current.focus();
}, 0 );
}
}

componentWillUnmount() {
clearTimeout( this.timeoutID );
}

createOnClick( callback ) {
return ( ...args ) => {
const { hasNonPostEntityChanges, setEntitiesSavedStatesCallback } =
Expand Down Expand Up @@ -182,7 +166,6 @@ export class PostPublishButton extends Component {
return (
<>
<Button
ref={ this.buttonNode }
{ ...componentProps }
className={ `${ componentProps.className } editor-post-publish-button__button` }
size="compact"
Expand Down
30 changes: 22 additions & 8 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 @@ -2,7 +2,7 @@
* WordPress dependencies
*/
import { __ } from '@wordpress/i18n';
import { Component } from '@wordpress/element';
import { Component, createRef } from '@wordpress/element';
import {
Button,
Spinner,
Expand All @@ -27,6 +27,20 @@ export class PostPublishPanel extends Component {
constructor() {
super( ...arguments );
this.onSubmit = this.onSubmit.bind( this );
this.cancelButtonNode = createRef();
}

componentDidMount() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated: Haha, looks like these are very old components, some of the few that are still using the component class. Unrelated, but we should refactor this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I did notice that.

Copy link
Contributor

@ciampo ciampo Sep 19, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Wow, I haven't seen a componentDidMount in a while 😂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

45 occurrences of componentDidMount() in the current codebase 😉

// This timeout is necessary to make sure the `useEffect` hook of
// `useFocusReturn` gets the correct element (the button that opens the
// PostPublishPanel) otherwise it will get this button.
this.timeoutID = setTimeout( () => {
this.cancelButtonNode.current.focus();
}, 0 );
}

componentWillUnmount() {
clearTimeout( this.timeoutID );
}

componentDidUpdate( prevProps ) {
Expand Down Expand Up @@ -85,15 +99,9 @@ export class PostPublishPanel extends Component {
/>
) : (
<>
<div className="editor-post-publish-panel__header-publish-button">
<PostPublishButton
focusOnMount
onSubmit={ this.onSubmit }
forceIsDirty={ forceIsDirty }
/>
</div>
<div className="editor-post-publish-panel__header-cancel-button">
<Button
ref={ this.cancelButtonNode }
accessibleWhenDisabled
disabled={ isSavingNonPostEntityChanges }
onClick={ onClose }
Expand All @@ -103,6 +111,12 @@ export class PostPublishPanel extends Component {
{ __( 'Cancel' ) }
</Button>
</div>
<div className="editor-post-publish-panel__header-publish-button">
<PostPublishButton
onSubmit={ this.onSubmit }
forceIsDirty={ forceIsDirty }
/>
</div>
</>
) }
</div>
Expand Down
4 changes: 2 additions & 2 deletions packages/editor/src/components/post-publish-panel/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,12 @@
}

.editor-post-publish-panel__header-publish-button {
padding-right: $grid-unit-05;
padding-left: $grid-unit-05;
justify-content: center;
}

.editor-post-publish-panel__header-cancel-button {
padding-left: $grid-unit-05;
padding-right: $grid-unit-05;
}

.editor-post-publish-panel__header-published {
Expand Down
42 changes: 21 additions & 21 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 @@ -433,24 +433,24 @@ exports[`PostPublishPanel should render the pre-publish panel if post status is
class="editor-post-publish-panel__header"
>
<div
class="editor-post-publish-panel__header-publish-button"
class="editor-post-publish-panel__header-cancel-button"
>
<button
aria-disabled="true"
class="components-button editor-post-publish-button editor-post-publish-button__button is-primary is-compact"
class="components-button is-secondary is-compact"
type="button"
>
Submit for Review
Cancel
</button>
</div>
<div
class="editor-post-publish-panel__header-cancel-button"
class="editor-post-publish-panel__header-publish-button"
>
<button
class="components-button is-secondary is-compact"
aria-disabled="true"
class="components-button editor-post-publish-button editor-post-publish-button__button is-primary is-compact"
type="button"
>
Cancel
Submit for Review
</button>
</div>
</div>
Expand Down Expand Up @@ -586,24 +586,24 @@ exports[`PostPublishPanel should render the pre-publish panel if the post is not
class="editor-post-publish-panel__header"
>
<div
class="editor-post-publish-panel__header-publish-button"
class="editor-post-publish-panel__header-cancel-button"
>
<button
aria-disabled="true"
class="components-button editor-post-publish-button editor-post-publish-button__button is-primary is-compact"
class="components-button is-secondary is-compact"
type="button"
>
Submit for Review
Cancel
</button>
</div>
<div
class="editor-post-publish-panel__header-cancel-button"
class="editor-post-publish-panel__header-publish-button"
>
<button
class="components-button is-secondary is-compact"
aria-disabled="true"
class="components-button editor-post-publish-button editor-post-publish-button__button is-primary is-compact"
type="button"
>
Cancel
Submit for Review
</button>
</div>
</div>
Expand Down Expand Up @@ -783,24 +783,24 @@ exports[`PostPublishPanel should render the spinner if the post is being saved 1
class="editor-post-publish-panel__header"
>
<div
class="editor-post-publish-panel__header-publish-button"
class="editor-post-publish-panel__header-cancel-button"
>
<button
aria-disabled="true"
class="components-button editor-post-publish-button editor-post-publish-button__button is-primary is-compact"
class="components-button is-secondary is-compact"
type="button"
>
Submit for Review
Cancel
</button>
</div>
<div
class="editor-post-publish-panel__header-cancel-button"
class="editor-post-publish-panel__header-publish-button"
>
<button
class="components-button is-secondary is-compact"
aria-disabled="true"
class="components-button editor-post-publish-button editor-post-publish-button__button is-primary is-compact"
type="button"
>
Cancel
Submit for Review
</button>
</div>
</div>
Expand Down
4 changes: 2 additions & 2 deletions test/e2e/specs/editor/various/publish-panel.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ test.describe( 'Post publish panel', () => {
).toBeFocused();
} );

test( 'should move focus to the publish button in the panel', async ( {
test( 'should move focus to the cancel button in the panel', async ( {
editor,
page,
} ) => {
Expand All @@ -74,7 +74,7 @@ test.describe( 'Post publish panel', () => {
page
.getByRole( 'region', { name: 'Editor publish' } )
.locator( ':focus' )
).toHaveText( 'Publish' );
).toHaveText( 'Cancel' );
} );

test( 'should focus on the post list after publishing', async ( {
Expand Down
Loading