Move Custom block library to Content

Be sure to check in with yoroy first, as this is a Ux thing

We did :)

This is not an new feature. With D7 Bean module you create block types from Structure and create/add instances from Content menu.
May be using the same menu items labels as well could help?

We need a whiteboard and catchup to decide what goes where exactly. I like the bean thing above, though.

+++ b/core/modules/block_content/config/optional/views.view.block_content.yml
@@ -1,13 +1,15 @@
-langcode: en
+ngcode: en

Not sure why the diff is showing "+ngcode" is that weird?

Here is an update path for this.

I agree with the need for this. I just completed user training on my first Drupal 8 site and got comments like this:

"Where are the blocks again?"
"If custom blocks are content, then why aren't they just under content? Why are they buried in another part of the site?"

Thanks for that feedback @Chris Burge. Lets do this.

I think this is a great idea.
Rerolling #16, please check the diff as there was a conflit I had to manually fix on block_content/block_content.routing.yml

Here is the update path test.

Fixing some failing tests...

+++ b/core/modules/block_content/tests/src/Functional/BlockContentTypeTest.php
@@ -121,7 +121,6 @@ public function testBlockContentTypeEditing() {
     $this->assertBreadcrumb('admin/structure/block/block-content/manage/basic/fields', [
       $front_page_path => 'Home',
       'admin/structure/block' => 'Block layout',
-      'admin/structure/block/block-content' => 'Custom block library',
       'admin/structure/block/block-content/manage/basic' => 'Edit Bar',
     ]);

This made me realize that when you go to configure the fields for a block type, the path no longer makes sense.

Also, the block tab is not showing up yet.

I wonder if this has anything to do with #30 #2804195: Views does not create parent local task for a default local task

This should fix the last failing test.

Hi,

I have created a patch for it to be moved to the Content Area.

In my opinion it totally make sense having "Content" in content area.

Since custom blocks are primary building blocks of Drupal. I reckon this needs quite urgent look to be merged to latest 8.

Regards,
Amjad

Sorry #35 was wrong patch...

@amjad1233:

Before classifying this issue as "major", please review Priority levels of issues and explain how the issue meets the criteria there.

A change like this should not be made in a patch release (8.5.x). It is appropriate to target the 8.6.x branch. See Allowed changes during the Drupal 8 and 9 release cycles.

I am re-uploading the patch from #33, which already passes tests. I do not have time for a full review now, but I notice that the patch from #33 includes most of the changes from the one in #37. I also see a few surprising changes in #37, like this:

+++ b/core/modules/block_content/config/optional/views.view.block_content.yml
@@ -468,7 +468,7 @@ display:
         - url
         - url.query_args
         - user.permissions
-      max-age: 0
+      max-age: -1

Why do we want to change caching behavior? Even if we do want to change it, how is it in scope for this issue?

We discussed this at the weekly UX meeting today. Although most everyone agrees that this issue is a good idea, it will be hard (very hard) to get a change like this committed without some user testing.

A possible way forward would be to combine this with some other improvements in a contrib module, or possibly an experimental one. Of course, this means implementing each improvement twice, once as a core patch and once as part of the module. Once we have such a module, we can use it for user testing.

I think that #2809291: Add "edit block $type" permissions, already listed as a related issue, would be a good candidate for such a module. Even better would be a more comprehensive review of the URL and menu structure, or information architecture (IA) of the Drupal admin UI. There are some issues listed under #2888657: [meta] Less confusing and more consistent wording needed in module/theme add/install/update that might be good candidates.

#2987964: Move custom block types admin link to admin/structure has a different solution for this

Just a reroll.

1. 
   I think we want the "Custom Block Library" to also show up in the tabs

2. +++ b/core/modules/block_content/block_content.module
   @@ -33,7 +33,7 @@ function block_content_help($route_name, RouteMatchInterface $route_match) {
   -      $output = '<p>' . t('Each block type has its own fields and display settings. Create blocks of each type on the <a href=":block-library">Blocks</a> page in the custom block library.', [':block-library' => \Drupal::url('entity.block_content.collection')]) . '</p>';
   +      $output = '<p>' . t('After creating a custom block type, you can create content of that type by clicking <em>Place block</em> on the <a href=":block-layout">Block layout</a> page or in the <a href=":block-library">Custom block library</a>.', [':block-library' => \Drupal::url('entity.block_content.collection'), ':block-layout' => \Drupal::url('block.admin_display')]) . '</p>';
   

   The actual help text changes besides the routes seem out of scope here

3. In #2987964: Move custom block types admin link to admin/structure we are moving block types under Structure because with #2957425: Allow the inline creation of non-reusable Custom Blocks in the layout builder core provides another way to use block types without using the block layout.

   re #39

   A possible way forward would be to combine this with some other improvements in a contrib module, or possibly an experimental one.

   I wonder if it would be easier to commit #2987964: Move custom block types admin link to admin/structure first and then do this issue.

Great to see activity here!

Re #43.1 Yep, mentioned this on #30 - I think it's because of #2804195: Views does not create parent local task for a default local task so perhaps we should pause this one until that gets fixed?

I would like to second #5. "Custom block library" makes some sense in its current location, but very much would like to see the labels just become "Add block", "Blocks", etc. The only issue might be the distinction with dedicated, "coded" blocks (although "Custom block" might start to become more appropriate for those than for content-based blocks).

I wonder if it would be easier to commit #2987964: Move custom block types admin link to admin/structure first and then do this issue.

Agreed!

Patch updated with:

1. Rerolled to the latest 8.8 branch
2. Added link task to the admin content. The admin view is optional, it overrides the default entity list builder when enabled
3. The message for empty block list now follows the same format as in content, media or files: "No custom blocks available."
4. Updated the URL to the block content library in tests that made them fail
5. Added test coverage for the new admin content local task

Interdiff fails because different baselines, but I managed to generate one to easily see the mentioned changes.

#2987964: Move custom block types admin link to admin/structure does not block this, but would be appropriate to be committed under the same CR

This is great, but it makes #2862859: Create a top level, extendable, "Content" admin menu route that behaves like "Structure" even more of a pressing need, since once this is fixed, there will be TWO tabs here provided by core which are potentially inaccessible if node module is disabled.

I didn't know that issue, added as related here.

... once this is fixed, there will be TWO tabs here provided by core which are potentially inaccessible if node module is disabled

We have also the Comments and Files tabs from core. I suggested a solution for it (#2862859-31: Create a top level, extendable, "Content" admin menu route that behaves like "Structure") that would not affect the changes makes here.

Patch not working in drupal-9.3.x-dev showing error
Needs to re-roll

picking it.

+++ b/core/modules/block_content/tests/src/Functional/Update/BlockContentReusableUpdateTest.php
index 9f9f1d1..6915333 100644
--- a/core/modules/block_content/tests/src/Functional/Update/BlockContentUpdateTest.php

+++ b/core/modules/block_content/tests/src/Functional/Update/BlockContentUpdateTest.php
@@ -67,4 +68,28 @@ class BlockContentUpdateTest extends UpdatePathTestBase {
+  public function testMoveCustomBlockLibrarytoContent() {

I have tried to add most of the changes in #48 patch but I am not sure where these changes need to add in the current patch as core/modules/block_content/tests/src/Functional/Update/BlockContentUpdateTest.php file is not available in code.

Patch not working in drupal-9.3.x-dev

Rerolled
Added a fixture and update test for the post_update hook
Fixed some test failures that were using the old path

Seems to be a random FunctionalJavascript error moving back to NR

Thank you for picking that one up and working on it! I've successfully applied the patch in #64 to Drupal 10.1.0-dev. There are three details I've noticed.

1. There are currently two pages for the custom block library: /admin/content/block-content and /admin/structure/block/block-content (see screenshot). the page at /admin/structure/block/block-content should better be removed.

2. as illustrated in the screenshot as well the custom block library page under structure has a filter option while the custom block library page under content is missing one. so adding a filter option in case the views module is enabled according to #7 would be good.

3. The position of the custom blocks tab between content and comments looks correct. but i've noticed in case the admin toolbar module is installed the order of the tabs differs to the order of corresponding menu section.

probably reasonable to open a follow up issue in the admin_toolbar queue as soon as this issue lands? or would it make sense to open an issue already now there?

So when I apply the patch, running drush updb, and cr I'm not seeing /admin/structure/block/block-content I get a 404

And yes I agree there should be a follow up on admin_toolbar that will be postponed until this lands.

oh no i forgot the drush updatedb. i've only run drush cr. I've retried and #67.1 is invalid. /admin/structure/block/block-content got properly removed here as well. my bad, apologies!

No worries! Moving back to NR

Usability review

We discussed this issue at #3316853: Drupal Usability Meeting 2022-10-28. That issue will have a link to a recording of the meeting.

For the record, the attendees at today's usability meeting were @AaronMcHale, @benjifisher, @rkoller, @shaal, and @worldlinemine.

The last time this issue came up at a usability meeting (see #39) I suggested usability testing. That would be nice, but it does not look as though it is going to happen. Perhaps the next best thing is to spread the idea around and get some fedback. Ask your local Drupal meetups what they think. Mention it in a BoF session at the next Drupal Camp you attend. Who knows, maybe someone will get excited by the idea and do some real user tests.

Leave a short comment on this issue describing the collective response. Or leave a long comment if new ideas come up.

For now, I created #3318110: [meta] Reorganize Block items in the administration menu, and I am collecting this issue and a few others as children of that issue. I think all of these issues should be considered together.

I am adding tags for the required approvals.

This seems like a great idea. Definitely approval and hope it can be added.

Had to update a test since https://www.drupal.org/project/drupal/issues/2350939 landed

1. +++ b/core/modules/block_content/block_content.links.menu.yml
   @@ -0,0 +1,5 @@
   +  title: 'Custom block library'
   ...
   +  description: 'Create and edit Custom block content.'
   
   +++ b/core/modules/block_content/block_content.links.task.yml
   @@ -1,17 +1,11 @@
   +  title: Custom blocks
   

   I have seen talk of renaming this to just 'Blocks' where did we end up on that?

2. +++ b/core/modules/block_content/block_content.post_update.php
   @@ -20,3 +22,34 @@ function block_content_removed_post_updates() {
   +        $display['display_options']['path'] = 'admin/content/block-content';
   

   I think we should check that this path is what the module ships by default before we update it.

   Sites own their config and may have moved it elsewhere already

3. +++ b/core/modules/block_content/block_content.post_update.php
   @@ -20,3 +22,34 @@ function block_content_removed_post_updates() {
   +        $menu =& $display['display_options']['menu'];
   +        $menu['description'] = 'Create and edit custom block content.';
   +        $menu['expanded'] = FALSE;
   +        $menu['parent'] = 'system.admin_content';
   

   Same for these changes

4. +++ b/core/modules/block_content/block_content.post_update.php
   @@ -20,3 +22,34 @@ function block_content_removed_post_updates() {
   +        $display['display_options']['empty']['area_text_custom']['content'] = 'No custom blocks available.';
   +        $view_updated = TRUE;
   

   Is this out of scope?

5. +++ b/core/modules/block_content/tests/src/Functional/Update/BlockContentUpdateTest.php
   @@ -0,0 +1,49 @@
   +    $this->runUpdates();
   

   Let's run some assert before the test to make sure things are in the state we expect.

   e.g. we're not using the fixture file anywhere that I can see, so either its not needed, or the site isn't in the state we expect before we run the updates.

Also, just stating I'm +1 for this change

@larowlan, thanks for the review!

#77.1: I think the current plan is to change the title from "Custom blocks" to "Blocks" as part of #3318549: Rename Custom Block terminology in the admin UI.

That plan is not set in stone, but there was a long discussion in Slack before settling on it. Part of the idea of separating the issues is that we can have a separate discussion on that issue about the appropriate label.

Big +1 to this change. I address this already on any site where clients are editing blocks by adding a second menu link to the custom block library under 'Content'.

I also agree with the change from 'Custom block library' to 'Blocks'. The term 'library' is pretty random here as it's not used anywhere else in Drupal AFAIK and 'Custom blocks' is meaningless to content editors.

I have to Upload the patch
And Addressed #77 - point - 1,3,4
Needs work on points 2,5

The changes in #80 (addressing #77.1 and #77.3) should be undone, please see #78 for justification. We'll keep the change in that issue.

I really think we should fix #2862859: Create a top level, extendable, "Content" admin menu route that behaves like "Structure" first.

77.1 = will be done in another ticket
77.2 = added quick check
77.3 = will be done in another ticket
77.4 = don't think it's a problem to be here
77.5 = added quick assertion

Thought I think https://www.drupal.org/project/drupal/issues/2987964 needs to land first.

Updated IS for where we're at on this. Thanks @smustgrave for the quick work and updates to fix #80.

Per #82 and #83, we need checks on:

1. Confirm whether #2987964: Move custom block types admin link to admin/structure and #2862859: Create a top level, extendable, "Content" admin menu route that behaves like "Structure" are blockers for this issue.
2. Review the patch in #2862564-80: Move Custom block library to Content.

Joachim has a good point in #82 – these issues were first developed together as part of the same work. One kind of needs the other.

I don't necessarily see #2862859: Create a top level, extendable, "Content" admin menu route that behaves like "Structure" being a hard blocker, partly because that issue has been stuck for a long time, but mainly because we're just following existing precedent here. Under Content there's already a tab for Media, here we're simply adding a similar tab for (Custom) Blocks. Seems more like a nice to have to me.

#2862859: Create a top level, extendable, "Content" admin menu route that behaves like "Structure" would be a significant change, and if anything by doing this issue we simply make the case stronger for getting that issue done.

> but mainly because we're just following existing precedent here. Under Content there's already a tab for Media, here we're simply adding a similar tab for (Custom) Blocks. Seems more like a nice to have to me.

Yes, but committing this issue makes the problem in #2862859: Create a top level, extendable, "Content" admin menu route that behaves like "Structure" worse:

- there are too many tabs at /admin/content
- if you disabled node module, you can't get to any of them

By committing this we make the case stronger for #2862859: Create a top level, extendable, "Content" admin menu route that behaves like "Structure", yes -- by making an existing UX bug worse.

if you disabled node module, you can't get to any of them

Actually that's not true, the /admin/content route and links are actually provided by the system module in system.routing.yml, system.links.menu.yml and system.links.task.yml.

As you can see from this screenshot, on a Drupal 10.1.x site with Comment and Media installed but Node uninstalled, there is still a clear route to access the Media and Comment tabs, the only issue I can see is that the Content tab does not have much information other than a message saying there are no administrative items.

Even in the case of permissions, to get to the Comments tab under Content, the only permissions the user appears to need are:
- Administer comments and comment settings
- Use the administration pages and help

And optionally:
- Use the toolbar
- View the administration theme

So, while not the most optimal experience, I don't think it's a hard blocker to this issue from a UX perspective.

there are too many tabs at /admin/content

This is of course a discussion for that specific issue, but I also don't believe that is a strong blocker because it is very subjective. In which contexts are there "too many tabs", and couldn't the same logic be applied to menu links. In my screenshot there are four tabs right now, would adding a fifth one be "too many". It might be, but it's very context dependent (screen size, etc), and on mobile the tabs contract into a dropdown list of links (in Claro). I'm sure there's other parts of the Admin UI with more than four or five tabs right now. One could also make the argument that the proposed solution in that issue is no better because in it increases the number of clicks to get to the list of content (from 1 to 2).

Anyway, my point is that I still don't see a strong case for blocking this issue on that issue, particularly given that it's been stuck for so long.

I agree with #86 and #88, to be honest I don't see #2862859: Create a top level, extendable, "Content" admin menu route that behaves like "Structure" landing for quite a while. I don't even think we've nailed down which solution to go with there. It's also a highly disruptive change (I can see a lot of push back coming from the community) given that admin/content has been the go to for a list of nodes in Drupal for a long time.

I've presented the current plan for moving and untwining the block related pages in Core at the weekly Lean Coffee Table at the Drupal User group in Munich to the attendees @drubb, @franz-m, @it-cru, @jurgenhaas, @martin-mayer as well as to the Fox Valley Computer Professionals attendees Anthony E. Scandora, @bsnodgrass, Sally Gradle, @TBone242.

With both groups in the context of the moving the custom block library to content one key need gradually manifested while going through all the proposed changes and the screenshots illustrating it. Everyone realized you have for one the blocks Core ships with, then you have the reusable custom blocks, then you have the not reusable custom blocks only available in the context of layout builder and then you also have blocks that are made available with every contrib module that ships with any blocks. While writing up the comments I've installed the Gutenberg module by accident and discovered the following overview that illustrates the problem pretty well. :

In that installation the only two blocks that are visible inside the custom block library are first block and another standard block. but the consensus and desire after realizing in both groups was to make all available blocks visible and accessible in the custom block library even if it means that 100s of blocks of different types and sources are shown there.
Everyone wanted to be in control. if the blocks would be labeled correctly in the list view and the available options would differ based on the type that would be fine. but that way the users would be in control. it would also solve another problem someone mentioned. sometimes one has to search if a view has already created a certain block or if you have to create it on your own.

aside the fact that many of the different block types are not visible and accessible via the custom block library i guess the more underlaying problem is that people not really understand the difference between all those different blocks. there is no apparent sharpness of separation between the different block concepts. if there is the problem of clear differentiation users are getting confused why a block is called custom block here, and the custom blocks in layout builder are not shown in custom block library. then you also have the following issue making blocks created in layout builder reusable: https://www.drupal.org/project/drupal/issues/2999491 . overall it is more or less confusing if you try to stay in the loop.

making adjustments about what is shown in the custom blocks library is definitely out of the scope of this issue. but it is a more than valid and important point imho. aside that everyone is 100% inline moving the custom block library to /admin/content

I've read through all the new comments that came in during the last few days. i agree with #88 and #89. I've also tested uninstalling the node module with the patch in #83 applied. Same result like aarons only difference the custom blocks tab is next to the content one and fully functional. so i also wouldn't consider #2862859: Create a top level, extendable, "Content" admin menu route that behaves like "Structure" a hard blocker. even though i think it should definitely be discussed and worked on. @benjifisher already added the issue to the review queue for the ux meeting: https://www.drupal.org/project/drupal/issues/3320757#comment-14786871

Sounds like we've reached consensus on #2862859: Create a top level, extendable, "Content" admin menu route that behaves like "Structure" not being a hard-block. I've moved the issue into the Proposed resolution of the IS as a to-do alongside this issue.

I also adjusted the IS to note the patch in question for review is #83, not #80, as @smustgrave did a great job of applying feedback and creating the all-important interdiff.

Do we have an existing issue that would be helpful to place the comment from #2862564-90: Move Custom block library to Content in? That's helpful feedback for a UX improvement on custom block variants and I don't want that to get lost in this issue.

Remaining tasks, unless I'm missing any additional feedback:

1. Confirm whether #2987964: Move custom block types admin link to admin/structure is a blocker for this issue.
2. Review the patch in #2862564-83: Move Custom block library to Content.

re #92 there is no issue for that feedback yet. but instead of simply open an issue I would rather tend to discuss all the insights from the latest feedback sessions. cuz for example the idea of letting the custom block library display not just custom blocks touches several other issues. therefore i think it would make sense to have a broader discussion about the latest insights first. @aaronmchale already put discussing the results of his feedback session to the queue for the next ux meetings tomorrow (everyone is welcome to join - #3320757: Drupal Usability Meeting 2022-11-18). and in addition there could also be a discussion in #block-content on the drupal slack. that way the to be created issue or issues (i guess there might spin up one or two more) will be more focused with a clear proposed resolution.

cuz for example the idea of letting the custom block library display not just custom blocks

@rkoller are you referring to inline blocks added via layout_builder to display there too? Because that is not something that core would support since these blocks are designed as inline (i.e only used in the specific entity they're added to). If you want them to display though you can always add a custom view to override the page and don't add the Reusable filter, or override BlockContentListBuilder so it doesn't filter on the reusable property in getEntityIds

#2987964: Move custom block types admin link to admin/structure definitely is a blocker for this issue, because the Block Types sits under the Custom Block Library in terms of the routes/links, so it's not practical to do this issue before that one.

As soon as the block types ticket lands I’ll immediately pivot to this.

Started the MR. This includes #2987964: Move custom block types admin link to admin/structure already. So hopefully once that lands this just needs to be rebased and is ready for testing.

Also https://www.drupal.org/project/drupal/issues/2987964#comment-14815445 discussed removing Needs product manager review that I wonder if it applies here.

#2987964: Move custom block types admin link to admin/structure just landed so this is good to go!

Believe I addressed all the threads but left a few open for review.

I think we should address #3325167: Revisit the redirect to 'add block' form in the 'add block content' form either at the same time or after this issue.

The current behaviour of saving a block and getting redirect to Block Layout is weird but not a huge deal given that in 10.0 they sit next to each other, but once Block Content moves to Content, that redirect will become a much bigger problem.

My vote is to address after to keep this one moving. #3325167: Revisit the redirect to 'add block' form in the 'add block content' form shouldn't be blocked by anything so could probably be worked on now.

My vote is to address after to keep this one moving. #3325167: Revisit the redirect to 'add block' form in the 'add block content' form shouldn't be blocked by anything so could probably be worked on now.

Oh yeah I wasn’t suggesting we should block either issue. Just want to emphasise that doing this issue would make that issue more important.

I made a few more comments on the MR. Back to NW.

I am making some updates to the issue summary. We need to update the change record, so I am adding the issue tag for that.

Addressed the feedback.

Updated the change record

Added screenshots.

Hiding the files as this is being done in the MR.

Removing the needs product manager review tag since this is a self-contained usability improvement.

Thanks @catch!

Updated the after screenshot in the issue summary. the position of the custom blocks tab was on the far right instead of the intended position between content and comments.

@smustgrave:

Thanks for the updates. I think the only code changes we still need are for the help text.

The text on /admin/help/block_content is now

About

The Custom Block module allows you to create and manage custom block types from the Custom Block types page and content-containing blocks from the Custom blocks page. Custom block types have fields; see the Field module help for more information. Once created, custom blocks can be placed in regions just like blocks provided by other modules; see the Block module help page for details. For more information, see the online documentation for the Custom Block module.

Uses

Creating and managing custom block types

Users with the Administer blocks permission can create and edit custom block types with fields and display settings, from the Custom Block types page under the Structure menu. For more information about managing fields and display settings, see the Field UI module help.

Creating custom blocks

Users with the Administer blocks permission can create, edit, and delete custom blocks of each defined custom block type, from the Custom blocks. After creating a block, place it in a region from the Block layout page.

I would like to make a few changes:

1. Split up the first sentence into two parts. Put the second part (custom blocks) after the second sentence. Currently, the first paragraph switches between block types and blocks, which is confusing.
2. For consistency, add "page" after the first link under "Creating custom blocks".

On second thought, here is an alternative to (1). The current text under "About" duplicates the information under "Uses". It is a little out of scope, but I think it would be an improvement to

1. Shorten the first sentence: "The Custom Block module allows you to create and manage custom block types and content-containing blocks.
2. Delete the second and third sentences.
3. Add a link to the "Field module help" (along with the existing link to "Field UI module help") under "Creating and managing custom block types".
4. (optional) Add the phrase, "just like blocks provided by other modules", to the second sentence under "Creating custom blocks".

It is definitely out of scope for this issue, but I notice that the help text does not mention the per-block-type permissions added in #2809291: Add "edit block $type" permissions. Maybe we should mention those permissions here, or maybe that is too much information for this page.

I reviewed the Help Topics pages. I have just two suggestions:

1. A little out of scope, but on /admin/help/topic/block.overview, insert "manage" into the sentence, "The Custom Block module allows you to custom block types and custom blocks." under "Managing blocks overview".
2. On /admin/help/topic/block_content.add, the first step is
   

   In the Manage administrative menu, navigate to Custom block library.

   An earlier version of the MR added a link in the admin menu, but now that link is not there. Break this up into two steps: navigate to Content from the menu, then choose the Custom blocks tab.

Attempted to address help issues.

@smustgrave:

Maybe I should have been more explicit about some of the changes.

On /admin/help/block_content

Most of the changes look good, but add the phrase, "just like blocks provided by other modules", to the end of the second sentence under "Creating custom blocks":

After creating a block, place it in a region from the Block layout page, just like blocks provided by other modules.

On /admin/help/topic/block.overview

Keep the word "to" and add "manage":

The Custom Block module allows you to manage custom block types and custom blocks. See the related topics listed below for specific tasks.

On /admin/help/topic/block_content.add

Since Custom blocks is not in the administration menu, you cannot navigate to that link in the menu. It has to be two steps:

1. In the Manage administrative menu, navigate to Content.
2. Open the Custom blocks tab.
3. Click Add custom block. If you have more than one custom block type defined on your site, click the name of the type you want to create.

Or you could combine the first part of Step 3 with Step 2:

1. In the Manage administrative menu, navigate to Content.
2. In the Custom blocks tab, click Add custom block.
3. If you have more than one custom block type defined on your site, click the name of the type you want to create.

Addressed points in #114

There is just one more thing for /admin/help/topic/block_content.add: see the MR.

updated help topic.

@smustgrave:

Thanks for updating the Help and Help Topics pages. I reviewed the changes, and I am marking this issue RTBC.

I am also making several updates to the issue description:

• Mark the "remaining task" for the help pages as complete.
• Rewrite the Problem/Motivation section, now that #2987964: Move custom block types admin link to admin/structure has landed.
• Under "User interface changes", list the affected help pages.
• Update the screenshots in that section. The existing ones did not show the updated page title, and they did not show the Administration menu nor the path.
• Add screenshots of the parent pages to that section.

Thanks everyone; glad to see this is getting close!

I left a few comments on the MR.

Meanwhile, there are still references to the "Custom block library" that probably need to be cleaned up:

[ayrton:drupal | Tue 13:02:26] $ grep -ri "block library" * | grep -v "vendor" | grep -v "node_modules"
core/modules/block_content/block_content.post_update.php: * Moves the custom block library to Content.
core/modules/block_content/tests/src/Functional/Update/BlockContentUpdateTest.php:   * Tests moving the custom block library to Content.
core/modules/block_content/src/BlockContentInterface.php:   * When creating a new block content block from the block library, the user is
core/modules/block_content/src/BlockContentInterface.php:   * When creating a new block content block from the block library, the user is
core/modules/block_content/src/Entity/BlockContent.php:   * When creating a new custom block from the block library, the user is
core/modules/block_content/src/BlockContentViewsData.php:      'title' => $this->t('Empty block library behavior'),
core/modules/block_content/src/Controller/BlockContentController.php:      // We have navigated to this page from the block library and will keep track
core/modules/block/config/optional/tour.tour.block-layout.yml:    label: 'Custom Block Library'
core/modules/block/config/optional/tour.tour.block-layout.yml:    body: 'The block management screen also has an another tab on the top which is used to add Custom blocks. The name of the tab is "Custom block library". This tab ultimately provides a link to add custom blocks.'

The first two are part of the upgrade path, but the rest will need to be updated.

Thanks!

Tagging for RM review of:

It occurs to me to wonder if this is a disruption we should avoid. People could have this linked by path rather than route all over the place: Shortcuts, user-created menu items, in HTML. As external bookmarks. Should we consider providing the old aliases, possibly as redirects, and removing them in D11? The same might apply to the previous issue.

(Others' feedback on the point is also welcome.)

It occurs to me to wonder if this is a disruption we should avoid. People could have this linked by path rather than route all over the place: Shortcuts, user-created menu items, in HTML. As external bookmarks. Should we consider providing the old aliases, possibly as redirects, and removing them in D11? The same might apply to the previous issue.

This crossed my mind as well. I think we could provide a route with the old path, that does a redirect to the new path, and issues a @trigger_error() deprecation message. One drawback is we haven't finished #3159210: Support route aliasing (Symfony 5.4) and allow deprecating the route name yet, not sure how important that is since we're not changing the route for a path, but changing the path for a route.

All the renaming is going to happen separately in the next ticket.

All the renaming is going to happen separately in the next ticket.

Re #120, #121:

In #73, I added the tag for product manager review because this change is so disruptive. I was pleasantly surprised when @catch removed that tag in #108. Well, I was more surprised when the same thing happened on the earlier issue, #2987964: Move custom block types admin link to admin/structure.

Maybe I am missing something, but I do not see that #3159210: Support route aliasing (Symfony 5.4) and allow deprecating the route name is relevant. I think we are treating routes as intended: less subject to change than the paths and page titles associated with them.

I think a redirect and a new route would be pretty easy to implement: perhaps entity.block_content.collection.bc? But I wonder how effective it would be to trigger a deprecation notice. The only way to see it would be an automated test that did something like drupalGet('admin/structure/block/block-content'), right? A log message might be more helpful: a warning in Drupal 10, an error in Drupal 11, and then remove the BC route in Drupal 12.

Re #122:

@smustgrave, what issue do you have in mind?

I think we have agreed that #3318112: [PP1] Move "Block layout" from Structure to Appearance is the "next" issue.

After that comes #3318549: Rename Custom Block terminology in the admin UI, which includes changing "custom blocks" to "content blocks". But we are getting rid of "Custom block library" in the UI in this issue, so I think now is the time to update the references listed in #119. I am happy to do that myself, but then we will need to find someone else to take the reviewer role for this issue.

Moving the block layout and renaming don’t block each other so can happen at the same time.

A little more manual testing

I forgot to test with the Views module uninstalled.

Comparing to the screenshots in the issue summary (with Views installed) the Administration menu, paths, and breadcrumbs are all the same. The only differences I notice are

• On both parent and child pages, the Files tab (local task) is missing. It is only there if Views is enabled.
• On both pages, the filters are present only when Views is enabled.

Those are the same differences with the current MR or with 10.1.x.

I am setting the status to NW for #119 and the new threads on the MR.

Maybe I am missing something, but I do not see that #3159210: Support route aliasing (Symfony 5.4) and allow deprecating the route name is relevant. I think we are treating routes as intended: less subject to change than the paths and page titles associated with them.

I think a redirect and a new route would be pretty easy to implement: perhaps entity.block_content.collection.bc?

It's more that if we add that route, we'd want to immediately deprecate it for later removal, and adding a standard way to deprecate routes would help with that - for example it's unlikely but someone could reference the route name directly in code too. However since we don't expect anyone to do that, I don't think that issue really affects things, we can do a 'manual' deprecation (@trigger_error() or unsilenced warning) if the path is visited, and that is probably enough.

I wasn't that sure about the redirect route overall, but @xjm mentioned shortcuts, and shortcuts does indeed store internal:/admin/structure/foo rather than a route name, so that makes the redirect route a bit more pressing. Shortcuts stores internal paths so that people can reference say admin/content and still have that work even if they completely replace the path with a custom view, so I don't think we should try to do anything like updating shortcuts. E_USER_WARNING does seem like a better idea - anyone getting this redirect is very likely to be in a position to fix the link to the old path.

So to confirm we are doing renaming now in this ticket now?

And changing that text no blocks available follows what content does we can update but will be going against the grain

Updated "no blocks available" text.
Updated all instances of custom block library.

I confirmed that @xjm's suggestions on the MR have been applied. I also replied to one open thread.

I searched for "custom block library" (case insensitive). The only places I found it were in the new update function and the related test. These are the places referenced in #119:

The first two are part of the upgrade path, but the rest will need to be updated.

That leaves the question of whether to add a redirect from the old path (Comments #120, #121, #124, #127).

I am setting the status to RTBC to get the attention of the release managers. Should we handle the redirect as part of this issue or in a separate one?

Keep in mind that #2987964: Move custom block types admin link to admin/structure already made a related change of path. One option is to revert that change, re-open that issue, and re-postpone this one.

Another option is to handle both redirects as part of this issue.

We could also add a follow-up issue. It might also handle #3318112: [PP1] Move "Block layout" from Structure to Appearance, which is postponed on this issue. All three issues are connected to the same change record. (If not yet, then we we will fix that.)

I would suggest opening a blocking issue for the upgrade path for #2987964: Move custom block types admin link to admin/structure. I considered reverting it, and we'll do that if the upgrade path is not finished in time for 10.1.0-alpha1, but hopefully we won't have to. Then, address the upgrade path for this issue in this issue.

@xjm, thanks for the guidance. I added #3333383: Create a redirect for the new Block types path, and I am postponing this issue on it.

Based on the slack conversation with xjm in the contribute channel

Opened #3333394: [policy, no patch] Determine a best practice for providing BC for internal paths that change between minor releases

Added a route to this issue to redirect the old path to the new.

Since there's an issue with the MRs uploading a patch of the MR.

For any failures please do not reroll this issue is being worked on in the MR this is just for tests.

Needed to update test now that there is a redirect.

Random ckeditor5 failure

After recent discussion on #2317981: Move block content edit and delete routes under admin/content/block to improve IA for editors and fix breadcrumbs and at #3331562: Drupal Usability Meeting 2023-01-13, that issue is now a follow-up to this one.

Removing the [PP-1] here because we have a path forward for the BC layer along the lines of #3333383: Create a redirect for the new Block types path

Left a review on the MR.

Can we get a follow up to add a local action on the block library to 'add block', this would be postponed on both this issue and #3325167: Revisit the redirect to 'add block' form in the 'add block content' form

Great work folks

Addressed threads but left open for review

Opened #3333673: [PP-1] Add a local action on the block library per #141 ready for review.

@larowlan I don't think #3333673: [PP-1] Add a local action on the block library needs to be postponed on #3325167: Revisit the redirect to 'add block' form in the 'add block content' form, as far as I'm aware there's nothing in #3325167 that would block #3333673, since the former is just adding a link to create a block, while the latter is about what happens when you hit the save button.

@AaronMcHale sorry I should have been more detailed in why I think it blocks it. When you click 'add custom block' from the block library, you're sent to the place block form if you have administer blocks permission. If you don't, you end up on a 404 page. So the UX is pretty bad. We want to expand the permissions so that you can create/edit/delete custom blocks without the administer blocks permission, so without that change, the experience will be pretty bad for those without the administer blocks permission. Granted all this hinges off us getting the permission change in too, but I think that's our next priority.

Updated MR based on #3333383: Create a redirect for the new Block types path

It looks like the last piece of feedback on the MR was around that deprecation message which I've updated to match the other issue that was just committed :) hoping for an RTBC on this now!

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

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

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

Temporary issue with the bot

Sounds like this needs works

We need feedback on whether the empty arm of the upgrade path needs tests. To me this is slightly overkill but others might disagree.

@mstrelan and I already tested locally with the getOption approach and it failed as per the MR comment.

Last bits have been addressed.

Setting to RTBC as per my earlier testing and all feedback has now been addressed.

Updating issue credits, crediting those who provided meaningful reviews that changed the course of the patch, those who were involved in usability testing and review and those who made significant issue summary updates

Did another round of manual testing and another code review, looks good to me.

🎉🎉🎉

Committed d7c4cf1 and pushed to 10.1.x. Thanks!

Confirmed the change record already mentions this.

Updated the draft release notes to reference this and the block-types move.

Nice work everyone

See you in #3318549: Rename Custom Block terminology in the admin UI and #2317981: Move block content edit and delete routes under admin/content/block to improve IA for editors and fix breadcrumbs

Closed #2523154: Improve workflow for creating custom blocks moving over credit. Placing here as I don't have ability to add credit to the META.

very bad idea 👎