Add view tab + standard template for block content

So my current questions are

1. Is this the correct way?
2. Should we use that .bc route?
3. Do we need a "view block content" permission? Was having a hard time thinking the use case.
4. Is it okay I updated the previous .bc routes.

Left some more feedback on the MR

Thanks for the work on this... I could be misrepresenting, but I believe there were concerns raised with taking this approach in ✨ Ability to display block_content entities independently, also outside of Blocks Postponed: needs info however (eg. #2704331-19: Ability to display block_content entities independently, also outside of Blocks → ). Do we have a response to that at all?

This template is only going to be picked up when using the view route of the block. As a preview of what the block will look like if that helps

Addressed the feedback. Ready for another review

This template is only going to be picked up when using the view route of the block. As a preview of what the block will look like if that helps

Thanks for humoring me @smustgrave, that wasn't clear to me when looking at the IS and description of the linked module.

Just one minor thing and one question, left in the MR

Addressed feedback

Seems removing that render key caused some failures. Need to investigate why.

Lets see if that works.

Fixed!

@Wim Leers I added some screenshots those work?

Ah … so it's not just a Twig template, it's also new routes and tabs! That wasn't clear at all to me based on the title + summary 😅

Seems like #13 also contains pretty critical information that belongs in the issue summary. And perhaps also on the docblock in the Twig template itself? 🤔

Updated will update the template too!

Left a small in the template.

Left a comment on the MR

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.

Rebased

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

Still keen to understand why we need to manually add the list cache tag

Odd so removing that line worked now. No clue but I'll take it.

This looks good to me, thanks @smustgrave

What's the use case for the block content view tab? I understand editors may want to use it to preview the block but I'm not sure if the view tab is ideal for that.

Reading the description of the module, I wonder if 📌 Fall back to 'edit-form' as default $rel in EntityBase::toUrl() Fixed has helped solve part of the problem?

If we decide to do this, I think we may need to implement something similar to what we did in Media where this would not be enabled by default. It seems strange that anonymous users who may have access to view blocks, would be able to visit these URLs. See #3017935: Media items should not be available at /media/{id} to users with 'view media' permission by default → for the media issue.

Think you answered it. This is to provide editors the ability to preview blocks without having to place on a page first.

Since block content doesn’t have a settings from guess one will have to be added.

Updated issue summary to include the settingsForm being added.

Started a CR as well

Wasn't sure how we wanted to handle admin/content/block/{block-content} when the standalone_url setting is off. Can't deprecate the route as it is used.

Think it's ready for some review.

Left a review

I've taken a look yesterday night, one minor detail I am unsure about is into which group the "block content settings" link is placed on /admin/config. It not really feels like the perfect fit to put it under "user interface". My first goto in the list of available groups would have been content authoring probably. but that is only due to the fact that the term is closest to the general term "content", defining stand alone urls for content blocks isnt exactly "content authoring" either. while the term "user interface" is simply too generic (i wouldn't even considered or expected a setting like that there). for media settings it is easier since there is a dedicated group media available. tricky. "user interface" just seems not the exact right fit imho.

Addressed most of the feedback but leaving NW for the open threads.

@rkoller I didn't know either where it should go but speaking to @larowlan we picked that one as there didn't seem to be a better spot.

i completely agree. and i've only looked at it from the perspective where to sort the link in the list of available groups. Another option might be to introduce a new one. Yet another option might be to go with the user interface group where the patch currently puts it and wait what comes out of the restructuring the admin ui menu structure and tackle the question/problem in the context of that effort (might be the pragmatic approach).

or in addition i could bring up the topic either informally in the ux channel or raise it during the next ux meeting. just to get a few more perspectives (but i doubt anyone might have a better idea for the destination with the list of current groups).

Well if we add a new Block Content or Block section, similar to Media. Would it be odd if it's the only setting?

May make sense for contrib just can't think of anything on the horizon for core to add another link to that section.

well without the patch applied user interface also has just a single link (Shortcuts) in it.

but just adding a new group should also handled with care imho. would it make sense to add it, i mean are there many different contrib modules related to blocks that would justify that?

Create a new section for "Block" @larowlan some of the stuff requested we remove would break existing behavior.

Feedback addressed

Needs a reroll

Will address it tomorrow!

Resolved all feedback. Still passing.

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

Rebased

Addressed feedback.

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

Rebased

From #39 and #41 it feels like @rkoller isn't sure that a new group is the right approach. I think we should get the UX team to make a suggestion either way. We could bundle in the wording on the form as part of that - I realise this is similar to the Media settings form but in this instance its only for preview so the wording/use case is slightly different. Tagging for UX team (I think that's the correct tag)

Leaving at needs review

i have had added this issue to the shortlist for todays usability meeting, but yesterday a request for the review of the navigation module came in. since they try to get it into core as experimental we've discussed the navigation module today. we hopefully find time next week or i might pick the brains of the other regulars during the week.

