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

Framework: Adding the possibility to lock the editor #3686

Merged
merged 3 commits into from
Dec 6, 2017

Conversation

youknowriad
Copy link
Contributor

@youknowriad youknowriad commented Nov 28, 2017

This PR is the first one introducing the Block Templates Locking. (details on those here #3588 ).
Locking the editor means, you can't add new blocks, you can't move blocks, you can't remove blocks.

Testing instructions

  • Add A CPT like this:
function register_post_type() {
	$args = array(
		'public' => true,
		'label'  => 'Books',
		'show_in_rest' => true,
		'template' => array(
			array( 'core/image' ),
			array( 'core/paragraph', array(
				'placeholder' => 'Add a book description',
			) ),
			array( 'core/quote' ),
		),
		'template_lock' => 'all', // or 'insert' to allow moving
	);
	register_post_type( 'book', $args );
}
add_action( 'init', 'register_post_type' );
  • Create a new post with the registered CPT
  • The editor should initialize with the predefined list of blocks defined in this template.
  • You can't add/remove or move blocks.

Notes

  • I was wondering if I should disable the global HTML mode when the editor is locked
  • Hiding the buttons is easy enough, but it's easy to miss keyboard interactions

Next

  • We should add a validation step when opening existing posts with a locked editor. If the post doesn't match the template, we should raise a warning.

@youknowriad youknowriad self-assigned this Nov 28, 2017
@youknowriad youknowriad requested a review from mtias November 28, 2017 09:12
@codecov
Copy link

codecov bot commented Nov 28, 2017

Codecov Report

Merging #3686 into master will increase coverage by 1.04%.
The diff coverage is 5.63%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3686      +/-   ##
==========================================
+ Coverage   37.67%   38.71%   +1.04%     
==========================================
  Files         279      277       -2     
  Lines        6737     6901     +164     
  Branches     1226     1279      +53     
==========================================
+ Hits         2538     2672     +134     
- Misses       3538     3566      +28     
- Partials      661      663       +2
Impacted Files Coverage Δ
editor/components/inserter/index.js 0% <0%> (ø) ⬆️
blocks/library/paragraph/index.js 23.07% <0%> (ø) ⬆️
...mponents/editor-global-keyboard-shortcuts/index.js 0% <0%> (ø) ⬆️
blocks/library/heading/index.js 25% <0%> (ø) ⬆️
...ponents/block-settings-menu/block-delete-button.js 0% <0%> (ø) ⬆️
editor/components/block-switcher/index.js 2.7% <0%> (-0.16%) ⬇️
editor/components/block-list/block.js 0.72% <0%> (-1.51%) ⬇️
blocks/library/list/index.js 6.89% <0%> (ø) ⬆️
editor/components/block-mover/index.js 21.42% <15.38%> (+11.42%) ⬆️
editor/edit-post/modes/visual-editor/inserter.js 78.94% <40%> (-14.39%) ⬇️
... and 27 more

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 429fb42...eed266c. Read the comment docs.

@StaggerLeee
Copy link

Do not forget very variety of cases with partial lock.
I think it is in roadmap, just to be aware of it.

  • Lock blocks with CF7 shortcode, block with Gmap, (Admin decide) and still leave Users options to free add/edit/change Contact address, Info, text, etc...

@mtias mtias added the [Feature] Templates API Related to API powering block template functionality in the Site Editor label Nov 28, 2017
@youknowriad
Copy link
Contributor Author

@StaggerLeee Yes, that's on the roadmap. With block nesting, the implementation of these flexible layouts would not be so complex. Just add a "free" block list container and leave the rest locked.

@mtias
Copy link
Member

mtias commented Nov 28, 2017

@youknowriad nice work. I have been thinking that we might still want to allow moving blocks as an option instead of full lock, which might change the api to be:

// Locks everything, like current implementation
template_lock => 'all'

// Locks inserting and deleting, but allows moving the blocks and changing their order
template_lock => 'insert'

Too schematic? Would we need an array instead?

@youknowriad
Copy link
Contributor Author

@mtias Do you have an example of a use-case where we'd allow moving blocks but not inserting/moving? Honestly, I'm not sure this is something that would be heavily used.

Implementing it is easy enough though.

@youknowriad
Copy link
Contributor Author

Ok after a small discussion, I implemented the locking level using a string value.

@westonruter
Copy link
Member

(Note: I updated the description to fix a typo: s/register_book_type/register_post_type/g.)

@mtias
Copy link
Member

mtias commented Nov 29, 2017

Related: #2632

@youknowriad youknowriad force-pushed the try/block-template-locking branch 2 times, most recently from eed266c to b413329 Compare December 3, 2017 15:38
@youknowriad youknowriad force-pushed the try/block-template-locking branch from b413329 to 657c587 Compare December 6, 2017 08:25
@youknowriad youknowriad force-pushed the try/block-template-locking branch from 50104e6 to a16ef0c Compare December 6, 2017 08:53
@youknowriad
Copy link
Contributor Author

If not major concerns, I'll be merging this so we can prove it.

@youknowriad youknowriad merged commit 022ae30 into master Dec 6, 2017
@youknowriad youknowriad deleted the try/block-template-locking branch December 6, 2017 15:59
@mtias
Copy link
Member

mtias commented Dec 7, 2017

Nice work!

] );
} }
onSplit={
insertBlocksAfter ?
Copy link
Member

Choose a reason for hiding this comment

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

Should this be the responsibility of the block implementer to account for? Or could we just turn insertBlocksAfter and setAttributes into noop for a locked editor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Mmm, I didn't think about it. Sounds reasonable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Templates API Related to API powering block template functionality in the Site Editor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants