Add reusable option to inline block creation

Patch tested, able to set inline custom blocks to reusable on creation, however there are two issues:

• The checkbox on the form is still there when editing (with the value always set to FALSE), the field should not be shown.
• The description probably needs more information warning the user that this option cannot be changed once the block is created.

I've modified patch in #2 to expose the option only when the block is new. Also until saving, the default value will change based on the configured value.

woops. rerolling #8 with correct file paths..

Thanks for the patch!

1. +++ b/core/modules/layout_builder/src/InlineBlockEntityOperations.php
   @@ -276,7 +276,9 @@ class InlineBlockEntityOperations implements ContainerInjectionInterface {
   +    if ($block['#block_content']->get('reusable')->value == 0 && ($duplicate_blocks || (empty($pre_save_configuration['block_revision_id']) && !empty($post_save_configuration['block_revision_id'])))) {
   

   The == 0 here seems brittle.

2. +++ b/core/modules/layout_builder/src/Plugin/Block/InlineBlock.php
   @@ -202,6 +212,11 @@ class InlineBlock extends BlockBase implements ContainerFactoryPluginInterface,
   +    if ($form_state->getValue('reusable') == 1) {
   

   Same problem here.

Additionally, still needs IS update

I suspect this might create some issues with the edit workflow, assuming that this is still referenced by revision ID. If you create a reusable block that can be edited outside of the context of this node, then you have a problem with the reference. Changing it outside will result in a new revision, but I assume that the displayed block will then still display the old revision and likely no way to select the most recent one. IMHO a non-reusable block needs to be referenced by ID only, and to create drafts, you need to use the workflow module.

I don't we should do this just because we can.

This would introduce some usability problems that we should consider/solve before we add this feature.

I personally don't think we should code this and then think of the usability concerns afterwards. It would be great if we could address the usability concerns first.

Access control

Currently when you add a non-reusable block in Layout Builder access to that block is controlled by access to the entity using the Layout. So if you are placing it on content that is "Draft" no one can access the block in any way.

1. Will the user understand the difference between this behavior and resuable blocks which once created could be place via another entity using the layout builder, block layout or other modules? How are we conveying that that to the user?
2. How easily could the user accidently check this checkbox and expose what they thought was access controlled content?
3. Since they cannot change the setting after saving what would be the process of deleting the block if they made a mistake and created what they assumed was going to be access controlled because the content was draft?

   They would have to go to the Custom Block library, find the block and then delete it.
   Since often you would not display the label for a block you placed in the Layout Builder editors may not make sure these labels are unique. Also other editors could likely use the same labels

   It seems like they could easily pick the wrong block and mistakenly delete a different one leaving the block they created to be placed elsewhere.
   Do we have any mechanism to know if this block was used anywhere else?

4. How are we conveying during editing a reusable block that the content they are entering will be available in other places and is not controlled by access to the entity they are using the Layout Builder on.

Revisions

re @Berdir #11

IMHO a non-reusable block needs to be referenced by ID only, and to create drafts, you need to use the workflow module.

I can see how this would be good idea but would also change the behavior from the user's perspective when creating blocks in Layout Builder.

Right now for inline blocks and other block configuration you create within the layout builder that content/configuration will not change unless it is changed within the Layout for the entity. With the reusable blocks we are introducing content you can change inside the Layout Builder but can also be change from outside the layout builder.

How are we conveying that to the user?

Also right now for content you enter into the Layout Builder via inline blocks you can look back through the revision to see what the blocks looked like at any point. Of course right now you can pull in a reusable block but you can't edit its content inside layout builder.

Now we would be introducing content you could edit inside layout builder but which maybe changed from the state it was at a particular revision.

To be clear, I'm not proposing to implement this. I was just pointing out that *if* this is implemented, then IMHO using the revision-reference approach wouldn't work as there is AFAIK no UI that will allow you to re-select the most recent/default revision for an embedded block. In paragraphs we also introduced a separate module to have reusable paragraphs which are referenced with a regular entity reference field and not ERR.

The simple solution is of course to not support it, maybe just include a link that allows to create a new reusable block and then you can embed it later.

Adding a related issue here since the concept of drafts were mentioned. There is still some work to do in this area when using block content and layout builder.

This functionality exists in Drupal 7 with Panels IPE, FPP and Panelizer - we use it in Panopoly, and a number of Panopoly-based sites depend on it in order to allow non-technical editors (not admins) to easily create re-usable widgets for their site.

To the questions/concerns about how this could be implemented, I can describe how it works in D7...

IMHO a non-reusable block needs to be referenced by ID only, and to create drafts, you need to use the workflow module.

In D7, a non-resuable FPP is referenced by the revision uuid, and resuable blocks are referenced by the fpid (the primary key of an FPP). In D7 you can convert from a non-reusable FPP to a reusable one (but not vice-a-versa) which changes how the reference is done. I don't know how you'd put an editorial workflow on FPPs in D7, and I haven't ever seen that, so I can't speak to that part.

But what this means you can edit a reusable FPP both on a specific page, and from the list of reusable FPPs, and changes in either place are reflected everywhere that FPP is used. This is conveyed to use by a bit of the form that is only exposed on resuable FPPs (for configuring the "admin title" of it) and a little bit of warning text:

Regarding access control:

In D7, I don't think FPPs access control has anything to do with the entity it's on: FPPs have permissions similar to content types, so if you can create or edit an FPP of a particular type globally, then you can edit on FPP of that type on a Panelized entity. Panelizer does respect the access control of the entity, though, so if it's non-reusable and you can't edit the Panels display of a given entity, then you can't edit that non-reusable FPP in any practical way.

Anyway, I'd love if this functionality was implemented in core, but I understand if core doesn't want to take that on. We could certainly implement this in contrib instead for use in Panopoly 2.0 :-)

I've been testing this patch a little bit!

Looking at the serialized Section in the field data, when I've created a block as reusable through Layout Builder, it appears to be saved differently, than if you had added a pre-existing reusable block. For example, the reusable block created in layout builder has an id of "inline_block:BLOCK_TYPE" whereas the existing reusable block has an id of "block_content:UUID". I would have expected the reusable blocks created in layout builder to be store the same as pre-existing reusable blocks.

Also, I'm able to continue to edit the content of reusable block added in layout builder, but not the pre-existing reusable block that I added (I actually used the same block, added twice, so it's extra weird :-)). It would be nice if users with permission to edit a reusable block could do it from layout builder, but I could certainly understand why some sites might not want to do that. That bit on it's own makes it sound like this should be optional functionality, perhaps in a contrib module...

Here's an unfinished alternative approach to this patch. Rather than just continuing to use inline_block plugins, it actually converts to a block_content plugin when a user checks the "Reusable" checkbox. It also allows doing that conversion at any point on an inline_block, even a pre-existing one, but the conversion is always one way.

This is incomplete because the companion to this would be to allow editing the content on block_content plugins, so that you can continue to edit the block after it's been made reusable, but I haven't implemented that yet.

BTW, I think everything I'm doing in this patch (and probably will be doing even once complete) could be done in hook_form_FORM_ID_alter() and so doesn't need to be core. But I'm starting here, just in case some variation of this could be acceptable for inclusion in core.

This patch is a mess of copy-paste coding, but it gives the general shape of how I think this functionality should work (modeled on how this was done in D7). Please let me know what you think!

