Add support for global regions

This MR builds on top of #3489302: Preview entire page not just content area, so let's land that ASAP to make this MR (much) easier to review.

I did review the ADR already though!

Quoting the IS:

Let the frontend consider them as a single tree and set of components, with additions to identify regions.

🤔

Is this intended to be temporary or permanent?

Most Content Creators will not be able to modify the "global regions" (so: component trees in the theme's PageTemplate config entity), typically only Site Builders will be allowed to do that.

Or is that considered an afterthought for now, because we do not have the concept of "locking" a component (sub)tree yet, which we'll also need for #3455629: [PP-1] [META] 7. Content Templates — aka "default layouts" — affects the tree+props data model?

EDIT: I see I wrote a related comment on the MR 2 days ago: https://git.drupalcode.org/project/experience_builder/-/merge_requests/4...

#7: WFM! 👍

#8:

Hah, I just finished a whole write-up about that over at #3489302-28: Preview entire page not just content area, because @lauriii pointed out the preview was still imperfect. The reason for that is closely connected to what you just described here.

Details:

The "content" region should therefore contain a "block.system_main_block" component.

Let's start with that, but eventually, the main content block should be placeable in any region of the theme. But that depends on the UX for editing just the PageTemplate (independent of any concrete route) being defined and designed. For now your proposal is pragmatic and fine 👍

There is still a problem in that we need to limit the front end here further than we already have; the user needs to be prevented from moving components out of the "main content block" slot and into another part of the "content" region.

Right!

That is exactly where we need the designs to provide clear affordances that make the user aware what they're editing: I think that by default the global regions should NOT be editable while editing e.g. Page or Node 7. Otherwise the Content Creator might unwittingly be modifying the PageTemplate and hence affect all routes/HTML responses on the site, not just the one for Page or Node 7.

I also touched upon that need in #3489302-28: Preview entire page not just content area.

That last blockquote in #9 is actually why I'm concerned about this bit in the IS:

As well as being able to edit a node layout which lives inside the "main content" block, we also need to be able to place components in other theme regions in Experience Builder.

… because that literally blurs the lines between "the component tree in the sole content entity being edited" and "the component trees in the theme's PageTemplate that literally affect the entire site".

Let the frontend consider them as a single tree and set of components, with additions to identify regions.

And this is AFAICT wrong? @lauriii himself said the same thing over at #3489302-26: Preview entire page not just content area, because the pieces he identified as missing in those screenshots aren't rendered by an XB component tree.

The global regions should not be editable by default when editing a page or node. The interaction that design defined for this is that users need to double click a global region, which opens up the region in a "focus mode" where user is now allowed to make changes to the global region. How this looks visually has been defined here: https://www.figma.com/design/Ps3m4APGHIILsBrm0Uj31N/Experience-Builder?n....

It is possible to place other blocks in the same region above or below the "main content" block.

I'm not convinced we would need to support this. This is easy enough to workaround by providing a "Above content" and "Below content" regions so I'm not convinced that we should convolute the UX to add support for this use case.

Let's start with that, but eventually, the main content block should be placeable in any region of the theme.

If we need this, this seems like a setting for the theme. This doesn't seem something that would have to be editable in XB. I'm not aware of common use cases that would require moving the main content block to another region after the initial theme setup.

This doesn't seem something that would have to be editable in XB

Oh wow, you're saying that in the XB-powered world, a theme would actually define in its metadata which region should contain the main content block (Drupal\Core\Block\MainContentBlockPluginInterface), and it should then contain nothing else? If so, we'll need to adjust the validation constraints for the PageTemplate config entity.

If so: do we want the same treatment for the page title (Drupal\Core\Block\TitleBlockPluginInterface) and messages (Drupal\Core\Block\MessagesBlockPluginInterface)?

That's crucial information not present in the that potentially would've made #3478537: Introduce an XB `PageTemplate` config entity simpler… 😅

If so: do we want the same treatment for the page title (Drupal\Core\Block\TitleBlockPluginInterface) and messages (Drupal\Core\Block\MessagesBlockPluginInterface)?

99% of the time sites would display these foundational blocks in a region and there wouldn't be any complexity involved with it. However, there are some use cases where context is used so that the block could be displayed in a different region depending on the page.

The approach that I think would work here is to allow theme to decide a placement of a block. Once theme has decided on it's placement, it would not be available to be used in XB. This would particularly well for Drupal\Core\Block\MessagesBlockPluginInterface and Drupal\Core\Block\MainContentBlockPluginInterface. Maybe it doesn't even have to be generic since it's quite specific to these two blocks.

The outlier here is Drupal\Core\Block\TitleBlockPluginInterface because in most scenarios, it would not make sense to display the title block for Pages and Nodes because the title would be defined either in content or the content type template. However, other controllers may need the title to be displayed. If we implemented the generic way for themes to place blocks, we could potentially fit the title use case there too without introducing conditionality for the placement by allowing the template to not render the variable when the title is not desired. Otherwise I guess for now we'd just allow users to place the title in case that it's something they'd need.

This is all really fascinating and brings me flashbacks from #2476947: Convert "title" page element into a block.

Following a meeting between @wim leers, @longwave, @f.mazeikis and I, I created the following notes:

1. At the top level, we will have an array of regions - always at least a `content` region which will replace ‘root’ concept.
2. We should default the tree to having a ‘content’ region on everything. It’s special cased and will error if it doesn’t exist.
3. Permissions handled back end - regions a user doesn’t have access to won’t be sent (but the HTML in the preview will still be present).
4. Suggestion: Always send full tree from client to server to enable editing more than one region in a single ‘round trip’
5. Validation: backend can return unique identifier - front end can use that to highlight the errored “thing” where ever it might be
6. Later: discuss with @Lauriii the repercussions/design behind what things do/don’t go into the temp store. I think the consensus was that pretty much everything should always save to temp and be published in one action at a later point to avoid things like a pattern existing but it contains components that were never published.
   

@longwave in #14

Aha, so the content region becomes (very) special-cased! That can work. Note that the PageTemplate config entity's validation constraints require those 3 special blocks to be present, so rather than doing that fallback logic in PageTemplateDisplayVariant::build() (similar to the code you quoted from BlockPageVariant::build()), we can and IMHO should do it in PageTemplate::create(). (There's precedent for that: see implementations of EntityInterface::create()).

