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

feat: workflow unpublished entry #2914

Merged
merged 4 commits into from
Nov 26, 2019
Merged

Conversation

barthc
Copy link
Contributor

@barthc barthc commented Nov 25, 2019

Summary
Closes #727

Currently swap the editorial toolbar section from:
unpublish-before

to:
unpublish-after

cc: @tomrutgers

@erezrokah erezrokah self-requested a review November 25, 2019 13:09
@tomrutgers
Copy link
Contributor

I'm not entirely sure about the color scheme for the button, I associate green with actions like publishing, not unpublishing. Maybe we should use a gray version.

I prefer this one
button-gray

Over the default:
button-gray-default

The other colored buttons used are a bit harsh IMHO.
colored-buttons

Thoughts?

@barthc
Copy link
Contributor Author

barthc commented Nov 25, 2019

If we are going gray for unpublish , then we also need to modify the color scheme for buttons save ,Delete published entry, etc just to be consistent.

@tomrutgers
Copy link
Contributor

Why? It's already a different color button..

Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

Nice work @barthc.
I'm going over it now

@@ -63,6 +64,8 @@ const en = {
publishing: 'Publishing...',
publish: 'Publish',
published: 'Published',
unpublish: 'Unpublish entry',
unpublishing: 'Unpublishing',
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be Unpublishing... similar to Publishing... right?

@erezrokah
Copy link
Contributor

How about:

image

Or:

image

@@ -63,6 +64,8 @@ const en = {
publishing: 'Publishing...',
publish: 'Publish',
published: 'Published',
unpublish: 'Unpublish entry',
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use just Unpublish like Publish and create new or Publish now?

@erezrokah
Copy link
Contributor

@barthc looks good pending UI decisions. Also do you mind adding a scenario for unpublishing for the test backend in the Cypress tests?

@tomrutgers
Copy link
Contributor

@erezrokah Digging that first one!

@erezrokah
Copy link
Contributor

@erezrokah Digging that first one!

Just thinking we should be consistent with one of these:
image

@tomrutgers
Copy link
Contributor

Even better!

@barthc
Copy link
Contributor Author

barthc commented Nov 25, 2019

@erezrokah honestly the dropdown unpublish was my first approach but later changed it 😀

@tomrutgers
Copy link
Contributor

That settles it then 😄

@erezrokah erezrokah force-pushed the feat/workflow-unpublish-post branch from be5ba4f to 72a0bb8 Compare November 26, 2019 10:00
@erezrokah
Copy link
Contributor

erezrokah commented Nov 26, 2019

@tomrutgers, @barthc I rebased the branch, made the UI changes, added a cypress test and a fix to re-load the unpublished entry (was needed due to this recent change: https://github.com/netlify/netlify-cms/pull/2878/files#diff-7d0283827744acf7472c54d35c205500R305).

@barthc
Copy link
Contributor Author

barthc commented Nov 26, 2019

@erezrokah was about to push changes for the dropdown and the cypress test 😀

@tomrutgers
Copy link
Contributor

Looking good!

@erezrokah
Copy link
Contributor

erezrokah commented Nov 26, 2019

@barthc Sorry for that, I'm pushing for keeping the PR number low and we have some more changes coming around that area :)

@erezrokah erezrokah merged commit 41bb9aa into master Nov 26, 2019
@barthc barthc deleted the feat/workflow-unpublish-post branch November 26, 2019 10:50
nathankitchen pushed a commit to nathankitchen/netlify-cms-backend-azure that referenced this pull request Feb 24, 2020
* feat: workflow unpublished entry

* fix: post rebase fix - load unpublished entry after unpublish

* feat: change unpublish button to dropdown

* test(cypress): add unpublish entry cypress test
vladdu pushed a commit to vladdu/netlify-cms that referenced this pull request Jan 26, 2021
* feat: workflow unpublished entry

* fix: post rebase fix - load unpublished entry after unpublish

* feat: change unpublish button to dropdown

* test(cypress): add unpublish entry cypress test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unpublishing for the editorial workflow
3 participants