Here's a minor update to the patch that adds a warning message when editing a block_content block.

And now some screenshots!

Here's adding an inline_block, where I've checked the "Reusable" checkbox and filled in an "Admin title":

Note: the "Admin title" field doesn't appear until the reusable checkbox is checked.

And here's editing that same block after it has been converted to a block_content plugin (so, the ability to edit the content, as well as the warning appear on any block_content block, not just those reusable ones added in layout builder):

Issue summary update based on the template

This is a bit uglier than the core patch, but I did an implemenation of this from a sandbox module that uses form alters:

https://git.drupalcode.org/sandbox/dsnopek-3059373/blob/8.x-2.x/src/Alte...

So, this is possible to do from contrib! It'd still be nice to get it into core, though. :-)

Patch #19 was giving me an error on this line $block_content->setInfo($this->configuration['label']); the configuration variable was undefined - I tried changing this to $block_content->setInfo($configuration['label']); but then that updated the Admin title which is not what I expected. This also did NOT update the original block title, only the Admin title, which I thought was confusing. Lastly I commented out the line $block_content->setInfo($this->configuration['label']); and then I was able to save the reusable blocks with their own title field saving, and not updating the admin title field. This is the ideal in my opinion, but not sure what else commenting that line out may affect. Still feel like there are some UX/UI consistencies to work on.

Adding a patch that removes the $block_content->setInfo($this->configuration['label']); line from the submitForm function.

I haven't tested the core patch as extensively as I've tested the sandbox contrib module. In the module, it's actually doing a different line:

$block_content->setInfo($form_state->getValue('label'));

I suspect something got lost in all my copy-paste between the two implementations, and that if we replaced the original line with the above, that it might work?

Hi, I've tried the patch and il seems working but I've some concerns about usability. Let's say I've to add a lot of reusable blocks, I'll end with a long list. Should be possible to use entity browser (or something like that) instead adding the blocks to the layout editor panel?

Patch at #24 was causing issues, if we do not enter the admin title.

Issue - As admin title is not a mandatory field, we can leave it blank. Also, as the value needs to be entered only when you want to make it reusable, we cannot make it mandatory. Hence, when left blank, there is no title being displayed on custom block library (admin/structure/block/block-content) page. Also, when placing a reusable block via layout builder, this new block, wont show as there is no title there to be linked.

I believe, we can set the label as admin title, of not provided.
Attaching a patch for same.

Changing status.

#28: Yes falling back to label makes a lot of sense.

Nit: Coalescing $block_info can now be simplified with the "??" operator.

Just tagging so we don't forget this should get a usability review

We discussed this on the UX meeting today; BIG +1 to this feature. People definitely expect to be able to edit "content-y" thingies from the front-end of their site, and this is a HUGE workflow improvement over having to halt your Layout Builder experience, go dig around in admin/block/XXX to add/edit content there, and then return and figure out a way to abstractly reference it.

Two small pieces of feedback, and one larger one that can maybe be a follow-up:

1) The description of the "Reusable" checkbox could use some work. It's currently "Would you like to be able to reuse this block?" But that basically uses the word to define the word. :) Better would be something that explained the impact of choosing this option; for example "This creates a block that can be used across multiple layouts, as well as in site-wide regions" (probably better worded than that). The administrative title description is good; it explains that the label is used to find and reuse the block later.

2) The ! in the warning is a bit AHHH! We don't tend to use that anywhere else; the warning itself is an ! :) So let's make it just a period and calm people down a bit. ;)

3) Maybe not for this issue, as there's some design work involved, but we already have iconography related to "local" vs. "global" changes (a map marker vs. globe icon) and it would be nice to re-use that here since we're essentially trying to communicate the same concepts, but this introduces another way of doing it.

And given the workflow improvement here for content authors, escalating to Major.

Crediting folks from the UX meeting, which will eventually be at https://www.youtube.com/watch?v=0Fjk4cFiIV0 We talked about this in approx. the last 15 mins of the call.

@webchick thanks for reporting back from UX meeting.

I have a question and I really wish I had mentioned in #31 when I tagged with needs usability review. Sorry

From #12 under access control

Currently when you add a non-reusable block in Layout Builder access to that block is controlled by access to the entity using the Layout. So if you are placing it on content that is "Draft" no one can access the block in any way.

How are we conveying during editing a reusable block that the content they are entering will be available in other places and is not controlled by access to the entity they are using the Layout Builder on

I have questions in #12 around this but I was wondering in the UX meeting if this can up.

I just wonder if people will understand if they add a block and check this box the content they add via the block will be available for others to see regardless of the state of the entity. So if you add on page and that page is draft meaning most users can't see the page, the reusable blocks will be immediately available to other users who have access to use custom blocks.

So the scenario would be

• User 1 adds block and make it reusable to Page 1.
• Page 1 is draft state and has never been published
• User 2 cannot see Page 1 because they don't have access to the page in Draft
• User has access to the layout builder because they have permission Configure layout overrides for pages that the user can edit
• User 2 can therefore see all reusable blocks added to Page 1(still in Draft state) by using Layout Builder on different page(they will be listed in blocks to add)

I don't think this security problem per see more of issue as to how can we make this situation clear to the user.

Will they assume that all blocks they add in the Layout Builder will have there access controlled by the fact that page they are adding them to is in Draft.

Great questions, and definitely not something we remotely talked about, so re-adding the tag. :) I will have a thinker and report back.

@tedbow yeah that's a really good consideration.

After the UX meeting yesterday I also had another thought: is it possible for a user to find themselves in a situation where the site permissions are setup in such a way where they can edit a Layout Override of an Entity and can add non-reusable custom blocks through LB, but can't create site-wide reusable blocks through the Custom Block UI? If that is possible, how would the additions in this patch handle that, would the user still be able to create reusable custom blocks via LB?

Generally this patch needs functional tests.

Re #40 I would think that access to create reusable blocks should respect admin_permission that is administer blocks. But this should be determined by calling createAccess() on the access handler.

We would need tests for this case, among others, to make sure access is respected

I was working on issue 2942975 and I was also using this patch. I feel this patch needs to be updated with uuid and bundle information about the block in json api output.

e250fe81-4297-47fe-a479-5b75b67e7e3b": {
"uuid": "e250fe81-4297-47fe-a479-5b75b67e7e3b",
"region": "content",
"configuration": {
"id": "block_content:a61fd3b3-97d8-4b62-99ab-5a643746d66d",
"label": "Third block reusable",
"provider": "block_content",
"label_display": "visible",
"status": true,
"info": "",
"view_mode": "full",
"type": "embed_block",
"uuid": "a61fd3b3-97d8-4b62-99ab-5a643746d66d"
},
"additional": [],
"weight": 4
}

Attaching the Patch and inter diff for the same.

Thanks

NW for tests

#42 works.

But is it safe to use in Production?

I am happy to use this but need advice and is the patch going to be effected by any core releases in future?

Hi,

Tnx for the patch works great!

There is still one small issue > https://www.drupal.org/project/drupal/issues/3013125 this is resolved in core 8.8.5 but when using this patch and check 'Reusable' issue is still present.

