Skip to content

Enable post locking in Gutenberg#4217

Merged
youknowriad merged 119 commits intomasterfrom
feature/post-locking
Oct 4, 2018
Merged

Enable post locking in Gutenberg#4217
youknowriad merged 119 commits intomasterfrom
feature/post-locking

Conversation

@adamsilverstein
Copy link
Copy Markdown
Member

@adamsilverstein adamsilverstein commented Jan 1, 2018

Description

Enable the post locking feature in gutenberg:

Fixes #4331

  • uses the existing mechanism to set the post as locked.
  • works in both and between classic/gutenberg editors.
  • uses the existing core code, slightly refactored to pass Gutenberg linting.
  • show the locked dialog when opening a post that is already opened by another user.
  • show the takeover dialog shown when someone tries to take over a locked post.

How Has This Been Tested?

  • Create or edit a post.
  • Log in as a separate user and go to the posts screen, note the post is locked.
  • Test the three buttons: preview, take over and post list
  • Take over the post, make some edits without saving, then toggle back to the browser where the other user had the post open, this user will get an autosave fired and the 'another user has take over' dialog.

Screenshots (jpeg or gifs if applicable):


Types of changes

  • Add heartbeat.js in editor/utils, exporting setupHearthbeat
  • call setupHearthbeat before initial editor render.
  • monitor heartbeat for lock events - displaying a "user has taken over" modal when another user takes over the post
  • localize data for existing locks, displaying an "Already Locked" modal

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows has proper inline documentation.

@adamsilverstein adamsilverstein mentioned this pull request Jan 1, 2018
3 tasks
@adamsilverstein
Copy link
Copy Markdown
Member Author

Closing in favor of #4218 which builds on this and adds heartbeat based autosaves as well.

@gziolo gziolo deleted the feature/post-locking branch May 7, 2018 11:15
@adamsilverstein adamsilverstein restored the feature/post-locking branch June 8, 2018 16:35
@adamsilverstein
Copy link
Copy Markdown
Member Author

Reopening to work on standalone post locking since we took a different route with autosaves.

@adamsilverstein adamsilverstein self-assigned this Jul 27, 2018
@mtias mtias modified the milestones: Merge: Editor, 4.1 Oct 3, 2018
@youknowriad youknowriad modified the milestones: 4.1, 4.0 Oct 4, 2018
/**
* External dependencies
*/
import jQuery from 'jquery';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should add jquery as a dependency of the wp-editor script

This still needs to be done.

updatePostLock,
};
} ),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We're inconsistent with newlines between these members of the array. I'd recommend omitting them for broader consistency with the project.

@youknowriad youknowriad force-pushed the feature/post-locking branch from 1e140a0 to 240f3e7 Compare October 4, 2018 15:53
@karmatosed
Copy link
Copy Markdown
Member

Looks good to me. Thanks for all the hard work.

Copy link
Copy Markdown
Contributor

@mcsf mcsf left a comment

Choose a reason for hiding this comment

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

Added i18n feedback, which I'll address right now. (By typing it out I can refer to it later.)

@youknowriad youknowriad merged commit 37bd1e6 into master Oct 4, 2018
@youknowriad youknowriad deleted the feature/post-locking branch October 4, 2018 16:50
@youknowriad
Copy link
Copy Markdown
Contributor

Thanks for the hard work everyone. Now it's in we can enhance it with API endpoints at a more reasonable pace :)

@aduth
Copy link
Copy Markdown
Member

aduth commented Oct 4, 2018

Now it's in we can enhance it with API endpoints at a more reasonable pace :)

Is this tracked as an issue somewhere I can follow?

@danielbachhuber
Copy link
Copy Markdown
Member

Is this tracked as an issue somewhere I can follow?

There's https://core.trac.wordpress.org/ticket/44862

// https://developer.wordpress.org/plugins/javascript/heartbeat-api/
jQuery( document )
.on( 'heartbeat-send.refresh-lock', this.sendPostLock )
.on( 'heartbeat-tick.refresh-lock', this.receivePostLock );
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@adamsilverstein is there a reason we need to rely on jQuery here? could we have gotten by with addEventListener() (and for IE attachEvent)?

given the assumption that WordPress is already loaded jQuery seems free, but for external uses these few lines can pull in a huge dependency

thoughts?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should do similar as to what was done in #8311 as a short-term solution.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In fact, at least for the heartbeat-tick event, we should already have the action provided through #8311.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Pull request at #11781

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.