Improved pre/post publish flow. #5767

jorgefilipecosta · 2018-03-23T18:31:12Z

Fixes: #5343
Summary of button labels and flows as specified by @jasmussen.

Editors and admins:

Update — updates immediately, no pos-publish flow
Publish... — opens post-publish flow
Schedule... — opens post-publish flow
Contributors:

Publish... — opens post-publish flow where the confirm button says "Submit for Review".

How Has This Been Tested?

With an editor/admin:
Open a new post, write something, verify that "Publish..." button appears, press it, see the pre-post publish panel, click "Publish" and see the post-post publish panel. (basic flow)
Open a new post, write something, schedule it to the future, verify that "Schedule..." button appears, press it, see the pre-post publish panel, click "Schedule" and see post publish panel for scheduled posts. Props to @jasmussen for designing it.
Open an already Schedule post, e.g: the one before. Change it, see "Schedule" appears, press schedule and see post was updated no pre-publish panel was shown.
Open an already Schedule post, e.g: can still the one before. Change the scheduled date, for now, see the Publish button changed from "Schedule" to "Publish...", press Publish... verify pre-post publish panel appear and the normal publish flow happens.
Open a post submitted for review by a contributor, verily we see "Publish..." as publish button and the normal publish flow can be followed.

With a contributor:
Create a new post write something, see the publish button appears as "Publish...", press it, see the pre-publish panel appear, with the publish button text as "Submit for review". Press submit for review and verify the publish sidebar closes.
Open a post already submitted for review (e.g: the one before), verify the "Publish" button appears as "Submit for review" and pressing it does not show the pre-publish panel (again).

jasmussen · 2018-03-24T07:53:32Z

I LOOOOVE this! Thank you so much. This is a huge improvement.

Re: scheduling

Right now it's: Schedule... → (pre-publish) Schedule → Schedule → done

For publishing, it's Publish... → (pre-publish) Publish → Publishing... → post-publish → done

I'm missing here the post-publish flow for scheduled posts. I understand why this isn't there, but it totally could be. Here's what the normal post publish looks like:

But we could tune the post-publish a bit, to this:

The flow would then be: Schedule... → (pre-publish) Schedule → Scheduling... → done

(note the term "Scheduling..." when it's working)

What do you think?

I love this:

It totally works.

jorgefilipecosta · 2018-03-26T21:28:34Z

Hi @jasmussen thank you for the feedback. I updated this PR, now we show a panel for "pos schedule" with the content you specified.
Here is a screenshot of it:

jasmussen · 2018-03-27T05:52:42Z

Have a sick kid, so judging solely by the screenshot, and assuming that the candy-striped button says "Scheduling..." while it's working to schedule, I thin this is SOLID and good to go. 👍 👍

I think it's time to ship this! Thanks so much.

jasmussen · 2018-03-27T12:16:46Z

Nice work! It's 99% there from my perspective:

Can you make it say "Scheduling..." when it's working?

jorgefilipecosta · 2018-03-27T18:22:29Z

Can you make it say "Scheduling..." when it's working?

Hi @jasmussen I added a condition for that :)

jasmussen · 2018-03-27T18:24:46Z

Solid! 👍👍 from me and thank you!

gziolo

I left some minor comments to address before merging, but in general looks good to go 👍
This publish logic is full of conditions 😃

gziolo · 2018-03-28T05:14:49Z

editor/components/post-publish-button/label.js

@@ -37,6 +37,8 @@ export function PublishButtonLabel( {
 		return __( 'Publishing…' );
 	} else if ( isPublished && isSaving ) {
 		return __( 'Updating…' );
+	} else if ( isBeingScheduled && isSaving ) {


There are very simple corresponding tests which help to avoid regressions. Can we cover this case, too?

gziolo · 2018-03-28T05:23:31Z

editor/store/test/selectors.js

+			expect( isCurrentPostPending( state ) ).toBe( false );
+		} );
+
+		it( 'should return false for auto draft posts', () => {


The selector explicitly checks for status so it might be enough to keep only one from those 2 tests that verify against draft and auto-draft.

gziolo · 2018-03-28T05:24:26Z

editor/store/test/selectors.js

+			expect( isCurrentPostScheduled( state ) ).toBe( false );
+		} );
+
+		it( 'should return false for auto draft posts', () => {


Similar as above. One of the tests might be obsolete.

I followed your feedback and now we test for auto-draft in scheduled posts and for the normal draft's in pending posts so no tests testing the same thing exist in each test case but we still test for but status in the app.

gziolo · 2018-03-28T05:26:37Z

editor/components/post-publish-panel/postpublish.js

-						<Button className="button" href={ post.link }>
-							{ viewPostLabel }
-						</Button>
+						{ ! isScheduled &&


There is no Eslint rule for that but we usually wrap an element with parentheses when after && and is multiline.

gziolo · 2018-03-28T05:30:21Z

editor/components/post-publish-panel/index.js

+		const { user, onClose } = this.props;
+		const userCanPublishPosts = get( user.data, [ 'post_type_capabilities', 'publish_posts' ], false );
+		const isContributor = user.data && ! userCanPublishPosts;
+		if ( isContributor ) {


Nicely moved to one place. I hope we move that logic to selector one day. Not possible at the moment without a bigger refactor :)

Summary of button labels and flows: Editors and admins: Update — updates immediately, no pos-publish flow Publish... — opens post-publish flow Schedule... — opens post-publish flow Contributors: Publish... — opens post-publish flow where the confirm button says "Submit for Review".

gziolo · 2018-03-28T11:34:58Z

Thanks @jorgefilipecosta, great work 👍

aduth · 2018-09-12T15:01:31Z

editor/components/post-publish-panel/postpublish.js

 		return (
 			<div className="post-publish-panel__postpublish">
 				<PanelBody className="post-publish-panel__postpublish-header">
-					<a href={ post.link }>{ post.title || __( '(no title)' ) }</a>{ __( ' is now live.' ) }
+					<a href={ post.link }>{ post.title || __( '(no title)' ) }</a> { postPublishNonLinkHeader }


We should not concatenate translated strings, because the order of the concatenated parts can change in localization:

Use format strings instead of string concatenation—sprintf(__('Replace %1$s with %2$s'), $a, $b); is always better than __('Replace ').$a.__(' with ').$b; .

https://codex.wordpress.org/I18n_for_WordPress_Developers#Best_Practices

To preempt: It is unsolved this need to include React elements in translated strings. We can look to some other efforts as prior art:

https://github.com/Automattic/interpolate-components

But if we don't want to solve the larger problem, we're probably better off not localizing at all here.

We should definitely fix this — cc @jorgefilipecosta

I might take it upon myself to look at what would be needed for component interpolation.

Description of the issue / task at #9846

jorgefilipecosta self-assigned this Mar 23, 2018

jorgefilipecosta requested review from karmatosed and jasmussen March 23, 2018 18:31

atimmer mentioned this pull request Mar 26, 2018

Publicize: Adding to Gutenberg Extensibility for Publicize Support #5795

Closed

3 tasks

jorgefilipecosta force-pushed the update/pre-post-publish-flow branch from 9073c22 to 9ef0a71 Compare March 26, 2018 21:21

jorgefilipecosta force-pushed the update/pre-post-publish-flow branch from 9ef0a71 to a9086a0 Compare March 26, 2018 21:30

jorgefilipecosta force-pushed the update/pre-post-publish-flow branch from a9086a0 to fd23f5b Compare March 27, 2018 10:46

jorgefilipecosta requested a review from a team March 27, 2018 11:26

jorgefilipecosta force-pushed the update/pre-post-publish-flow branch from fd23f5b to a1365c7 Compare March 27, 2018 18:21

gziolo approved these changes Mar 28, 2018

View reviewed changes

jorgefilipecosta force-pushed the update/pre-post-publish-flow branch from a1365c7 to ffc742a Compare March 28, 2018 10:43

jorgefilipecosta merged commit 464b8c6 into master Mar 28, 2018

jorgefilipecosta deleted the update/pre-post-publish-flow branch March 28, 2018 11:07

gziolo mentioned this pull request Mar 29, 2018

No publish button #5874

Closed

aduth reviewed Sep 12, 2018

View reviewed changes

aduth mentioned this pull request Sep 12, 2018

i18n: Implement a solution for interpolation of React elements in strings #9846

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improved pre/post publish flow. #5767

Improved pre/post publish flow. #5767

jorgefilipecosta commented Mar 23, 2018 •

edited

Loading

jasmussen commented Mar 24, 2018

jorgefilipecosta commented Mar 26, 2018 •

edited

Loading

jasmussen commented Mar 27, 2018

jasmussen commented Mar 27, 2018

jorgefilipecosta commented Mar 27, 2018

jasmussen commented Mar 27, 2018

gziolo left a comment

gziolo Mar 28, 2018

gziolo Mar 28, 2018

gziolo Mar 28, 2018

jorgefilipecosta Mar 28, 2018

gziolo Mar 28, 2018

gziolo Mar 28, 2018

gziolo commented Mar 28, 2018

aduth Sep 12, 2018 •

edited

Loading

aduth Sep 12, 2018

mcsf Sep 12, 2018

aduth Sep 12, 2018

aduth Sep 12, 2018

Improved pre/post publish flow. #5767

Improved pre/post publish flow. #5767

Conversation

jorgefilipecosta commented Mar 23, 2018 • edited Loading

How Has This Been Tested?

jasmussen commented Mar 24, 2018

jorgefilipecosta commented Mar 26, 2018 • edited Loading

jasmussen commented Mar 27, 2018

jasmussen commented Mar 27, 2018

jorgefilipecosta commented Mar 27, 2018

jasmussen commented Mar 27, 2018

gziolo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gziolo commented Mar 28, 2018

aduth Sep 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorgefilipecosta commented Mar 23, 2018 •

edited

Loading

jorgefilipecosta commented Mar 26, 2018 •

edited

Loading

aduth Sep 12, 2018 •

edited

Loading