I reused @vedpareek patch (https://www.drupal.org/files/issues/2019-11-20/2999491-42.patch) and added 'label_display' property.

I've tested locally #46 and it solved the problem with the label_display. Previous than that (#42), I had that problem. With the new patch it is not there anymore.

Thanks

Hello,

I am providing a new patch from #45 with the following changes:

• Now the services are injected.
• Provide a new permission "create reusable blocks" to grant access or not to this functionality.

I think the new permission is specially usefull in those cases on which you would like to have some users to define those reusable blocks with some control on them to avoid create to many of them without control.

Cheers

I know this issue is related to having reusable blocks, but if this gets included then the jsonapi output for these reusable blocks should also be updated. See https://www.drupal.org/project/drupal/issues/2942975

Currently, the jsonapi output doesn't include any information of the added blocks which could be used to navigate and fetch the data. Having the type and uuid added in the jsonapi output of the blocks would help in doing so and this was taken care of for inline blocks in #42 but not for reusable inline blocks and blocks added from the Custom Block Library.

Scenario 1:

• Create an inline block on Page 1 and mark it as reusable
• Add the reusable block on Page 2 (new page) or on Page 1
• The reused block will be missing type and uuid in the jsonapi output for both the pages

Scenario 2:

• Create a block under Custom block library
• Add the block on a page
• The block will be missing type and uuid in the jsonapi output

Adding a patch that will add type and uuid to blocks that are being reused (originally created inline via layout_builder) as well as reused from Custom Block library.

This might be taken care of in a separate issue once this feature is included but since this is relevant I am reporting it here first so that this comes into discussion. Let me know your thoughts.

Minor update to #49 (I love the permission idea btw), the block_form should be nested under settings in the form array since that is how it in layout builder before it's made re-usable. Anyone using the field_group module will have noticed their fieldgroups not rendering correctly in the block form prior to this change.

Note: I didn't base this on the patch in #50 since that one's failing testing and possibly requires some additional discussion.

Thanks for moving this issue along everyone!

1. +++ b/core/modules/layout_builder/layout_builder.permissions.yml
   @@ -4,6 +4,9 @@ configure any layout:
   +create reusable blocks:
   +  title: 'Create reusable blocks'
   +  description: 'Create reusable blocks which can be used in different layouts'
   

   I don't think we should create new permission see my comment in #41. But more details...

   This description doesn't actually fully describe what the permission allows you to do.

   If a user has this permission then they can make reusable blocks that can be used in different and can be used be placed outside of Layout Builder i.e the regular Custom Block Library.

   So before this patch only users with "administer blocks" permission could add and edit blocks in the custom block library but this patch would allow users with this new to permission to also add blocks to the custom library and edit any block in the custom library.

   You could even add/edit any block in the custom library without actually adding it to layout by just adding the existing custom block to the layout and editing while you add it and then deleting from the layout without every saving the layout itself. But since the block saved you have just edited a block without adding to the layout.

   It is bit convoluted but it does give you add/edit access to content blocks just not delete access. The user just won't have access to add/edit at current admin/structure/block path. This would make it hard for site admins to understand the effect of giving this new permission to people. Probably also most user's given this permission won't figure out that it gives them access to add new block into the custom block library and edit any block in the library without actually placing the blocks in the library, but that would just make it more confusing.

   I think any new permission should be added in follow-up issue to the block_content module and then have layout_builder respect this permission

2. +++ b/core/modules/layout_builder/src/Form/ConfigureBlockFormBase.php
   @@ -174,6 +201,48 @@ public function doBuildForm(array $form, FormStateInterface $form_state, Section
   +      /** @var \Drupal\block_content\BlockContentInterface $block_content */
   +      $block_content = $form['settings']['block_form']['#block'];
   +      $form['reusable'] = [
   +        '#type' => 'checkbox',
   +        '#title' => $this->t('Reusable'),
   +        '#description' => $this->t('Would you like to be able to reuse this block? This option can not be changed after saving.'),
   +        '#default_value' => $block_content->isReusable(),
   +        '#access' => $this->currentUser->hasPermission('create reusable blocks'),
   +      ];
   +      $form['info'] = [
   +        '#type' => 'textfield',
   +        '#title' => $this->t('Admin title'),
   +        '#description' => $this->t('The title used to find and reuse this block later.'),
   +        '#access' => $this->currentUser->hasPermission('create reusable blocks'),
   +        '#states' => [
   +          'visible' => [
   +            ':input[name="reusable"]' => [ 'checked' => TRUE ],
   +          ],
   +        ],
   +      ];
   

   Could this be done in \Drupal\layout_builder\Plugin\Block\InlineBlock::buildConfigurationForm()

3. +++ b/core/modules/layout_builder/src/Form/ConfigureBlockFormBase.php
   @@ -174,6 +201,48 @@ public function doBuildForm(array $form, FormStateInterface $form_state, Section
   +    if ($this->block->getBaseId() === 'block_content') {
   +      // Show the block content form here.
   +      /** @var \Drupal\block_content\Plugin\Derivative\BlockContent[] $block_contents */
   +      $block_contents = $this->entityTypeManager->getStorage('block_content')->loadByProperties([ 'uuid' => $this->block->getDerivativeId() ]);
   +      if (count($block_contents) === 1) {
   +        $form['messages'] = [
   +          '#theme' => 'status_messages',
   +          '#message_list' => [
   +            'warning' => [$this->t("This block is reusable! Any changes made will be applied globally.")],
   +          ],
   +        ];
   +        $form['settings']['block_form'] = [
   +          '#type' => 'container',
   +          '#process' => [[static::class, 'processBlockContentForm']],
   +          '#block' => reset($block_contents),
   +          '#access' => $this->currentUser->hasPermission('create and edit custom blocks'),
   +        ];
   +      }
   +    }
   

   I think if we can we should try to avoid having block type specific logic in this "base" form.

   We could add new plugin type like \Drupal\layout_builder\Plugin\Block\InlineBlock, maybe InlineReusableBlock then we could put the logic there. This would also allow different logic then \Drupal\block_content\BlockContentForm which I think we need anyway.

   This would be layout builder only plugin which would mean we would need to update layout_builder_plugin_filter_block_alter() to also filter out the new plugin id.

4. +++ b/core/modules/layout_builder/src/Form/ConfigureBlockFormBase.php
   @@ -228,6 +329,40 @@ public function submitForm(array &$form, FormStateInterface $form_state) {
   +      $form_display->extractFormValues($block_content, $block_form, $complete_form_state);
   +      $block_content->save();
   +    }
   ...
   +      $block_content->setInfo($block_info);
   +      $block_content->save();
   

   One of the problems of actually saving the block_content entity in \Drupal\layout_builder\Form\ConfigureBlockFormBase::submitForm() is that means that even if we add a block to the layout or edit an existing block in the layout but never actually save the layout the change to the block would still be saved.

   So if a user:

   • clicks the layout tab
   • changes a reusable block in the layout, both label and body(or another field value)
   • click "Discard changes" because they decide they didn't actually want to make the change
   • Viewed the entity using the layout
   • The block label change would NOT show their change because this is stored in the layout
   • The block body field change would show because the block_content entity was actually saved and not revert.

   Probably if we had the new block plugin type InlineReusableBlock we could handle saving the same InlineBlock does. It saves the serialized block in \Drupal\layout_builder\Plugin\Block\InlineBlock::blockSubmit() but the actual entity is not saved util the layout is saved in \Drupal\layout_builder\InlineBlockEntityOperations::saveInlineBlockComponent() which calls \Drupal\layout_builder\Plugin\Block\InlineBlock::saveBlockContent()

   This means that for inline blocks the changes are NOT permanent until the layout is actually saved. I think we would want the same behavior.

   We could have a new class LayoutBuilderBlockPluginBase that both the existing InlineBlock and new InlineReusableBlock would extend and move the common logic in there.

One other for my suggestion #54.3 to create the new InlineReusableBlock plugin we would have to change the existing uses of BlockContentBlock to use InlineReusableBlock instead to allow editing. But we don't actually have to change them until you try to edit the reusable block.

Quick update on my last patch which didn't catch 'em all. Leaving as needs work for now though since there's still all of comment #54/55 to address (which I think all makes a lot of sense!).

In regards to the new permission, I'm wondering if it makes more sense to just go by the block content bundle add/edit permissions on a per-block basis?

• To enable the "Reusable" checkbox user needs the "create block content" permission
• To edit reusable blocks when placing in LB user needs the "edit block content" permission

Seems like this should align with a site-builder's expectations. The "edit" permission especially as this would be functionally similar to editing a custom block using a contextual link in a classic block layout site build.

Is it intended that the the class .block-inline-blockTITLE is removed after checking the re-usable flag? I'm currently styling based on that class.

In response to the usability questions in #38:

To me it seems ok to immediately have a reusable block available for, well, reuse elsewhere.

The promise on the checkbox is that the block is made reusable. So that's what should happen. I don't think the *when* of becoming available for reuse needs to depend on the publish/access state of the blocks container/layout, even if that is where the block originates.

For what it's worth, Wordpress has something similar to this. In the Gutenberg editor you can add a block-level element (or group multiple ones first), and mark it as a reusable block. What I did:

- Create Post 1, add some sample content. I grouped an image + text and marked it reusable as "roys reusable block".
- Save Post 1 as draft
- Create Post 2, add stuff, also adding "roys reusable block".
- Publish Post 2.
- Post 2 shows in full, including "roys reusable block".

(This does not yet answer the more detailed questions in #12, but it's a start :)

Sharing my thoughts on #54.4.
@tedbow I kinda agree to what you are trying to say here, but I see few hiccups here, mentioned below. Please help us understand how do you envision this.

1. In case of big number of content being added in blocks, assuming the page is build completely using Inline blocks, wont it take some sweet time saving the content ?
2. In case you prepare the complete content set and you did not save the page, you will loose all your content. Its better to save some content and loose some, than loosing 100%.
3. Rather than this, shall we think of adding a button, which says, Discard changes ? This way we can revert the block to older content revision or delete them completely, if new.

As suggested in #60 point 3 we have implemented an workaround to revert the block to its previous version when user discard the changes of layout by clicking already available discard changes button. In order to do this we have done following changes,

• In ConfigureBlockFormBase, when user update a reusable block, just before update is saved we stored a copy of block in a temporary storage.
• In DiscardLayoutChangesForm , on submit we have checked if any of the reusable block has its previous state stored, if yes then we have reverted the block to that stored state with new revision.
• On Save layout we have clear all the temporarily stored block if any for that node.

please find the patch for the same agains 9.1.x and 8.9.x branch.

@arshadkhan35 There's an error:

Error: Call to undefined method Drupal\layout_builder\Plugin\SectionStorage\OverridesSectionStorage::getContainingEntity() in Drupal\layout_builder\Form\ConfigureBlockFormBase->submitForm() (line 349 of /app/core/modules/layout_builder/src/Form/ConfigureBlockFormBase.php)

#0 [internal function]: Drupal\layout_builder\Form\ConfigureBlockFormBase->submitForm(Array, Object(Drupal\Core\Form\FormState))
#1 /app/core/lib/Drupal/Core/Form/FormSubmitter.php(113): call_user_func_array(Array, Array)
#2 /app/core/lib/Drupal/Core/Form/FormSubmitter.php(51): Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object(Drupal\Core\Form\FormState))
#3 /app/core/lib/Drupal/Core/Form/FormBuilder.php(593): Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object(Drupal\Core\Form\FormState))
#4 /app/core/lib/Drupal/Core/Form/FormBuilder.php(321): Drupal\Core\Form\FormBuilder->processForm('layout_builder_...', Array, Object(Drupal\Core\Form\FormState))
#5 /app/core/lib/Drupal/Core/Controller/FormController.php(73): Drupal\Core\Form\FormBuilder->buildForm(Object(Drupal\layout_builder\Form\UpdateBlockForm), Object(Drupal\Core\Form\FormState))
#6 [internal function]: Drupal\Core\Controller\FormController->getContentResult(Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\RouteMatch))
#7 /app/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array(Array, Array)
#8 /app/core/lib/Drupal/Core/Render/Renderer.php(573): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#9 /app/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(124): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure))
#10 /app/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array)
#11 /app/vendor/symfony/http-kernel/HttpKernel.php(158): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#12 /app/vendor/symfony/http-kernel/HttpKernel.php(80): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1)
#13 /app/core/lib/Drupal/Core/StackMiddleware/Session.php(57): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#14 /app/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#15 /app/core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#16 /app/core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#17 /app/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#18 /app/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(52): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#19 /app/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#20 /app/core/lib/Drupal/Core/DrupalKernel.php(706): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#21 /app/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#22 {main}

Add a block in layout builder.
Save it.
Then click configure and try to edit and update the block.

Thanks @abhaisasidharan for quick review, The issue has been resolved, please find updated patch for the same.

Fixed custom command failure errors in #63, thanks!

Some test cases were failing. Attaching interdiff as well.

A test case was failing because of an error:

The website encountered an unexpected error. Please try again later.

Drupal\Core\Config\Schema\SchemaIncompleteException: Schema errors for core.entity_view_display.node.bundle_with_section_field.default with the following errors: core.entity_view_display.node.bundle_with_section_field.default:third_party_settings.layout_builder.sections.0.components.b158757a-b5c3-4c0a-84a1-f4a36e0ced2c.configuration.type missing schema, core.entity_view_display.node.bundle_with_section_field.default:third_party_settings.layout_builder.sections.0.components.b158757a-b5c3-4c0a-84a1-f4a36e0ced2c.configuration.uuid missing schema, core.entity_view_display.node.bundle_with_section_field.default:third_party_settings.layout_builder.sections.0.components.6c5a676a-c735-4af7-8e4a-86866b327f87.configuration.type missing schema, core.entity_view_display.node.bundle_with_section_field.default:third_party_settings.layout_builder.sections.0.components.6c5a676a-c735-4af7-8e4a-86866b327f87.configuration.uuid missing schema in Drupal\Core\Config\Development\ConfigSchemaChecker->onConfigSave() (line 94 of core/lib/Drupal/Core/Config/Development/ConfigSchemaChecker.php).

call_user_func(Array, Object, 'config.save', Object) (Line: 142)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch(Object, 'config.save') (Line: 231)
Drupal\Core\Config\Config->save() (Line: 273)
Drupal\Core\Config\Entity\ConfigEntityStorage->doSave('node.bundle_with_section_field.default', Object) (Line: 452)
Drupal\Core\Entity\EntityStorageBase->save(Object) (Line: 252)
Drupal\Core\Config\Entity\ConfigEntityStorage->save(Object) (Line: 339)
Drupal\Core\Entity\EntityBase->save() (Line: 591)
Drupal\Core\Config\Entity\ConfigEntityBase->save() (Line: 300)
Drupal\layout_builder\Plugin\SectionStorage\DefaultsSectionStorage->save() (Line: 214)
Drupal\layout_builder\Form\DefaultsEntityForm->save(Array, Object)
call_user_func_array(Array, Array) (Line: 113)
Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object) (Line: 51)
Drupal\Core\Form\FormSubmitter->doSubmitForm(Array, Object) (Line: 593)
Drupal\Core\Form\FormBuilder->processForm('entity_view_display_layout_builder_form', Array, Object) (Line: 321)
Drupal\Core\Form\FormBuilder->buildForm(Object, Object) (Line: 73)
Drupal\Core\Controller\FormController->getContentResult(Object, Object) (Line: 39)
Drupal\layout_builder\Controller\LayoutBuilderHtmlEntityFormController->getContentResult(Object, Object)
call_user_func_array(Array, Array) (Line: 123)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 573)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 124)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) (Line: 97)
Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() (Line: 158)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 80)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 706)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

