Add reusable option to inline block creation

Created on 13 September 2018, over 5 years ago
Updated 3 April 2024, 10 days ago

Problem/Motivation

We are now able to create inline blocks from the Layout Builder but they are always set to non-reusable. It would be nice to have a checkbox to decide of the block should be reusable or not, similar to functionality in FPP for Drupal 7. This would allow editors to create reusable blocks while editing layouts for individual pieces of content, rather than having to jump to the "Block layout" section.

Proposed resolution

  • Add a "Reusable" checkbox to inline_block plugins in Layout Builder
  • If checked, it'll expose an "Admin title" field for naming the reusable block
  • Once saved, it'll convert the underlying content block entity to reusable, and convert the inline_block plugin on the Section to a block_content plugin
  • Allow editing the content block referenced by block_content plugins, and include a warning that let's editors know that their changes will be reflected globally

Completed tasks

@tbd

Remaining tasks and feedback

  1. See - 📌 Improve the page building experience Active
  2. 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.
  3. 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.
  4. 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.
  5. 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.
  6. 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.
  7. UX Feedback from #2999491-87: Add reusable option to inline block creation : Fix UX on status message.
  8. 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.
  9. 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?
  10. 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.
  11. From #2999491-80: Add reusable option to inline block creation : 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.
  12. Conduct usability review (again), per #2999491-88: Add reusable option to inline block creation .
  13. Write tests

Follow-up issues

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 Active :

...It might be a good idea to have a follow-up issue to decide how to choose when adding a new block.

User interface changes

  • Adds checkbox and admin title field when editing inline_block in Layout Builder:
  • Adds content block fields and warning when editing block_content in Layout Builder:

API changes

None.

Data model changes

None.

Per #2999491-88: Add reusable option to inline block creation :

It might be a good idea to have a follow-up issue to decide how to choose when adding a new block.

Release notes snippet

@todo

Feature request
Status

Needs work

Version

11.0 🔥

Component
Layout builder 

Last updated 1 minute ago

Created by

🇨🇦Canada a_mitch