(Also, I forgot about the content region being assumed by Drupal core to exist, even though themes could not have such a region. It's reasonable, pragmatic, and not in anyway a BC break to assume that region, so: yay! 😄)

@lauriii in #15

• RE: main content + messages blocks. Sounds great to me! @longwave proposed both the main content & messages blocks in the content region.
• RE: title block.
  

  it would not make sense to display the title block for Pages and Nodes because the title would be defined either in content or the content type template.

  For accessibility and consistency reasons (and others), it's important that the title (of a controller with some logic or of a content entity) is always rendered in the same place, using the same markup.
  That's why I doubt the content entity's title can realistically become part of the content entity (template)'s component tree? 🤔

  Look at /admin/structure/types/manage/article/display in the Standard install profile: the article node's title field does NOT appear there! It also does not appear in core.entity_view_display.node.article.default.yml at all. This is due to:

        ->setDisplayOptions('view', [
          'label' => 'hidden',
          'type' => 'string',
          'weight' => -5,
        ])
  

  not also doing

  ->setDisplayConfigurable('view', TRUE)
  

  (see the code in \Drupal\field_ui\Form\EntityDisplayFormBase::getFieldDefinitions(): it filters away any field definition that is not "display-configurable").

@jessebaker in #16

Thank you so much for summarizing our conversation! 🤩👏

RE: 1+2 → yep, that's what @longwave explained in #14 and I now see can work.

RE: 4 → this implies that a single request from the client might modify both the content entity currently visible in the XB UI and the active theme's PageTemplate config entity simultaneously! We discussed that in some detail, and @longwave believes this this is viable, although he agrees this is entirely new ground in all of Drupal (saving a content + config entity together in a single transaction), especially given that we'll have to save them into tempstore/a workspace and then publish it all together.
⚠️ This does imply that the wse_config contrib module will become a hard dependency for XB!
💁‍♂️ Alternatively we could've gone with the config overrides PoC I did back in April, but that was just for verifying we wouldn't need Workspaces to achieve the intended UX. I think since then, the design has evolved sufficiently, as well as technical discussions, to have concluded that XB will rely on Workspaces. I think it'd be good for @lauriii to formally document/state that though :)

RE: 5 → @tedbow and I got the basics of that "transform validation errors' server-side property paths to client-side request body equivalents" logic working in #3478565: Add Entity Update controller. So I'm convinced it's possible albeit not trivial. But both @longwave and @jessebaker believe it's important to keep the client A) as simple as possible, B) to prevent out-of-sync-sequential-updates to the preview: by being able to send everything at once from the client to the server (this issue: PageTemplate config entity + content entity, future: PageTemplate config entity + content type template config entity aka #3455629: [PP-1] [META] 7. Content Templates — aka "default layouts" — affects the tree+props data model + content entity), we minimize both round trips/latency and ensure a single update to the preview that immediately reflects changes across all those entities.

👆 Point 5 above (the bolded bit at the end) is IMHO the strongest reason to go down this path of keeping the client blissfully unaware of any server-side representations, and make the server side much more complex by forcing it to map a single request body onto many entities to update. 👍

#3489302: Preview entire page not just content area is in, updating title.

Issue summary fully updated now.

Made @larowlan aware of this direction over at #3467954-31: META: Evolve XB UI's data model to allow non-SDC components' inputs, DynamicPropSource support, etc., to ensure he doesn't go in a conflicting direction.

For accessibility and consistency reasons (and others), it's important that the title (of a controller with some logic or of a content entity) is always rendered in the same place, using the same markup.
That's why I doubt the content entity's title can realistically become part of the content entity (template)'s component tree? 🤔

I don't think mandating this as a requirement matches well with real life requirements that sites have from design perspective. I agree that title has some nuance to it which makes it different from messages and the main.

Here are few example where it's easy to see there are many sites where there isn't a lot of consistency on how the title is displayed between pages:

1. https://www.doritos.com/
2. https://www.jaffa.fi/
3. https://www.acquia.com/

discuss with @Lauriii the repercussions/design behind what things do/don’t go into the temp store. I think the consensus was that pretty much everything should always save to temp and be published in one action at a later point to avoid things like a pattern existing but it contains components that were never published.

Anything that would impact the live site should go through the temp store. I was originally thinking that maybe adding new sections wouldn't go through the temp store but there could be some interesting edge cases with that because someone might be creating a section using a component that only exists in the temp store. 🤔

#3491013: Rework layout API to separate components and slots is in!

Let's rebase this :)

Another related issue. Going for a rebase here as #3494114: Implement auto-save of the page template config entity will need the changes here

Pushed some work to start building the regions based on the page template. It's failing openapi spec validation at the moment

Will continue tomorrow.

1. #3492669: Use PageTemplate status flag to enable per-theme page templates is in.
2. As is #3494331: Create a value object for the return of ComponentTreeHydrated::getValue, which @larowlan alluded to in his review.
3. 4 E2E tests are still failing.
4. I pushed one proposed commit (feel free to revert altogether) to outline how I think we should address @larowlan's review at https://git.drupalcode.org/project/experience_builder/-/merge_requests/4...

=

Crediting Nathan who pointed me at cli viewport size as a possible cause of the failing e2e test when only in headless mode

Crosspost

@longwave in #33: to have it available in the annotation-based plugin definition' but also use that information from the plugin definition in the code. This is nothing new; I'm just matching the existing pattern that you and @f.mazeikis introduced (see BlockComponent::SOURCE_PLUGIN_ID) in #3475584: Add support for Blocks as Components? 😅

🥳

See y'all in:

1. #3467954: META: Evolve XB UI's data model to allow non-SDC components' inputs, DynamicPropSource support, etc.
2. #3494114: Implement auto-save of the page template config entity
3. #3494684: Add ::clientModelToInput() to ComponentSourceInterface, the inverse of ::inputToClientModel()
4. #3494687: Add contextual menu frontend support for moving components into regions
5. And last but not least: @longwave is going to file a follow-up for https://git.drupalcode.org/project/experience_builder/-/merge_requests/4... — we hopped on a Zoom to clarify that :)

(@longwave please link that issue and mark this as Fixed when you've done that!)

Post-humously bumped to Critical considering how many issues this unblocked! 😅