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

Enable Heartbeat Post Locking #4331

Closed
adamsilverstein opened this issue Jan 5, 2018 · 13 comments
Closed

Enable Heartbeat Post Locking #4331

adamsilverstein opened this issue Jan 5, 2018 · 13 comments
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Feature] Saving Related to saving functionality [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Enhancement A suggestion for improvement. [Type] Task Issues or PRs that have been broken down into an individual action to take

Comments

@adamsilverstein
Copy link
Member

Issue Overview

Enable heartbeat based post locking functionality.

Work with the existing editor functionality for post locking. Enhance when collaborative editing is available by offering 'Collaborate' as an additional option in the takeover modal and possibly post list options.

Expected Behavior

  • Post locking should work similar to or better than the classic editor's post locking feature: https://codex.wordpress.org/Post_Locking as well as respecting any locking of posts in the classic editor
  • Posts that are being edited are locked to prevent multiple editors overwriting each others changes
  • When you try to open the post the editor is disabled and a modal offers 'go back', 'preview' and 'take over', plus 'collaborate' if collaborative editing is available - the editor remains inactive/inaccessible unless you choose to take over or collaborate.
  • When you are editing a post and another user takes over, a similar modal pops up. Currently you can no longer edit, the only option is to return to the posts list screen. Perhaps this should instead offer collaborate or take over?

Testing

  • Create and edit a post.
  • In a separate browser, log in with a different user. Check post list screen and look for the post lock indicators.
  • Test in the classic editor and Gutenberg.

Current Behavior

  • Posts do not auto lock.
  • Opening a locked post from the classic editor in Gutenberg does not respect the post lock.
  • Opening a locked post in the classic editor with the Gutenberg plugin active and choosing to take over the post does not work correctly - instead of the classic editor, I am taken to the Gutenberg editor.

Screenshots

Some screenshots from the classic editor:

post list lock indicator:

takeover modal, note editor is disabled:

another user took over the post being edited modal, note editor is disabled:

Related Issues and/or PRs

#4218

@danielbachhuber danielbachhuber added [Type] Enhancement A suggestion for improvement. Backwards Compatibility Issues or PRs that impact backwards compatability labels Jan 23, 2018
@jeffpaul jeffpaul added this to the Merge Proposal milestone Mar 24, 2018
@karmatosed karmatosed modified the milestones: Merge Proposal, Merge Proposal: Back Compat Apr 12, 2018
@aaronjorbin
Copy link
Member

Ran into this today. One thing we should make sure is that the post locking works if someone is in the classic editor as well. While unlikely after it's merged into core, it's entirely possible a classic editor or ramping type plugin could cause both editors to continue to exist.

@adamsilverstein
Copy link
Member Author

One thing we should make sure is that the post locking works if someone is in the classic editor as well.

This will work correctly as long as we use the same mechanism for locking for Gutenberg as for the classic editor. I tested this in my original PR (#4217) that used the heartbeat API and included post locking by opening a post in Gutenberg, then trying to open in the classic editor and the other way around.

I reopened my PR and will refresh and re-test to confirm.

@grappler
Copy link
Member

grappler commented Jul 6, 2018

I feel this feature should be added before 4.9.8 is released. Loosing changes could easily ruin people's experience with Gutenberg.

@adamsilverstein
Copy link
Member Author

I'll update the PR.

@adamsilverstein adamsilverstein added the Needs Design Feedback Needs general design feedback. label Jul 6, 2018
@adamsilverstein
Copy link
Member Author

@karmatosed from a design perspective, do you think an overlay and modal like we have in core currently is appropriate for the takeover interaction (see screenshots above). Or, do you have some other ideas howe we can implement these interactions: when a user opens a post another user has locked and when another user takes over editing of a post you are editing. Do we have an existing method/instance where we lock Gutenberg and/or display an overlay? Media?

@adamsilverstein
Copy link
Member Author

Ah, maybe the new Modal component: #6261

@karmatosed
Copy link
Member

Great idea @adamsilverstein, here is my suggestion. I do wonder if it has to be a primary button to view posts or a link will do - happy to go with whatever works there, my leaning is towards a link.

artboard 2

@karmatosed
Copy link
Member

Here is what the 3 buttons look like. I kind of wonder if there should be 3 buttons or links, I don't want to suggest a change in this that causes issues though. I did wonder about the order they are in, but again let's see maybe after about that, it's important to get it working for people.

artboard 2

@karmatosed karmatosed removed the Needs Design Feedback Needs general design feedback. label Jul 15, 2018
@danielbachhuber danielbachhuber added the [Feature] Saving Related to saving functionality label Aug 1, 2018
@adamsilverstein
Copy link
Member Author

@karmatosed - I posted updated code for this feature in #4217. Looking now I see I didn't do very well trying to match your design mocks with the current modal component because unlike your mocks, the modal includes a title area with a subtle divider line, then a content area below. I wound up putting the gravatar and text in the top (icon/title) section and the buttons in the "content" section.

You can see some screenshots of how it looks on the PR. Looking back at your mocks they really look much better, so maybe I should take another swing at it and add some flexibility into the modal to add a conditional to hide the divider or the title row/section entirely? I already added a conditional to remove the close button because these modals can't be closed directly. I can also try reworking the primary button into a link as you suggested, which will probably look better.

@adamsilverstein
Copy link
Member Author

Ok, I fixed the PR so the UI more closely match the designs you presented here @karmatosed and posted updated screenshots: #4217 (comment)

Could still use a little CSS love to make it look great.

@afercia afercia added the [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). label Aug 20, 2018
@davisshaver
Copy link
Contributor

+1 to merging this before 4.9.8. Important if not critical for high volume publishing.

@eddhurst
Copy link

Wanted to add a +1 to this. Incredibly important to at least trigger the lock as soon as an editor opens the page to prevent unexpected loss of data.

@grappler
Copy link
Member

grappler commented Oct 3, 2018

With WordPress 5.0 RC expected in a month I am getting anxious that this is not going to be in Gutenberg in time for WordPress 5.0 release which would not be good.

@mtias mtias added the [Type] Task Issues or PRs that have been broken down into an individual action to take label Oct 3, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backwards Compatibility Issues or PRs that impact backwards compatability [Feature] Saving Related to saving functionality [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Enhancement A suggestion for improvement. [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

No branches or pull requests