Updated patch with fix.

I can confirm that the 8.9.x patch from #63 works.

I tested the patch in #66 with Drupal 9.1 and it works as expected.

+++ b/core/modules/layout_builder/src/Form/ConfigureBlockFormBase.php
@@ -174,6 +212,48 @@ public function doBuildForm(array $form, FormStateInterface $form_state, Section
     $form['settings'] = $this->getPluginForm($this->block)->buildConfigurationForm($form['settings'], $subform_state);
...
+    if ($this->block->getBaseId() === 'block_content') {
...
+    elseif ($this->block->getBaseId() === 'inline_block') {

Why aren't these done as part of the block's own config form? It makes me nervous to have even more one-off cases here. If block_content were in contrib, it wouldn't have this opportunity...

Also this issue is tagged for tests and a usability review, it can't be marked RTBC until those are done.

Adjusting 'Remaining Tasks' section of issue to reflect both usability checks and tests. Have we resolved 'Determine if this is something we should tackle in core'?

Tested the patch in #66 with Drupal 9.1 and it works as expected, as well.

Tested patch #61 with Drupal 8.9.15 but get
Call to undefined method Drupal\layout_builder\Plugin\SectionStorage\OverridesSectionStorage::getContainingEntity() in Drupal\layout_builder\LayoutReusableBlockDiscardChanges->deleteReusableBlockTemporaryStorage() (line 72 of core/modules/layout_builder/src/LayoutReusableBlockDiscardChanges.php)

After patch #66, my site gets broken on adding custom blocks by layout builder.
I am getting this error message

Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: You have requested a non-existent service "layout_builder.reusable_block_discard_changes". in Drupal\Component\DependencyInjection\Container->get() (line 151 of /Users/gauravmahlawat/Desktop/devdesktop/drupal/core/lib/Drupal/Component/DependencyInjection/Container.php).

@Gauravmahlawat in #76: The patch introduces the service layout_builder.reusable_block_discard_changes, so you probably just need to clear the Drupal site cache, otherwise Drupal doesn't know about this new service and throws that error.

Is the admin title necessary? If I leave it blank, I'll see the block's (mandatory) title in the Laout Builder's Pane and also in the blocks admin configuration.

I agree with the UX challenges here:

Discard changes should not apply to re-usable blocks at all -- or at least this behavior should be opt-in.

Background:

Re-usable blocks are a MUST HAVE for workspaces, because then you can move each block independently through the workflow or deployment pipeline.

But with workspaces a discard changes means to remove the block from the workspace (semantically), not re-store a different revision.

Also unless I missed something this is not race condition safe and frankly I can't see how it could be (outside of a workspace):

- User A edits block in layout B
- User B edits block directly
- User A discards changes in block layout B

Now you could implement the forward revisions that workspaces use and never save the default configuration, but then the dilemma remains of when to apply changes to "live".

The problem is layout builder is implementing a workspaces "light" without being able to have any of the stronger data consistency guarantees that workspaces has. If it were me, I would re-implement layout builder on top of workspaces (hidden auto-workspace, if not already in a workspace), but for that workspaces would need to stop being an experimental module (of course).

For this patch I simply suggest to have discard changes not act on re-usable blocks and clearly state so.

Fixed Drupal CS issue of patch #66.

We do use #82 and it works well. Adding RTBC to push it forward

Hello, I am not sure if the @drclaw comment from #52 got lost in the latest patches (https://www.drupal.org/project/drupal/issues/2999491#comment-13841853) but field groups are not rendering correctly.

Adding some screenshots before and after enabling the reusable setting.

We discussed this issue at #3292468: Drupal Usability Meeting 2022-07-01. That issue will have a link to a recording of the meeting.

For the record, the participants in the usability meeting were AaronMcHale, andregp, rkoller, shaal, simohell, and me.

I will add a summary of the discussion later. For now, I want to add a reminder that there are some open questions in Comments #38, #40, and #80. And I have a screenshot (using the Umami demo profile) that I want to share:

Usability review

See #8 for a link to the issue for the usability meeting.

First of all, we all think that this issue is a big improvement. Too many people are not aware of the distinction between reusable blocks and non-reusable ones. Just by letting them know about the distinction, this issue will help. Letting a content editor change a block to reusable will help even more!

We would like to recommend some changes. We felt that a checkbox on the block's edit form is the wrong place to change it from non-reusable to reusable. This is a change that cannot be undone. The usual way to handle that is not a warning in the description text for the checkbox: it is to use a confirmation form. There was strong consensus for this recommendation.

We already have an irreversible action for a block: deleting it. We should use the same pattern for both. That is, add an item to the contextual links. Selecting the link triggers a confirmation form in the off-canvas sidebar.

At the same time, review the text for removing a block (from the layout). Perhaps we should have different text depending on whether the block is reusable.

Having the confirmation form should make it easier to convey the implications for access control: see #38. Describe the other implications in terms that make sense to the content editor: the block will appear in the custom block library; it can be used in more than one place.

It will also be convenient to choose reusable or not when adding a new block. There are several options, and we do not have a strong recommendation. For example: two links, something like "Add reusable block" and "Add block for this page only". Or, after choosing "Create custom block", select reusable or not along with the block type.

This issue already does a lot: it makes reusable blocks editable from LB, and it lets the editor convert a non-reusable block to reusable. It might be a good idea to have a follow-up issue to decide how to choose when adding a new block.

I am keeping the tag for a usability review. I would like to bring this issue up again once the changes have been made.

Hello all, not sure if this is an issue related to this patch or more of a general issue with blocks using Layout Builder. However, I thought I would raise it and let the community guide me.

We are using this patch on a project and it seems to work great! I recently found an issue with Layout Builder, Revisions, and Blocks. I did some testing with revisions and it would appear maybe I found an edge case bug in node revisions related to blocks and maybe this patch. When you add a block to Layout Builder and the block isn't selected as reusable that blocks revisions seem to be tracked with the nodes revisions and changes are tracked as expected. This works even if the block isn't set to track revisions. However, when a block is selected as reusable it would appear that revisions are not tracked both in node revisions or on the block even if revisions are enabled for both block and content type.

This seems like odd behavior to me and as I said may be known. I think the ideal solution here would be to have reusable blocks use the blocks revision history and create an entry in the nodes revision history referencing the blocks revision.

Please let me know if this issue should be posted some place else or if it's a known issue and I just didn't find it in my Google search. Thanks!

After watching the discussion on this meeting: Drupal Usability Meeting - 2022-07-01, I have a few suggestions:

1) Will it be an alternative to name it "Add to library" instead of "Add reusable block"/"Add to custom block library"?

2) When a custom block is created from front-end, I will think that the option to make it reusable (add it to the library) many times is something a content creator would do after the block is created. Therefor I will suggest to not add this choice avaiable in the block add form (because it could make more confusion and another option to to decide on in the process). Instead I think a "Add to library"-button/choice to the end of the edit form, and to the contextual links is a better place to have it. Then it is avaiable when the content creator need it on the front-end, and it would not create another option to choose from in the creation (add) form for custom blocks.

3) I think the warning message shown in comment #87 only should have one sentence (something like the last one in the screenshot), to make it shorter and not so "dramatic". I think it's better to add a link to a help page with more details or something if the info message don't describe enough.

I agree with @vegomedia, for me it makes more sense to add the option to make it reusable after the block is created with the text "Add to library"

attached reroll patch against 9.5.x-dev. Kindly review

I have fixed the issue against #92 and uploaded the patch. Please Review

Per #92 and #93, please read the context in #2999491-88: Add reusable option to inline block creation - your patch is not sufficient to move to Needs Review, given the outstanding work there.

Updating the Issue Summary (IS) to note the current state of this issue, as it was outdated. I used feedback from #88, and referenced conversations earlier in the issue.

Given #2999491-88: Add reusable option to inline block creation and the UX reviews associated with this, can we get explicit go-ahead on whether this should be tackled in core? I'm not sure if I've seen the comments in #2999491-12: Add reusable option to inline block creation and #2999491-13: Add reusable option to inline block creation answered, but I can move that action item from the remaining tasks once that's done.

If someone has the ability to hide comments on this issue, it could use some pruning and clean-up to keep the on-topic changes more succinct and easier to follow.

I need to update the IS with completed tasks, but that's a difficult synopsis for the issue thread as it stands. Gonna take a second shot at that later.

Building on #95 (thanks @Webbeh )

The reroll provided in #92 was not needed, tests demonstrate that the prior patch in #82 applies fine to 9.5.x. Removing credit for a reroll that wasn't needed and is effectively a copy of the prior patch (but with PHPCS-breaking whitespace added).

