Decouple the Layout Builder UI from entities

Crossposting my concerns from #2926914: Rewrite \Drupal\layout_builder\Section to represent the entire section, not just the block info. Configured layout/section data being stored as field content data seems wrong to me. The reason it's stored as field content data in HEAD is presumably that it currently only supports layout overrides for individual content entities, and since those individual content entities are saved as content, the sections are too. This issue is listed as a blocker for #2922033: Use the Layout Builder for EntityViewDisplays, because that issue will require attaching layouts to entity displays which are config entities

Rather than wrapping entities and forwarding stuff on, I think we should take a step back and look at the big picture. I feel that #2924058: Discuss using Layout Builder to control full site layout and replace Block UI a.k.a. unified layour building for block placement within theme regions (currently via the Block UI) is a stable blocker because of what I see to be serious UX issues (which I have notes on and will post about on that issue or the overall roadmap). I think that issue might even be a beta blocker for the same reason #2922033: Use the Layout Builder for EntityViewDisplays is: the site builder will potentially make bad site building decisions that might be difficult to recover from if the easy, user-friendly layout builder is only available for certain pages. (Indeed, a landing page by its nature will often want to override the "page chrome", not just the content layout.)

I also think that the layouts need to be available across the board in the same way and that users will want to use layouts they create in one place for another place. Setting up a layout will be a time consuming activity; if they set up the layout for the wrong thing and then realize that they have to undo and redo their work to get it on a different thing, that would be intensely frustrating. Tim suggested that we provide UI and API to transmute a content-stored layout into a config-stored layout, but then we get into fragmenting copies of layouts and whatnot, where they change the layout in one place and it doesn't change in another place. And how do you "copy" a layout when one layout has blocks placed inside the content, and another has blocks placed outside the content in theme regions or whatever?

I think that layouts need to be stored the same way for all three usecases (probably as their own config entity), not in three separate ways that have to have the API calls forwarded on to them. Tim also mentioned that in discussions back in the spring, japerry also had advocated for the kind of separate storage I'm proposing, but the team made a decision to attach the entire layout to the individual entity (versus storing the layout as its own thing and then just referencing it on the entity). (As Tim also mentioned the work at that point went by the product and UX teams but not the framework and release teams.) :)

I'm not sure how much it disrupts the roadmap to explore this more, or when the right time to revisit it is, but I do think it's beta-blocking and it seems like it would affect what we do in this issue.

Edit: clarity.

+++ b/core/modules/layout_builder/layout_builder.routing.yml
@@ -1,5 +1,5 @@
+  path: '/layout_builder/choose/section/{section_storage_type}/{section_storage}/{delta}'

this is slick

love that by moving from a magically keyed array to a value object this implementation just fell out.

great work @tim.plunkett

What else needs doing here? The patch looks good to me.

I plan on reviewing it tomorrow, and hopefully rtbcing it :-D That's my plan at least.

Eclipse

Ok, this is an enormous move forward and sooooo much better than what we're doing right now. Really paves the way for doing bundle defaults.

I didn't see anything that wasn't an enormous improvement, so. RTBC imo.

Eclipse

That's some weird testbot failure from comment 2. This is still rtbc.

So my points in #4 now move to the next issue in the dependency chain. :) I have not fully reviewed this patch, but I'm comfortable with it being committed pending @larowlan's (or another framework manager's) review and signoff, and would just like an opportunity to sign off on the next one.

Thanks everyone!

1. +++ b/core/modules/layout_builder/src/Routing/LayoutTempstoreParamConverter.php
   @@ -45,14 +45,40 @@ public function __construct(LayoutTempstoreRepositoryInterface $layout_tempstore
   +        $converter = NULL;
   

   return NULL? personal preference for early returns.

2. +++ b/core/modules/layout_builder/src/Routing/SectionStorageOverridesParamConverter.php
   @@ -40,7 +45,7 @@ protected function getEntityIdFromDefaults($value, array $defaults) {
          list(, $entity_id) = explode(':', $value);
   ...
   -    else {
   +    elseif (isset($defaults['entity_type_id']) && !empty($defaults[$defaults['entity_type_id']])) {
   

   if we returned earlier, we'd avoid the elseif here - but that's a personal preference.

neither warrant blocking commit as only personal preferences

Committed as c805443 and pushed to 8.5.x.