Live updates comments and jobs are added and updated live.
  • Blocks-Layouts

    Blocks and Layouts Initiative. See the #2811175 Add layouts to Drupal issue.

  • Needs usability review

    Used to alert the usability topic maintainer(s) that an issue significantly affects (or has the potential to affect) the usability of Drupal, and their signoff is needed. When adding this tag, make it easy to review the issue. Make sure the issue summary describes the problem and the proposed solution. Screenshots usually help a lot! To get sign-off on issues with the "Needs usability review" tag, post about them in the #ux channel on Drupal Slack, and/or attend a UX meeting to demo the patch and get direct feedback from designers/UX folks/product management on next steps. If an issue represents a significant new feature, UI change, or change to the general "user experience" of Drupal, use Needs product manager review instead. See the scope of responsibilities for product managers.

  • Needs tests

    The change is currently missing an automated test that fails when run with the original code, and succeeds when the bug has been fixed.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates : some issue and comment data are missing.

  • 🇮🇳India Gauravvv Delhi, India

    Updated attributions

  • last update 10 months ago
    Custom Commands Failed
  • Issue was unassigned.
  • Status changed to Needs review 10 months ago
  • 🇺🇦Ukraine dinazaur

    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.

  • Status changed to Needs work 10 months ago
  • 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.

  • 🇳🇱Netherlands seanB Netherlands

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

  • last update 8 months ago
    Custom Commands Failed
  • 🇮🇳India _utsavsharma

    Tried to fix CCF in #112.

  • last update 8 months ago
    29,897 pass, 10 fail
  • Status changed to Needs review 8 months ago
  • last update 8 months ago
    Custom Commands Failed
  • 🇳🇱Netherlands seanB Netherlands

    Another patch to try and fix some of the tests.

  • 31:26
    29:20
    Running
  • 🇳🇱Netherlands seanB Netherlands

    Whoops, pressButton should be called on $page.

  • 🇺🇦Ukraine dinazaur

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

  • 🇺🇸United States johnpicozzi Providence, RI

    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.

  • Status changed to Needs work 7 months ago
  • 🇺🇸United States smustgrave

    From reading 📌 Improve the page building experience Active definitely think this functionality is wanted in core.

  • 🇮🇳India abhaisasidharan

    I've fixed the failed tests in #115 Add reusable option to inline block creation Needs review .
    Agree with @johnpicozzi. I would love to see this in core.

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update 7 months ago
    Custom Commands Failed
  • 🇮🇳India abhaisasidharan

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

  • Open in Jenkins → Open on Drupal.org →
    Environment: PHP 8.1 & MariaDB 10.3.22
    last update 7 months ago
    30,348 pass, 4 fail
  • last update 6 months ago
    30,350 pass, 4 fail
  • last update 6 months ago
    Custom Commands Failed
  • Pipeline finished with Failed
    6 months ago
    Total: 172s
    #25939
  • last update 6 months ago
    Custom Commands Failed
  • Pipeline finished with Failed
    6 months ago
    Total: 183s
    #25942
  • last update 6 months ago
    Custom Commands Failed
  • Pipeline finished with Failed
    6 months ago
    Total: 171s
    #25955
  • last update 6 months ago
    Custom Commands Failed
  • Pipeline finished with Failed
    6 months ago
    Total: 714s
    #25961
  • 🇺🇸United States codechefmarc

    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!

  • Status changed to Needs review 4 months ago
  • 🇮🇳India Gauravvv Delhi, India

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

  • Pipeline finished with Failed
    4 months ago
    Total: 185s
    #59747
  • Status changed to Needs work 4 months ago
  • 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.

  • 🇺🇸United States codechefmarc

    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.

  • 🇺🇸United States chrisgross

    @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.

  • 🇺🇸United States chrisgross

    #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. If anyone can get the newer commits working on 10.2.x, please share it.

  • Pipeline finished with Failed
    3 months ago
    Total: 187s
    #74535
  • 🇺🇸United States capysara

    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.

  • 🇺🇸United States capysara

    capysara changed the visibility of the branch 2999491-add-reusable-option2 to hidden.

  • 🇺🇸United States capysara

    capysara changed the visibility of the branch 11.x to hidden.

  • Pipeline finished with Failed
    3 months ago
    #74545
  • 🇺🇸United States chrisgross

    @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 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 it can't see it 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 be out 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 that need them in the meantime.

  • Pipeline finished with Failed
    3 months ago
    #74554
  • 🇨🇭Switzerland Berdir Switzerland

    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.

  • 🇺🇸United States chrisgross

    @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?

  • Pipeline finished with Failed
    3 months ago
    Total: 586s
    #75627
  • 🇺🇸United States benjifisher Boston area

    The MR needs to be updated after 📌 Convert FieldTypeTest into a Kernel test Fixed .

  • First commit to issue fork.
  • Pipeline finished with Failed
    3 months ago
    Total: 615s
    #81029
  • Pipeline finished with Failed
    3 months ago
    Total: 491s
    #81035
  • 🇧🇷Brazil carolpettirossi Campinas - SP

    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

  • 🇮🇳India abhinesh

    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.

  • 🇺🇸United States benjifisher Boston area

    @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 .

  • 🇺🇸United States benjifisher Boston area

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

  • Pipeline finished with Failed
    28 days ago
    Total: 199s
    #120946
  • 🇺🇸United States benjifisher Boston area

    MR 4178 had conflicts from 📌 Abstract the shared parts of DefaultsEntityForm and OverridesEntityForm Fixed .

    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.

  • Pipeline finished with Failed
    28 days ago
    Total: 546s
    #120964
  • Pipeline finished with Failed
    27 days ago
    Total: 583s
    #121000
  • 🇺🇸United States jeffreysmattson

    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

  • 🇺🇸United States benjifisher Boston area

    @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.

  • 🇺🇸United States jeffreysmattson

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

  • 🇸🇬Singapore druprad

    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 Add bundle information to LayoutBuilder Postponed: needs info .

    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

Production build https://api.contrib.social 0.62.1