#93 Attempts to fix the problems in #92, so credit will remain intact for that, but I'd recommend just working with patch #82 as patches #92 and #93 are essentially #82 but with minor changes that don't move the issue forward yet break tests (added whitespace, removing brackets)

#88:
> This issue already does a lot: it makes reusable blocks editable from LB, and it lets the editor convert a non-reusable block to reusable. It might be a good idea to have a follow-up issue to decide how to choose when adding a new block.

It might be a good idea to have a folluw-up issue like "Add an option to fork a reusable block to a non-reusable inline-block (and optionally delete the reusable block IF it's not used elsewhere". (Which makes make-reusable effectively reversible.)
(And add a follow-up issue to add all that bliss to media, too.)

Per #97 - #3303306: Add an option to fork a reusable block to a non-reusable inline-block (and optionally delete the reusable block IF it's not used elsewhere now exists to handle the follow-up issue.

Updating the IS to reflect this follow-up issue.

Together with adding the reusable option, we also should take in account the performance issue in "Add block" form of Layout Builder sidebar with many resuable blocks, here is my issue about this: #3308773: Drupal has performance and memory limit problems with many reusable content blocks (block_content)

When the page is lazy loading the cloned blocks are not rendering in view
If i enable this option: set cloned block as reusable it works fine
What is missing when i clone the block without enable the option and any solution ?

This feature is targeted for 10.1. Drupal8 is no longer supported.

added patch against #82 in drupal 10.1.x

When testing these patches and making a custom block reusable it keeps the block type and all the settings intact which is good. However, it doesn't use the same custom block twig template naming and instead the twig template it uses are randomly generated alphanumeric strings.

For example if I set my custom block using the block--inline-block--content-grid.html.twig template to be reusable, the now reusable block wants to use the block—block-content—791a0697-7a25-418-9c21-a116f0f9b544.html.twig template which I obviously don't have a file created for to layout the information provided.

I hope I haven't missed something completely obvious during my tests. Any thoughts or guidance on this?

@lefteous:

The choice of Twig template is not in scope for this issue.

The theme suggestion block--block-content--791a0697-7a25-418-9c21-a116f0f9b544.html.twig you mentioned in #104 is not the only one. It is probably the most specific suggestion.

It is up to the site's theme to decide whether reusable blocks should be treated differently from non-reusable (inline) blocks. If so, then use Twig templates like block--inline-block--content-grid.html.twig and block--block-content--content-grid.html.twig. If not, then use something like block--content-grid.html.twig.

Updated attributions

Hello

tl;dr
Almost all points are fixed, I'm not a poet and the main issue here is to provide better descriptions of what is reusable block, all caveats of its usage, etc.

Determine if this is something we should tackle in core

Could someone clarify if we need this core? if not I can implement contrib solution.

UX Feedback from #2999491-88: Add reusable option to inline block creation: We felt that a checkbox on the block's edit form is the wrong place to change it from non-reusable to reusable. This is a change that cannot be undone. The usual way to handle that is not a warning in the description text for the checkbox: it is to use a confirmation form. There was strong consensus for this recommendation.
UX Feedback from #2999491-88: Add reusable option to inline block creation: Having the confirmation form should make it easier to convey the implications for access control: see #38. Describe the other implications in terms that make sense to the content editor: the block will appear in the custom block library; it can be used in more than one place.
UX Feedback from #2999491-88: Add reusable option to inline block creation: It will also be convenient to choose reusable or not when adding a new block. There are several options, and we do not have a strong recommendation. For example: two links, something like "Add reusable block" and "Add block for this page only". Or, after choosing "Create custom block", select reusable or not along with the block type.

Put all these points into one since they are all about the same. So I have reworked "Create custom block" form. It is now "two-step" form where in the first step the user can choose what type of block he/she wants to create. There is a "new" description that we will show for users of what is inline and reusable block, could someone review/improve it?

UX Feedback from #2999491-88: Add reusable option to inline block creation: We already have an irreversible action for a block: deleting it. We should use the same pattern for both. That is, add an item to the contextual links. Selecting the link triggers a confirmation form in the off-canvas sidebar.

Implemented contextual link as suggested in #88.

UX Feedback from #2999491-88: Add reusable option to inline block creation: At the same time, review the text for removing a block (from the layout). Perhaps we should have different text depending on whether the block is reusable.

As per this point, I think we should not change anything, since deletion of block does not delete block content entity itself. To delete the block-content user should go directly to the custom blocks library.

UX Feedback from #2999491-87: Add reusable option to inline block creation: Fix UX on status message.

Could not reproduce this.

UX question from #2999491-38: Add reusable option to inline block creation: I just wonder if people will understand if they add a block and check this box the content they add via the block will be available for others to see regardless of the state of the entity. So if you add on page and that page is draft meaning most users can't see the page, the reusable blocks will be immediately available to other users who have access to use custom blocks.

I added a warning but I think someone can add a better one.