my main concern about the new group here was twofold. for one the new group Block is created with only a single page Block Content Settings in it and that single page has only a single interface component, a checkbox Standalone Block Content URL on it. on the other hand i think we should be mindful about the number of categories/groups available as well as what types there are. the majority is currently organized by topic/general function, not many are based on an actual entity type/module? media for one but that contains not only pages for the media module but also anything related to images and files.

Usability review

We discussed this issue at 📌 Drupal Usability Meeting 2024-04-12 Needs work . The recording of the meeting can be found at: https://www.youtube.com/watch?v=1uFZZtA_xDw

For the record, the attendees at the usability meeting were @AaronMcHale, @BlackBamboo, @benjifisher, @rkoller, @simohell, @skaught, and @worldlinemine.

If you want more feedback from the usability team, a good way to reach out is in the #ux channel in Slack.

At first sorry for the delay, but getting our train of thought and the reasoning behind our recommendations into a (hopefully) coherent comprehensible shape, instead of providing just a incoherent list of bullet points with our recommendations, I was sort of struggling over the course of the last few days. :/

As one of the first steps in the meeting, we've compared the behavior of a block page with and without the MR applied:

MR not applied
Block description link: /admin/content/block/1
Operations Edit button: /admin/content/block/1

MR applied - "Standalone Block Content URL" and "Standalone media URL" ticked
Block description link: /admin/content/block/1 (uses Olivero)
Operations Edit button: /admin/content/block/1/edit (uses Claro)

Accessing /admin/content/block/1 as an anonymous user in a private browser window leads to an Access denied page in Olivero.

Media name link: /media/1 (uses Olivero)
Operations Edit button: /media/1/edit (uses Claro)

Accessing /media/1 as an anonymous user in a private browser window displays the page for the first media item in Olivero.

MR applied - "Standalone Block Content URL" and "Standalone media URL" unticked
/admin/content/block/1 redirects to /admin/content/block/1/edit
/media/1 redirects to a Page not found page

One detail that sparked our attention, on one hand you have a consistent patterns used for paths and the themes used for displaying the content in the different contexts with the standalone URLs for blocks and media ticked, on the other hand you have the discrepancy when standalone URLs are both unticked for blocks and media, how none available paths are then handled for each entity type.

From there the discussion shifted into a brief outline of the historical evolution of content entity types. Initially only nodes were field-able, then terms became field-able, in Drupal 8.0 blocks got field-able, and then finally media items. Over time by general demand the different content entity types evolved more and more, added new features and moved towards a feature parity and a sort of uniform set of UI components for the user.

But we have to be clear what the use case for an entity type is and it is important that we don't deviate from it or be ambiguous about it, otherwise things become confusing in particular for people who are learning Drupal for instance. If you create for example a block called image and you put an image field on it, what is the difference to a media item other than its location in the media library from the end users perspective? Same if you have block with four fields and a node with the same set of fields, the difference due to the strive for consistency in UI components and their micro copy is marginal, making it challenging to distinguish and pick one of the two.

In consequence the group came to the insight and conclusion, certain things should be consistent across content entity types, but there should also be fundamental differences between the various entity types. We should be deliberate when we make those choices to have deliberate inconsistencies. Otherwise when a user is looking at the different content entity types and they all look almost identical without any cue that person is lost.

So blocks are not standalone, blocks were never been designed to be viewed independently, they are placed into other entities. The main use case is with Layout Builder, currently there is no way to preview a block before adding it to a layout. Blocks are only viewable by authenticated users with with necessary permissions, that means the URL (for example /admin/content/block/1) is appropriate, that also means blocks should remain under /admin, that means the permissions are appropriate, so in consequence the "Standalone Block Content URL" would be obsolete. Blocks could only be previewed by users that are able to edit blocks while anonymous users or users without the necessary permission get an access denied page. That lead into the following conclusions:

1. There was a clear consensus to recommend removing the Block Content settings page again. Instead make that option (the tab) enabled per default for every block type for users with the necessary permission. The tab should be renamed to Preview, view you associate with viewing something in its final form, by making that deliberate switch in the wording we communicate you are able to preview something, but its final form will look different in a different surrounding context. And in regards of the preview it would be a reasonable step to use the admin theme Claro instead of the default theme Olivero respecting the context of the /admin/content/block/ path - using Olivero per default is waking wrong expectations as well as assumptions.
2. One suggestion that is completely out the scope for this issue, which could be moved to a followup issue is to add an option on a per block type basis on the edit page to set in which theme the blocks of a block type are previewed in. Meaning the user would be able to change the preview from the default Claro to the default theme for example. That way the user would be flexible to assign a block type to the context aka theme it is created for.
3. The last point that was raised was in regards of consistency, it was suggested to add a publishing option for blocks. But while writing up the issue I've remembered and after some searching I've found that there is already a far grown issue for suggestion: ✨ UI for publishing/unpublishing block_content blocks Needs work . So a double thumbs up for the issue and feature from the group.

I've removed the Needs usability review tag and changed the status back to Needs work

I just filed 🐛 Block library view links to the edit page for each content block even when the user does not have edit access Active which may be a duplicate. This issue has wider scope, but would resolve that.

May check back later to incorporate my STR into this issue etc. and properly close it as a duplicate.