Limit of Next Page (Page Break) block to root level only #18260

glendaviesnz · 2019-11-04T08:28:44Z

Description

Adds a new 'root' parent block type which allows a block to be limited to only insertion in the root context. This is initially as a suggested fix for a bug with the Page Break block ( #16411 ) but could be documented as a way to limit any block to the root context.

How has this been tested

Just tested manually at this stage. If it is agreed that this is a possible way forward I will look at adding some unit or e2e tests.

To test

Check out this branch
Add a Page Break block to the editor root - should work as normal
Try adding a Page Break block to a block that allows nesting like Columns - it should not appear as an available block to add

Screenshots

… to root level only

glendaviesnz · 2019-11-11T08:59:50Z

This seems to work, and doesn't break any existing tests, but I am sure I am overlooking some gotchas, or an easier existing way to limit a block to the root context.

mmtr · 2019-11-11T11:49:30Z

This seems to work, and doesn't break any existing tests, but I am sure I am overlooking some gotchas, or an easier existing way to limit a block to the root context.

I think this is a valid approach! Simple and easy 🙂

kwight · 2019-11-11T18:46:17Z

Works really well 👍

ellatrix · 2019-11-28T21:51:51Z

packages/block-editor/src/store/selectors.js

@@ -1036,6 +1036,9 @@ const canInsertBlockTypeUnmemoized = ( state, blockName, rootClientId = null ) =
 			return list;
 		}
 		if ( isArray( list ) ) {
+			if ( includes( list, 'root' ) && item === null ) {


How would it work if it's not a post, in case of full site editing? Maybe not something we should fix now, but maybe good to have in mind. Cc @epiqueras.

It would only let you insert it at the root level of the template.

I guess we would need to add core/post-content to the list of allowed parents? Along with any other template layout blocks that should be able to insert a page break.

Could we also add a list of parents that should be considered as a 'root' container at this point, rather than having to add them to each blocks allowed parents? eg.

const rootContainers = [ null, 'core/post-content ]; if ( includes( list, 'root' ) && includes( rootContainers , item ) )

Sure, but won't the page break still break if its rendered in a Post Content block? Or does it only break in Column blocks?

I just used Post Content block as an example because you had mentioned it - the array would just contain any parent blocks that we knew should be treated like a root element for the likes of page breaks.

It may be that it is not possible to identify what constitutes a 'root' block for a range of different child blocks, in which case it is probably best to leave 'root' purely as the editor root with if ( includes( list, 'root' ) && item === null ), and add any other possible parent blocks to the blocks own allowed parents array.

It may be that it is not possible to identify what constitutes a 'root' block for a range of different child blocks, in which case it is probably best to leave 'root' purely as the editor root with if ( includes( list, 'root' ) && item === null ), and add any other possible parent blocks to the blocks own allowed parents array.

A "root" block shouldn't be defined in relationship to a range of child blocks. A "root" block is simply a block that serves as the root of the block tree in some rendering context. For example, a Post Content block serves as the root of the block tree when rendering a single post, but doesn't when rendering an entire template. So I think we are safe with including Post Content in our definition of "root".

I just used Post Content block as an example because you had mentioned it - the array would just contain any parent blocks that we knew should be treated like a root element for the likes of page breaks.

Yeah, I understood that. I was just wondering if the Page Break block will cause issues as a child of the Post Content block. I think it should be fine.

Now that I think more about this. The Page Break block only makes sense as a top level block in post content. If we whitelist this using "root", you would be able to add them to templates and template parts which wouldn't make any sense or work.

To make this work, the selector needs to check that the block is being inserted at the root level and that we are editing a post, or that it's being inserted into a Post Content block.

that we are editing a post

We don't have a canonical way of doing this yet so I think that the way to move forward is using

parent: [ 'core/post-content' ]

and

if ( includes( list, 'core/post-content' ) && item === null ) {

for now. With a todo note to change it to

if ( includes( list, 'core/post-content' ) && ( item === 'core/post-content' || (getEditorMode() === 'post-content' && item === null) ) {

when possible.

That approach makes sense to me - have made this change, but haven't had time to test it properly - will do that tomorrow hopefully.

Looks good to me.

We'll actually only need to change it to includes( list, 'core/post-content' ) && getEditorMode() === 'post-content' && item === null, because the following return includes( list, item ); will take care of the other case where item === 'core/post-content'.

Have updated the TODO comment. Will try and get a unit test added in the next day or so.

@epiqueras, finally got back to this, have added a basic unit test to cover this.

packages/block-editor/src/store/selectors.js

packages/block-editor/src/store/test/selectors.js

epiqueras · 2019-12-09T18:20:24Z

packages/block-editor/src/store/test/selectors.js

+				blocks: {
+					byClientId: {},
+					attributes: {
+						block1: {},


This is unnecessary.

epiqueras · 2019-12-09T18:21:08Z

packages/block-editor/src/store/test/selectors.js

@@ -121,6 +121,15 @@ describe( 'selectors', () => {
 			},
 		} );

+		registerBlockType( 'core/post-content-child', {
+			save: ( props ) => props.attributes.text,


This block doesn't have attributes.

Co-Authored-By: Enrique Piqueras <epiqueras@users.noreply.github.com>

glendaviesnz · 2019-12-09T21:58:58Z

@epiqueras - apologies for the basic formatting issues, my local linting doesn't seem to be working and hadn't noticed.

epiqueras

Thanks!

CreativeDive · 2023-04-07T14:48:08Z

@glendaviesnz @youknowriad I tested this behavior with WP 6.2 and Gutenberg 15.5.0 and it no longer works. If the block has "core/post-content" set in the "parent" attribute in block.json, the block can be grouped. But that shouldn't be possible.

I have other custom blocks that are restricted to the root level of the editor, but the options to group these blocks are available. But that shouldn't be possible.

It would be nice if it works as expected again.

youknowriad · 2023-04-07T15:47:02Z

@CreativeDive would you mind creating an issue about it, so we don't forget about it.

CreativeDive · 2023-04-07T16:02:48Z

Created a new issue #49662

youknowriad · 2023-04-07T20:12:35Z

Thanks 👍

WIP - playing around with options to allow limiting of nextpage block…

e1df80f

… to root level only

glendaviesnz mentioned this pull request Nov 4, 2019

Page Break Block: should only be insertable at root level #16411

Closed

glendaviesnz changed the title ~~WIP - Limit of next page block to root level only~~ Limit of Next Page (Page Break) block to root level only Nov 11, 2019

glendaviesnz marked this pull request as ready for review November 11, 2019 08:58

glendaviesnz requested review from ajitbohra, ellatrix, jorgefilipecosta, Soean, talldan and youknowriad as code owners November 11, 2019 08:58

mmtr assigned glendaviesnz Nov 15, 2019

ellatrix reviewed Nov 28, 2019

View reviewed changes

glendaviesnz added [Type] Bug An existing feature does not function as intended [Block] More Affects the More Block - used for displaying the 'Read More' link [Block] Page Break Affects the Page Break Block labels Dec 3, 2019

Changes from code review

c86525f

glendaviesnz added the [Feature] Block API API that allows to express the block paradigm. label Dec 3, 2019

Glen Davies added 2 commits December 5, 2019 10:29

Update todo comment

084e277

Add unit test

16e78fc

epiqueras requested changes Dec 9, 2019

View reviewed changes

glendaviesnz and others added 5 commits December 9, 2019 13:49

Update packages/block-editor/src/store/selectors.js

8a2bdf5

Co-Authored-By: Enrique Piqueras <epiqueras@users.noreply.github.com>

Fix linting issue

82e2131

Co-Authored-By: Enrique Piqueras <epiqueras@users.noreply.github.com>

Fix linting issue

6a9f656

Co-Authored-By: Enrique Piqueras <epiqueras@users.noreply.github.com>

Fix linting issue

bdc4fec

Co-Authored-By: Enrique Piqueras <epiqueras@users.noreply.github.com>

Changes from review

febb426

glendaviesnz requested a review from epiqueras December 9, 2019 21:59

epiqueras approved these changes Dec 9, 2019

View reviewed changes

epiqueras merged commit 2ebfb64 into WordPress:master Dec 9, 2019

glendaviesnz deleted the fix/page-break-as-child-bug branch December 9, 2019 23:18

youknowriad added this to the Gutenberg 7.2 milestone Jan 6, 2020

CreativeDive mentioned this pull request Apr 7, 2023

BUG: Restricted Blocks to root level via parent "core/post-content" are nestable and groupable again #49662

Open

naiba4 mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 8.57.0 to 9.0.0 naiba4/gutenberg#52

Open

MaxMood96 mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 8.57.0 to 9.0.0 MaxMood96/gutenberg#151

Open

jay-Anonymous mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 8.57.0 to 9.0.0 jay-Anonymous/gutenberg#8

Open

monzerfoda mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 4.19.1 to 9.0.0 monzerfoda/gutenberg#23

Open

soloinovator mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 8.57.0 to 9.0.0 soloinovator/gutenberg#137

Open

yassine4999 mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 8.57.0 to 9.0.0 yassine4999/gutenberg#41

Open

petrescugit mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 8.57.0 to 9.0.0 petrescugit/gutenberg#19

Open

yaseenaljamal mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 8.57.0 to 9.0.0 yaseenaljamal/gutenberg#20

Open

makbarGroup mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 4.19.1 to 9.0.0 makbarGroup/gutenberg#59

Open

npneeraj mentioned this pull request Apr 6, 2024

[Snyk] Security upgrade eslint from 8.57.0 to 9.0.0 npneeraj/gutenberg#35

Open

This was referenced Apr 6, 2024

[Snyk] Security upgrade eslint from 7.32.0 to 9.0.0 Verdinjoshua26/gutenberg#114

Open

[Snyk] Fix for 2 vulnerabilities Verdinjoshua26/gutenberg#115

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Limit of Next Page (Page Break) block to root level only #18260

Limit of Next Page (Page Break) block to root level only #18260

glendaviesnz commented Nov 4, 2019 •

edited

Loading

glendaviesnz commented Nov 11, 2019

mmtr commented Nov 11, 2019

kwight commented Nov 11, 2019

ellatrix Nov 28, 2019

epiqueras Nov 28, 2019

glendaviesnz Dec 2, 2019

epiqueras Dec 2, 2019

glendaviesnz Dec 2, 2019

epiqueras Dec 2, 2019

glendaviesnz Dec 3, 2019

epiqueras Dec 3, 2019

glendaviesnz Dec 4, 2019

glendaviesnz Dec 9, 2019

epiqueras Dec 9, 2019

glendaviesnz Dec 9, 2019

epiqueras Dec 9, 2019

glendaviesnz Dec 9, 2019

glendaviesnz commented Dec 9, 2019

epiqueras left a comment

CreativeDive commented Apr 7, 2023

youknowriad commented Apr 7, 2023

CreativeDive commented Apr 7, 2023

youknowriad commented Apr 7, 2023

Limit of Next Page (Page Break) block to root level only #18260

Limit of Next Page (Page Break) block to root level only #18260

Conversation

glendaviesnz commented Nov 4, 2019 • edited Loading

Description

How has this been tested

To test

Screenshots

glendaviesnz commented Nov 11, 2019

mmtr commented Nov 11, 2019

kwight commented Nov 11, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

glendaviesnz commented Dec 9, 2019

epiqueras left a comment

Choose a reason for hiding this comment

CreativeDive commented Apr 7, 2023

youknowriad commented Apr 7, 2023

CreativeDive commented Apr 7, 2023

youknowriad commented Apr 7, 2023

glendaviesnz commented Nov 4, 2019 •

edited

Loading