UX question from #2999491-40: Add reusable option to inline block creation: is it possible for a user to find themselves in a situation where the site permissions are setup in such a way where they can edit a Layout Override of an Entity and can add non-reusable custom blocks through LB, but can't create site-wide reusable blocks through the Custom Block UI? If that is possible, how would the additions in this patch handle that, would the user still be able to create reusable custom blocks via LB?

I have removed "Create custom block" permission that was in patch before. As for me it's enough for user to have "Create and edit content blocks" permission. And if we are talking only about this permission it's not possible to reproduce behavior from the quote above.
If someone has suggestions why we should have second permission for reusable blocks I would like to hear them.

UX question from #2999491-80: Add reusable option to inline block creation: For this patch I simply suggest to have discard changes not act on re-usable blocks and clearly state so.

I also think that this is a bad idea to have a discard option for reusable blocks. I added a warning but I'd appreciate it if someone can come up with a better one.

Write tests

Since it's not clear if we want to have this in core, I didn't implement tests for it.

Here is "interdiff"

P.s. I messed a bit with branches in PR because I didn't know about new release strategy in core, just ignore the second branch.

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

The patch works as expected. We should probably allow the form display to be overridden, like in #3052551: Use a different form mode when editing block content in an inline block. Attached patch implements something like that.

Tried to fix CCF in #112.

Another patch to try and fix some of the tests.

Whoops, pressButton should be called on $page.

Changing status needs review because it is still not clear if we want to have this functionality in the core. Could someone clarify this?

My two cents here are this feature is important for folks using Layout Builder to build composable content. I have seen a number of use cases where content editors want to reuse a block from page to page. I would vote for including this in the core functionality.

From reading #3375371: [meta] Improve the page building experience definitely think this functionality is wanted in core.

I've fixed the failed tests in #115.
Agree with @johnpicozzi. I would love to see this in core. Using this patch on my website as well and works fine.

Made a small boo boo with phpcs error. Fixed that and adding an inter-diff with patch from 115 as well.

Hi there, we're running 10.1.6 and I tried the patches from #103 and #82, and neither worked for me. I was getting the same error as some others above:
Class "Drupal\layout_builder\LayoutReusableBlockDiscardChanges" not found

I see that the later patches / MR are against D11. Will this work with D10 at all or do we have to wait until D11 for this one? Thanks!

I have fixed Cannot redeclare method and removed duplicated code in core/modules/layout_builder/src/Element/LayoutBuilder.php

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

An update for my previous comment - I applied a patch based on this commit: https://git.drupalcode.org/issue/drupal-2999491/-/commit/1330eff91b12349... and that seemed to work great as it provides the required class LayoutReusableBlockDiscardChanges.php. I'm still testing it out and will report any bugs if I find any.

@codechefmarc Do you have a patch that applies cleanly against 10.2.1? I tried applying https://git.drupalcode.org/issue/drupal-2999491/-/commit/1330eff91b12349... and it failed.

#82 is the newest patch that is working for me after a re-roll against 10.2.x. I'm not sure if it's the mish-mash of patches and direct commits and jump straight to 11.x since then that are causing this confusion, but no combination I've tried will apply or work, even after attempted re-rolls. So if anyone has a working patch for 10.2.x, please upload it.

In the meantime, the attached patch is the only thing that works for me. Since #66 worked on 10.1.x, this is good enough for me, at least for now (this one is based on #82). If anyone can get the newer commits working on 10.2.x, please share a patch.

Just some friendly reminders
* This issue needs to target 11.x now
* Work should be done in the Merge request (which was created from comment #103, against D11).
* If needed, individuals can create their own patches from the MR (see the plain diff link under Issue fork above). But patch files should no longer be posted in the issue queue because this one is already very complex and hard to follow.
* The issue should be reviewed, but can't be marked RTBC due to pending items, see IS and Issue tags.

I'm hiding the patch files to help prevent some confusion going forward.

@capysara How do you recommend this should be handled by those of us who are already using older patches for versions that are no longer being targeted even though they are still current and supported? My team has been using a now three-year-old patch until recently for this functionality, which no longer applies after the 10.2.x updates. Throughout the many years I've worked with Drupal, this is this first time I've been told that this method of submitting patches for community use is not acceptable. A large number of issues tracked by developers are many, many years old and this has long been the method used by the community, in my experience.

Do we need to re-roll patches like this and then self-host them even though doing so means others who may need them can't find them or are you suggesting that we commit changes to drupalcode even if we know they will never be merged just so we can share them? It makes sense to target 11, but that version won't see a stable release for a while now, so if the rules have changed, I'd just like to make sure I am following them, while still providing solutions for those who need them in the meantime.

The current 11.x branch is Drupal 10.3, see https://www.drupal.org/about/core/blog/new-drupal-core-branching-scheme-... ( And even the real 11.x is actually not that far out). In most cases, merge requests against 11.x also apply against the current stable version. That's also the case here, I just verified that https://git.drupalcode.org/project/drupal/-/merge_requests/4178/diffs.diff applies on 10.2.x for me with both git apply -3 and patch -p1.

@Berdir Okay, I see that now. That sort of makes sense, although it seems very confusing for the versions to not match.

In any case, I already did attempt to patch that 4178 diff you linked, but for me, it fails to apply to 10.2.0 (I had to do this upgrade before 10.2.1 for change control reasons). What would be the proper protocol in that situation (where no available patch or diff applies but you don't actually expect a new merge request to be accepted).

The MR needs to be updated after #3414259: Convert FieldTypeTest into a Kernel test.

I solved the MR conflict for 11.x

However, we need an MR for 10.2.x as the diff from 4178 MR doesn't work with Drupal 10.2.x

I have added the re-rolled patch for Drupal version 10.2.x and tested it with Drupal 10.2.2. The patch applies cleanly.

@abhinesh:

As pointed out in Comment #129, it is confusing to have patches posted on an issue with a MR. But it is also helpful to have patches that apply to specific, tagged versions, as pointed out in Comment #133.

As a compromise, I think that any patches should indicate the version where they apply. Would you mind uploading a renamed version of your patch and hiding the existing one? Personally, I would use something like 2999491-141-10.2.2.patch

I guess you removed the conflicting change to core/modules/block_content/tests/src/Functional/Views/FieldTypeTest.php so that the patch applies to both 10.2.2 and 10.2.x. (See Comment #136.) That is worth mentioning.

@carolpettirossi:

Since you closed the new MR, I guess you already noticed that the current MR does apply to both 10.2.x and 11.x.

On my current project, we are updating from the patch attached to Comment #82. That patch adds the permission "create reusable blocks" (in the layout_builder module) and the more recent ones. We got errors until we removed that permission from user roles: see Permissions must exist.

#3041174: Adjust Layout Builder permission checking for inline blocks once more granular block permissions exist is finally getting some attention. I am adding it as a related issue.

MR 4178 had conflicts from #3035189: Abstract the shared parts of DefaultsEntityForm and OverridesEntityForm.

I rebased the feature branch on 11.x and resolved merge conflicts. This was challenging, since the MR included some duplicate commits and some that reverted changes from earlier commits. I may have made some mistakes when resolving merge conflicts. I decided to squash some commits so it will be easier to rebase the next time.

The commit "Fix phpcs reported issues" included changes to several files that had no other changes in the overall MR. I reverted those changes. That makes the MR simpler: not a big change to the number of lines, but it touches fewer files.

I did a quick test of basic functionality, adding one inline block and one reusable block.

Thank you for your work on this patch, however it no longer applies for me on Drupal 10.2.4. Using:

https://git.drupalcode.org/project/drupal/-/merge_requests/4178.diff

Looks like it is just in the use statements and the class comment.

I would like to help fix this but I don't know the proper procedure in a situation like this. I am going to download the diff, fix it, and use a local copy/patch for now as I need this today. I would like to know how I can help in a situation like this in the future though.

@JeffMattson:

Have you tried using the patch attached to Comment #141? It applied to Drupal 10.2.2, so it will probably also apply to 10.2.4.

Yes! The patch on Comment #141 works for 10.2.4. Thank you!

Patch #141 is not working on 10.1.8 so I have rebase the patch agained 10.1.8.

Attached will work for 10.1.x.
#141 will work for 10.2.x

Hello,

For information, there is on conflict with patch #4 from issue 3305126.

See composer install -vvv

Hunk #3 FAILED at 261.

1 out of 3 hunks FAILED -- saving rejects to file modules/layout_builder/src/Element/LayoutBuilder.php.rej

To solve it I had to apply first the #148 patch and then the patch from the other issue (by ordering patches in composer), thus the second one can apply successfully with an offset as it is declared as one continuous block.

I think it will be the same for #141 patch for 10.2.x

Hi,

On a project we had applied patch from comment 82 on Core 10.1

Patch from comment 141 has too much differences in terms of structure and the intermediary step to choose inline or reusable block provokes too many side effects on the project.

I am not sure if it is because of the hook_event_dispatcher modules that is triggered too early, but other modules (contrib and custom) altering the add and update blocks forms, may want to use the "getCurrentSection()" method and I had fatal errors because $this->delta was null at this moment. Commenting that out, I had then plenty of JS/ajax errors and no more time to that dig out.

So I took patch from comment 82 and then rebased it on 10.2.x, so the MR that I closed immediatly, just to generate the patch file.

Attaching the patch file for composer usage.

Can we figure out a way to not hardcode the form mode to 'edit'? I'm running into the same issue mentioned in #3052551: Use a different form mode when editing block content in an inline block.

Howdy all,

Hopefully this is the place for this.
We're using 10.2.5 currently with the patch from #141 (Also tested the following with #157 as well).

There is a issue we're running into with making inline blocks into reusable blocks.
If an inline block is made reusable and placed on multiple pages, but the original page is deleted then the block becomes broken.
This is happening after the layout_builder_cron() is run. It seems to disregard whether the block has the 'reusable' column set to '1' and removes the block from the database.

Steps to recreate:
1. Create a page
2. Create an inline block (any type)
3. Save the layout
4. Set the inline block to reusable
5. Create a new page
6. Add the reusable block to the new page
7. Delete the original page
8. Run Cron
9. The reusable block should be broken with text similar to "This block is broken or missing. You may be missing content or you might need to enable the original module."
10. In recent log messages you should see the block is broken after the layout_builder_cron() has completed its execution.

This only happens with inline-to-reusable, so if a block is created as reusable from the start there is no issue.
It also only occurs once the original page that the block was created on is deleted. If you delete the block from the original page, but don't delete the page the error will not happen. But as soon as you delete that page, whether it has the block or not, the block will break.

We haven't been able to solve this yet and we're hoping others might test and see if it's happening in their environments as well.

Edit: It looks like the set reusable functionality isn't removing the inline block's information from the inline_block_usage table. So when the layout_builder_cron() runs it sees that the original node with the inline block doesn't exist anymore so it deletes the block.

Hi,

Same comment and patch as 157 but for core 10.3.

New patch because of NestedArray class imported twice in a file.

Added a commit to the PR to address #158.

Added a commit to the PR to address #159 and remove any existing usage when a block is made reusable.

Hello,
Since Drupal 10.3, the patch 2999491--reusable-title-display--56 haven't been working any more.
I'm posting a new solution.
Regards

My patch is not working, only on local and I don't know why. Please don't use it.

I'm curious why translations and multilingual sites aren't being mentioned or considered. When I create a reusable block and translate it to French, if I want to add it to the French layout, the add form will shows the original English values. Even if I use the Layout Builder Asymmetric Translation module to translate it with the page, the block appears in French on the front end, but clicking edit shows the original English form with English values, the block label will also be displayed in English.

The patch from comment 56 worked well for me, but I encountered translation issues. I enhanced the patch to account for translations.
And now the layout builder sidebar displays the translated block titles instead of the English ones.

Hello All,

We are heavily using this patch in our projects and I see its hard to maintain as a patch on production sites, I created a module to help those who want to upgrade to Drupal 11 and keep this function runing.

https://www.drupal.org/project/layout_builder_reusable_blocks

Welcome to help me on the module.

Regards

Re-rolled patch for Drupal 11.3.x

The patch #168 was failing to apply on Drupal 11.3.x due to changes in ConfigureBlockFormBase.php.

Issue:
The original patch expected the file without WorkspaceDynamicSafeFormInterface which was added in Drupal 11. The context lines in the patch no longer matched the current file structure.

Changes in this re-roll:
- Updated context lines in ConfigureBlockFormBase.php to account for WorkspaceDynamicSafeFormInterface (line 14)
- Adjusted line numbers for ChooseBlockController.php hunk
- No functional changes from the original patch #168

Tested on:
- Drupal 11.3.2
- PHP 8.x

Attached a rerolled patch against 10.5.x from #141

Patch is based on #162

Changes:

• Enforced new revisions for reusable blocks: Reusable blocks now always save as a new revision to ensure compatibility with BlockContent::getDerivativeDefinitions.
• Improved error handling in Layout Builder: SectionComponent::getPluginId now returns a "broken" handler instead of throwing a PluginException, preventing fatal errors when plugins are missing.
• Restricted default template usage: Removed the ability to add reusable blocks directly to default templates.

Corrected the previous patch

Attached a rerolled patch against 10.5.x from #141.
Issue:
Getting an error here on clicking the link of a block "make block reusable" in layout builder
Drupal\layout_builder\SectionComponent->getPluginId() (line 218 of /var/www/html/docroot/core/modules/layout_builder/src/SectionComponent.php)
If this is failing, the id key is missing from the component's configuration array.

Changes in this re-roll:
We need to modify the handleSectionStorage method in core/modules/layout_builder/src/Form/MakeReusableBlockForm.php to ensure the configuration update is atomic and includes the provider key, which 10.5.x now requires for block plugins

The patch also modifies submitForm in core/modules/layout_builder/src/Form/ConfigureBlockFormBase.php.

We were looking to implement this feature in our Drupal 11 project and were glad to come across this issue. Many thanks to #169 (https://www.drupal.org/project/drupal/issues/2999491#comment-16189747
) for developing the contributed module https://www.drupal.org/project/layout_builder_reusable_blocks
.

We hope to see this functionality incorporated into Drupal core in the near future.

I've tried the patch from #176, but got the following error:

Error: Call to undefined method Drupal\block_content\Entity\BlockContent::setOwnerId() in Drupal\layout_builder\Form\ConfigureBlockFormBase->submitForm() (line 386 of /var/www/html/web/core/modules/layout_builder/src/Form/ConfigureBlockFormBase.php).

Removing ->setOwnerId() fixes the issue.
I've attached the updated patch with the line removed.