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.

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

Add "switch to draft" button in publish flow dropdown. #3718

Merged
merged 1 commit into from
Nov 29, 2017

Conversation

mtias
Copy link
Member

@mtias mtias commented Nov 29, 2017

This adds a button to switch a published post to draft form. See #1452.

image

@mtias mtias added the [Status] In Progress Tracking issues with work in progress label Nov 29, 2017
@@ -22,7 +22,7 @@ import {
} from '../../selectors';

function PostPublishWithDropdown( { isSaving, isPublishable, isSaveable, isPublished } ) {
const isButtonEnabled = ! isSaving && isPublishable && isSaveable;
const isButtonEnabled = ! isSaving && isPublishable && isSaveable || isPublished;
Copy link
Member

Choose a reason for hiding this comment

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

[0] /home/travis/build/WordPress/gutenberg/editor/components/post-publish-with-dropdown/index.js
[0]   25:54  error  Unexpected mix of '&&' and '||'  no-mixed-operators
[0]   25:68  error  Unexpected mix of '&&' and '||'  no-mixed-operators

Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Should I expect this to actually perform the action of reverting the post to draft?

@@ -41,7 +41,7 @@ function PostPublishWithDropdown( { isSaving, isPublishable, isSaveable, isPubli
<Dashicon icon="arrow-down" />
</Button>
) }
renderContent={ ( { onClose } ) => <PostPublishDropdown onSubmit={ onClose } /> }
renderContent={ ( { onClose } ) => <PostPublishDropdown onSubmit={ onClose } showSwitchToDraft={ isPublished } /> }
Copy link
Member

Choose a reason for hiding this comment

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

Might be good to split to multiple lines.

@mtias
Copy link
Member Author

mtias commented Nov 29, 2017

Should I expect this to actually perform the action of reverting the post to draft?

No, putting it here as a mockup for discussing with Joen. @mcsf is looking at making it into a component like PublishButton.

@codecov
Copy link

codecov bot commented Nov 29, 2017

Codecov Report

Merging #3718 into master will decrease coverage by 0.16%.
The diff coverage is 36.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3718      +/-   ##
==========================================
- Coverage   37.55%   37.38%   -0.17%     
==========================================
  Files         277      278       +1     
  Lines        6710     6775      +65     
  Branches     1223     1242      +19     
==========================================
+ Hits         2520     2533      +13     
- Misses       3531     3566      +35     
- Partials      659      676      +17
Impacted Files Coverage Δ
editor/components/post-publish-dropdown/index.js 0% <ø> (ø) ⬆️
...tor/components/post-publish-with-dropdown/index.js 0% <0%> (ø) ⬆️
...or/components/post-switch-to-draft-button/index.js 16.66% <16.66%> (ø)
editor/effects.js 57.57% <50%> (-1.3%) ⬇️
blocks/autocompleters/index.js 21.62% <0%> (-20.49%) ⬇️
editor/components/post-publish-button/label.js 88.23% <0%> (+0.73%) ⬆️
blocks/editable/index.js 11.55% <0%> (+1.44%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 50cdcf4...27f6f97. Read the comment docs.

@mcsf
Copy link
Contributor

mcsf commented Nov 29, 2017

Basic working state can be tested now:

gutenberg-switch-to-draft

Questions:

  1. Should we get rid of the Post updated! View post notice when the post is reverted to Draft? Or at least change the wording, as it might be confusing.
  2. When Switch to Draft is pressed that button disappears, but progress is still made apparent by the Publish/Update button next to it. Should Switch to Draft still linger (as a disabled button, potentially with the loading animation) while we wait for the server?

@mtias
Copy link
Member Author

mtias commented Nov 29, 2017

(as a disabled button, potentially with the loading animation) while we wait for the server?

This makes sense to me. Unless the dropdown menu disappears immediately.

Should we get rid of the Post updated! View post notice when the post is reverted to Draft? Or at least change the wording, as it might be confusing.

This one should be more intelligent in general. Changing status should not show "view".

@jasmussen
Copy link
Contributor

I would concur with Matías on his thoughts on that. The flow overall is receiving more thought separately, but with those two changes, we'll have something good I think.

@mcsf mcsf force-pushed the add/switch-to-draft branch from 0ca568e to 8f25618 Compare November 29, 2017 16:33
@mcsf
Copy link
Contributor

mcsf commented Nov 29, 2017

@mtias, @jasmussen: thanks for your notes.

This one should be more intelligent in general. Changing status should not show "view".

Taken care of.

The flow overall is receiving more thought separately, but with those two changes, we'll have something good I think.

Per Slack, I think this is good to merge as-is, so we can test the usability of the dropdown as it stands and tweak if needed.

@mtias mtias added [Feature] Document Settings Document settings experience and removed [Status] In Progress Tracking issues with work in progress labels Nov 29, 2017
@mcsf mcsf force-pushed the add/switch-to-draft branch from 8f25618 to 27f6f97 Compare November 29, 2017 16:40
@@ -0,0 +1,5 @@
.editor-post-publish-dropdown__publish-button-container {
.editor-post-publish-dropdown__switch-to-draft {
Copy link
Member

Choose a reason for hiding this comment

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

Is the nesting necessary?

Copy link
Contributor

Choose a reason for hiding this comment

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

The nesting per se isn't, but the added specificity is.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, needed the core-ui thing otherwise :(

@mcsf mcsf merged commit 77e8360 into master Nov 29, 2017
@mcsf mcsf deleted the add/switch-to-draft branch November 29, 2017 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Document Settings Document settings experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants