Mark Layout Builder as a stable module

Adding a stub RM checklist to the IS.

Adding the "Needs * review" tags.

Greetings! Putting on the ol' product manager hat here! :D

Between all the Layout Builder stable blockers that have been reviewed/committed in the past few days, as well as UX reviews over the past few months, I've actually managed to put this functionality through basically all of its paces:

- Enabling the functionality in the first place
- Creating/editing default vs. overridden layouts
- Testing a wide variety of fields/blocks and ensuring they work properly
- Using keyboard navigation to modify layouts
- Testing integration with other core features such as Quick Edit and translations
- Testing simultaneous editing edge cases
- Testing the new, more granular permissions

All of the issues that I bumped into were either covered by "must have" issues that were recently committed, such as #2988970: Layout Builder should make it easier to modify the default layout for an entity type when viewing an entity, or for which there are already existing "strong should have" issues in the queue, like #3028979: Blocks with fixed width elements can break multi-column Layout Builder layouts .

One thing that I guess is apparent is that the UX/design review of this has been done for the most part on a patch-by-patch basis, and it shows in some places (for example, the wide variety of border styles, pointed out by @lauriii). Overall though, I don't find those disruptive enough to hold this extremely important functionality back from the general public. I'm sure we'll continue to further refine the UI of this in 8.8 and beyond.

Therefore, removing this tag, and providing official sign-off. GREAT WORK to the team, as well as the UX and a11y teams for all of their important contributions. I am SO excited to see this feature land!!! :D

Removing the "Needs product manager review" tag per @webchick's comment above.

One thing we evaluate when we're getting ready to commit a module is the plan for post-stable work on the module. This can reflect team prioriries, essential missing features that were descoped from the immediate 8.7.0 release blocks (e.g. translation support), or important outstanding technical debt, especially aroud major issues, a11y and usability issues, et.

So this issue is a good opportunit to begin identifying the longer term roadmap, and what the next most-important things are between now and 8.7.0. We might want to outline these things on a roadmap.

+++ b/core/MAINTAINERS.txt
@@ -257,6 +257,11 @@ Language
+Layout Builder
+- Ted Bowman 'tedbow' https://www.drupal.org/u/tedbow

I am willing and able to be a Layout Builder module maintainer. I read Subsystem Maintainers responsibilities.

I am also willing and able to be a Layout Builder module maintainer. I've read the Subsystem Maintainers responsibilities.

The extension points that are being made public make sense.

+++ b/core/modules/layout_builder/src/Entity/LayoutBuilderEntityViewDisplayStorage.php
@@ -10,9 +10,6 @@
  * @internal
- *   Layout Builder is currently experimental and should only be leveraged by
- *   experimental modules and development releases of contributed modules.
- *   See https://www.drupal.org/core/experimental for more information.

+++ b/core/modules/layout_builder/src/Form/LayoutBuilderEntityViewDisplayForm.php
@@ -14,9 +14,6 @@
  * @internal
- *   Layout Builder is currently experimental and should only be leveraged by
- *   experimental modules and development releases of contributed modules.
- *   See https://www.drupal.org/core/experimental for more information.

These look like they still need explanation of why they're internal.

I was asked by @effulgentsia to look at this for a FM review.

I looked over the issue queue for layout_builder as part of the Maintenance and technical debt item in the issue summary. There are currently around 120 open issues. I reviewed them and found many of the ones that sounded like major issues already had a patch and many had been written by @tim.plunkett, which is a good indicator of their quality/likelihood of being compatible with the module architecture and direction. Some of them required API changes, e.g. #3035174: Rename SectionStorageTrait to SectionListTrait but these were done using our standard BC/deprecation approach - so no cause for concern there.

There are a large number of issues that pertain to accessibility, which is a core gate - but none of these were flagged as critical. In addition, these would not impact Drupal's WCAG compliance, as they are primarily related to site-building and content-authoring, so that comes under ATAG. #3007978: Accessibility Plan for Layout Builder tracks these.

Two issues stood out as problematic, particularly the second one:

• #2995893: Layout builder chokes on form exceptions that are part of natural form processing - the patch here is small and I think could be included - and would improve layout builder compatibility with blocks that provide forms that use FormState::setResponse in their submit handler, which I think comes under the integrations item in the issue summary
• #3030154: layout_builder__layout_section column hitting database limit - this feels like a data loss issue. The upgrade path here is difficult so we need to go into this with eyes open that this is a known issue.

I then reviewed for performance sake and found #3043330: Reduce the number of field blocks created for entities (possibly to zero). Because the FieldBlock plugin is part of Layout Builder and these blocks aren't visible to the regular block UI (_block_ui_hidden is TRUE) I think there is a performance win we can get there and this will improve #3043087: Retrieving plugins with entity context definitions from cache is expensive, which is noticeable when used with Layout Builder's FieldBlockDeriver. This will however, result in plugins that were previously present going away. Now, these plugins are declared by layout builder and designed for use with layout builder, however once they're in the wild and stable, then it may be more difficult to remove these. So I think that #3043330: Reduce the number of field blocks created for entities (possibly to zero) warrants serious consideration for being a stable blocker. The patch is small but the performance improvement is large - and if we do it now, we don't have to worry about taking away those plugins later. It would be down to a release manager as to whether removing plugins from core is permitted in a minor.

I also reviewed the internal classes, as @xjm did as part of the patch. I found that InlineBlockUsage is not made part of the api in this patch (currently its flagged as internal). Is there a need for that to become public and if so - what aspects of it?

Finally, under the maintenance item of the issue summary, I note that @tim.plunkett and @tedbow have been prolific in their output for the module, and I thank Acquia for their continued sponsorship. I would like to ask if there is a plan for that sponsorship to continue once the module becomes stable. The size of the issue queue for the module (~120 or so issues) indicates there is still much work to do and providing that sponsorship post stable module will help to clear out that backlog. Note I'm not saying 'Acquia should continue to sponsor Tim and Ted' (although obviously that would be great), I'm asking if the plan can be articulated for the sake of transparency. I think this is something we should do for all major core features that are sponsored by an Organisation, as even after the feature is delivered, there is ongoing work we need to plan for.

Crediting @Sam152 who I discussed my comments and process with.

@larowlan thanks for the review.

I'm asking if the plan can be articulated for the sake of transparency

I can speak to this a bit as an engineering manager at Acquia. We're definitely committed to finishing features (e.g. translation support) and responding to bugs as they affect the usability or stability of the module - especially in the context of Acquia's customers and partners, but not limited to. In addition, a certain amount of our DAT engineering time is allocated to subsystem maintenance for those that are maintainers.

It's tough to give much more detail than that because priorities do shift. Right now, we're betting on Layout Builder (as evidenced by @tedbow and @tim.plunkett's "prolific" output :)

#3030154: layout_builder__layout_section column hitting database limit is not reproducible - that is great news
#3043330: Reduce the number of field blocks created for entities (possibly to zero) sounds like its not as straight forward - I will comment over there
#2995893: Layout builder chokes on form exceptions that are part of natural form processing is RTBC 🎉

I can speak to this a bit as an engineering manager at Acquia. We're definitely committed to finishing features (e.g. translation support) and responding to bugs as they affect the usability or stability of the module - especially in the context of Acquia's customers and partners, but not limited to. In addition, a certain amount of our DAT engineering time is allocated to subsystem maintenance for those that are maintainers.

It's tough to give much more detail than that because priorities do shift. Right now, we're betting on Layout Builder (as evidenced by @tedbow and @tim.plunkett's "prolific" output :)

Thanks Adam, that's what I was after

I don't recall being asked a question like this before when volunteering as a subsystem maintainer for the other eight subsystems I work on.

Just to be clear, my question was not directed at you (or Ted for that matter). It was directed at the sponsoring organisation. I would ask the same question of any other feature, e.g. if we were talking about workspaces module, I'd ask that Pfizer articulate what their ongoing commitment would look like. I think that is something we (the community) should be doing as part of long-term sustainability/diligence. But thank you for articulating your commitment.

On the basis of @balsama's reply and the progress/explorations that have occurred overnight (in my part of the world) for the other issues, I'm comfortable removing the 'Needs Framework Manager review' tag.

Thank you Tim, Ted and Acquia for bringing this feature to Drupal.

Okay, we're down to just #3043646: For sites that have made layout overrides prior to 8.7.0 or sites that manually enable translation of the layout override field, add UI warnings!

Just a couple small things.

1. the X and + icons are missing.
   probably just a relative path problem when the css was moved.

2. +++ b/core/modules/layout_builder/src/Access/LayoutBuilderAccessCheck.php
   @@ -15,6 +15,7 @@
     * @internal
   + *   Tagged services are internal.
   

   I love that we are explicitly saying why things are internal!

3. +++ b/core/modules/layout_builder/src/QuickEditIntegration.php
   @@ -22,6 +22,7 @@
     * @internal
   + *   This is an internal utility class wrapping hook implementations.
     */
    class QuickEditIntegration implements ContainerInjectionInterface {
   
   +++ b/core/modules/layout_builder/src/Routing/LayoutTempstoreParamConverter.php
   @@ -11,6 +11,7 @@
     * @internal
   

   There a couple @internal's in this class remaining. But since the class itself is internal(yay!) they can be removed

4. \Drupal\layout_builder\Section and \Drupal\layout_builder\SectionComponent have todo point to closed issues.

   side note, won't be awesome if was CLI tool to scan code for todo's to closed d.o issues!

5. +++ b/core/MAINTAINERS.txt
   @@ -257,6 +257,11 @@ Language
   +Layout Builder
   +- Ted Bowman 'tedbow' https://www.drupal.org/u/tedbow
   +- Emilie Nouveau 'DyanneNova' https://www.drupal.org/u/dyannenova
   +- Tim Plunkett 'tim.plunkett' https://www.drupal.org/u/tim.plunkett
   

   I like the order here 😉
   oh.. its alphabetical by last name like all the others. Still

6. +++ b/core/modules/layout_builder/layout_builder.info.yml
   @@ -1,7 +1,7 @@
   -package: Core (Experimental)
   +package: Core
   

   🎉

Ok. Everything looks good. manually checked the icons and they show now

🥁🗞........

RTBC

@larowlan already covered pretty much everything I would have here.

Also, thanks @tim.plunkett for adding credits for the team here already.

However, anticlimactically:

core/themes/stable/css/layout_builder/layout-builder.css
  91:25  ✖  Expected double quotes  string-quotes                              
  94:25  ✖  Expected double quotes  string-quotes                              
 196:36  ✖  Unexpected "  "         selector-descendant-combinator-no-non-space
 200:36  ✖  Unexpected "  "         selector-descendant-combinator-no-non-space

@bnjmmn will post a patch fixing the linting errors.

Here is the lint-fixing anticlimactic patch 😊

Double checked the intediff against #37 and checked the changes.

🍗🤣.......

RTBC!

Okay, yay! :) Somewhat belatedly committed to 8.8.x and cherry-picked to 8.7.x.

Thanks everyone!

Congratulations, everyone! I’m proud to have been part of the team that made this happen. :)))

Huge milestone, thank you all!

Thanks to the many, many people who helped make Layout Builder a reality!!!!!

What an awesome milestone, congratulations to everyone who made this happen.

I'm part of a team who made the bet to go "all in" on layout builder on a large scale D8 site some 5 months ago. I can say that the reception from our client has been overwhelmingly positive and the capabilities it afforded us pushed the content authoring experience to a really high standard.

Thanks again to everyone and congratulations :)

🎉 This is fantastic, really great milestone, everyone who has contributed to Layput Builder should be very proud of what’s been achieved here. Special thanks and congratulations to @tim.plunkett and @tedbow as without their hard work I don’t think Layout Builder would be what it is today!

Like @Sam152, I took a gamble on using Layout Builder for an important project about a year ago and I’m pleased to say it has definitely paid off.

Sorry, I was somewhat exhausted yesterday after doing my final review and all the intense work on the last blockers these last two weeks. This is the thing I'm proudest of in 8.7 (and probably in any minor to date). Tagging for the highlights, of course.

Huge congrats! :)

From comments like #47 and #48, to articles like https://www.lullabot.com/articles/announcing-new-lullabotcom a month ago … I don't think there's ever been a more widely anticipated feature in Drupal 8 minor release yet! 🥳 👏

@everyone that contributed the last 2 years. You made something well-thought, easy to use and flexible! It´s really something to be proud of!! Congratulations!