Third party should be allowed to alter the section render array

Changing status to Needs Review & Triggering bots.

1. +++ b/core/modules/layout_builder/src/Section.php
   @@ -81,14 +83,21 @@ public function __construct($layout_id, array $layout_settings = [], array $comp
   +    $event = new SectionBuildRegionsArrayEvent($this->getComponents(), $this, $contexts, $in_preview);
   +    $event_dispatcher->dispatch(LayoutBuilderEvents::SECTION_BUILD_REGIONS_ARRAY, $event);
   

   I'm not sure I understand the purpose of this event. Maybe it'd be clearer with a test implementation.

2. +++ b/core/modules/layout_builder/src/EventSubscriber/SectionRegionsArray.php
   @@ -0,0 +1,44 @@
   +  public function onBuildRegions(SectionBuildRegionsArrayEvent $event) {
   +    $regions = $event->getRegions();
   +
   +    foreach ($event->getComponents() as $component) {
   +      if ($output = $component->toRenderArray($event->getContexts(), $event->inPreview())) {
   +        $regions[$component->getRegion()][$component->getUuid()] = $output;
   +      }
   +    }
   +
   
   +++ b/core/modules/layout_builder/src/Section.php
   @@ -81,14 +83,21 @@ public function __construct($layout_id, array $layout_settings = [], array $comp
   +    $build = $this->getLayout()->build($event->getRegions());
   ...
   +    $event = new SectionBuildRenderArrayEvent($build, $this, $contexts, $in_preview);
   +    $event_dispatcher->dispatch(LayoutBuilderEvents::SECTION_BUILD_RENDER_ARRAY, $event);
   

   Oh, this is backwards from what I expected. This code lives in the _region_ build event?

Why have two events at this point? Moving the core implementation to a subscriber itself allows someone to participate both before and after core. Couldn't a single event handle this?

If the second event also came with a setRegions() method, your example code set an earlier priority than core and manipulate the event payload there.

That might be attempting to streamline it too much, idk

I think I may be misunderstanding how this works or it is not working properly.

I've created an event subscriber called Section forms based on the example provided here: https://git.drupalcode.org/sandbox/dsnopek-3059795/-/blob/8.x-2.x/src/Ev...

My end goal is to take a value that is set on each section when it's been added and feed that value into a form so that each section has a form element button that I can use to perform `some` action.

My understanding based on this and the example is that I need a onBuildRegions event that I can then grab the section configuration then generate the form and output it.

Here is the simplified eventsubscriber I am working with, the log nor the exit ever get hit when visiting the page with the sections on it.

<?php

namespace Drupal\nicxvan_test_module\EventSubscriber;

use Drupal\Core\Logger\LoggerChannelFactoryInterface;
use Drupal\layout_builder\Event\SectionBuildRegionsArrayEvent;
use Drupal\layout_builder\LayoutBuilderEvents;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

/**
 * Adds Layout Builder section styles.
 */
class SectionForms implements EventSubscriberInterface {

  /**
   * @var \Drupal\Core\Logger\LoggerChannelFactoryInterface
   */
  protected $loggerFactory;

  /**
   * Constructs SectionStyles.
   *
   * @param \Drupal\Core\Logger\LoggerChannelFactoryInterface $logger_factory
   */
  public function __construct(LoggerChannelFactoryInterface $logger_factory) {
    $this->loggerFactory = $logger_factory;
  }

  /**
   * {@inheritdoc}
   */
  public static function getSubscribedEvents() {
    $events = [];

    // Skip this if the Layout Builder is not installed.
    if (class_exists('\Drupal\layout_builder\LayoutBuilderEvents')) {
      $events[LayoutBuilderEvents::SECTION_BUILD_REGIONS_ARRAY] = ['onBuildRegions', 1000];
    }

    return $events;
  }

  /**
   * Add styles to section regions.
   *
   * @param \Drupal\layout_builder\Event\SectionComponentBuildRenderArrayEvent $event
   *   The section component build render array event.
   */
  public function onBuildRegions(SectionBuildRegionsArrayEvent $event) {
    // Prevent layout_builders normal region building.

    $this->loggerFactory->get('nicxvan_test_module')->error('something wrong');

    $regions = $event->getRegions();
    $section = $event->getSection();
    $layout = $section->getLayout();

    kint($section);
    exit();

    $components = [];
    foreach ($event->getComponents() as $component) {
      $components[$component->getRegion()][$component->getUuid()] = $component;
    }

    foreach ($layout->getPluginDefinition()->getRegionNames() as $region) {
      if (empty($components[$region])) {
        continue;
      }

      // Or if no region style is configured, simply render each component.
      foreach ($components[$region] as $key => $component) {
        $regions[$region][$key] = $component->toRenderArray($event->getContexts(), $event->inPreview());
      }
    }

    $event->setRegions($regions);
  }

}

Same problem here. I try to implement a pluggable system for region and section styles. But this is not possible at this time.

I've closed #3101655: Layout section and component information accessible during render process as duplicate of this. This is older

Closed #3210230: Block placements by 3rd-party in layout section as duplicate.

@tim.plunkett,

I'm not sure I understand the purpose of this event. Maybe it'd be clearer with a test implementation.

I see the value. I described in deep the need, in the #3210230: Block placements by 3rd-party in layout section description. Will update also here the IS to clarify.

This certainly needs a change notice.

1. +++ b/core/modules/layout_builder/layout_builder.services.yml
   @@ -40,6 +40,10 @@ services:
   +    class: Drupal\layout_builder\EventSubscriber\SectionRegionsArray
   
   +++ b/core/modules/layout_builder/src/EventSubscriber/SectionRegionsArray.php
   @@ -0,0 +1,44 @@
   +class SectionRegionsArray implements EventSubscriberInterface {
   

   Let's rename it to SectionRenderArraySubscriber

2. +++ b/core/modules/layout_builder/src/Event/SectionBuildRegionsArrayEvent.php
   @@ -0,0 +1,128 @@
   + * @see \Drupal\layout_builder\LayoutBuilderEvents::SECTION_BUILD_REGIONS_ARRAY
   
   +++ b/core/modules/layout_builder/src/LayoutBuilderEvents.php
   @@ -26,4 +26,37 @@
   +  /**
   +   * Name of the event fired when a section's render array is built.
   +   *
   +   * This event allows modules to collaborate on creating the render array of
   +   * the Section object. The event listener method receives a
   +   * \Drupal\layout_builder\Event\SectionBuildRenderArrayEvent instance.
   +   *
   +   * @Event
   +   *
   +   * @see \Drupal\layout_builder\Event\SectionBuildRenderArrayEvent
   +   * @see \Drupal\layout_builder\Section::toRenderArray()
   +   *
   +   * @var string
   +   */
   +  const SECTION_BUILD_RENDER_ARRAY = 'section.build.render_array';
   

   Symfony event dispatcher component is dropping this constant and uses the event class full qualified name as event ID. Let's follow that path. Event constants may create issues, see also #2825358: Event class constants for event names can cause fatal errors when a module is installed

3. +++ b/core/modules/layout_builder/src/Event/SectionBuildRegionsArrayEvent.php
   @@ -0,0 +1,128 @@
   +class SectionBuildRegionsArrayEvent extends Event {
   
   +++ b/core/modules/layout_builder/src/LayoutBuilderEvents.php
   @@ -26,4 +26,37 @@
   +  /**
   +   * Name of the event fired when a section's regions array is built.
   +   *
   +   * This event allows modules to collaborate on creating the regions array of
   +   * the Section object that will be passed to the layout. The event listener
   +   * method receives a \Drupal\layout_builder\Event\SectionBuildRenderArrayEvent
   +   * instance.
   +   *
   +   * @Event
   +   *
   +   * @see \Drupal\layout_builder\Event\SectionBuildRegionsArrayEvent
   +   * @see \Drupal\layout_builder\Section::toRenderArray()
   +   *
   +   * @var string
   +   */
   +  const SECTION_BUILD_REGIONS_ARRAY = 'section.build.regions_array';
   
   +++ b/core/modules/layout_builder/src/Section.php
   @@ -81,14 +83,21 @@ public function __construct($layout_id, array $layout_settings = [], array $comp
   +    // Build the regions array.
   +    $event = new SectionBuildRegionsArrayEvent($this->getComponents(), $this, $contexts, $in_preview);
   +    $event_dispatcher->dispatch(LayoutBuilderEvents::SECTION_BUILD_REGIONS_ARRAY, $event);
   

   We don't need this event at all. Let's follow the component similar approach. We only need to dispatch one event and provide a subscriber that builds the region array with a priority of 100. Then contrib is able to hook in before or after.

4. +++ b/core/modules/layout_builder/src/Event/SectionBuildRenderArrayEvent.php
   @@ -0,0 +1,118 @@
   +  public function __construct(array $build, Section $section, array $contexts, $in_preview = FALSE) {
   

   Let's type $in_review to bool.

5. +++ b/core/modules/layout_builder/src/Event/SectionBuildRenderArrayEvent.php
   @@ -0,0 +1,118 @@
   +  public function getSection() {
   ...
   +  public function getContexts() {
   ...
   +  public function getBuild() {
   ...
   +  public function setBuild(array $build) {
   
   +++ b/core/modules/layout_builder/src/EventSubscriber/SectionRegionsArray.php
   @@ -0,0 +1,44 @@
   +  public static function getSubscribedEvents() {
   ...
   +  public function onBuildRegions(SectionBuildRegionsArrayEvent $event) {
   
   +++ b/core/modules/layout_builder/src/Section.php
   @@ -315,6 +324,16 @@ public function insertComponent($delta, SectionComponent $new_component) {
   +  protected function eventDispatcher() {
   

   As this is new code we can strict type the return.

6. +++ b/core/modules/layout_builder/src/Event/SectionBuildRenderArrayEvent.php
   @@ -0,0 +1,118 @@
   +  public function inPreview() {
   

   Maybe isInPreview()? And let's strict type the return to bool.

7. +++ b/core/modules/layout_builder/src/Event/SectionBuildRenderArrayEvent.php
   @@ -0,0 +1,118 @@
   +    $this->build = $build;
   +  }
   

   Let's allow chaining, return $this.

Improved IS.

Assigning to fix the issues.

Fixing #8.2 & #20.

Still needs:

• Tests
• Change record

Fixing PHP CS.

Still needs:

• Tests
• Change record

It was a wrong patch.

Fixing PHPUnit tests. I think it's OK that the new subscribers to be able to provide cache metadata.

Development has been moved in drupal-3062862 fork, merge request !588.

Fixed the PHP CS failure from #27.

Still needs:

• Tests
• Change record

Added tests.

Still needs:

• Change record

Added CR: https://www.drupal.org/node/3210520. Ready for review.

This looks more like a feature request.

I think that 3rd-party should also be aware about the section's delta. But how to pass that? Section::toRenderArray() doesn't own that info.

Somewhere along the way we lost one of the subscribers that allowed altering the final render array. Now it is only possible to alter the section regions and not the section itself. For example, if I want to add an attribute to the section like a new css class there is no longer a way to do this. The current event is dispatched before core/lib/Drupal/Core/Layout/LayoutDefault::build(), and that method strips out all data except previously declared regions. So trying to add an $regions['#attributes']['class'][] = 'my-custom-class' in the current event subscriber is useless since it is stripped out.

In speaking with @segovia94 we may have an edge case where we need the event that was removed. We're working on trying to apply styles to sections. In some cases the styles are applied; however, in others they are not applied correctly. Specifically where we have a sections within a section. Styles are applied correctly in layout builder but do not display correctly when the page is rendered in view mode.
Layout Builder view

Page view
Missing default classes from block style plugins and the gold class even though it is not visually shown in the screenshots.

@pbabin I think I may have confused you. The problem you ran into is not related and is due to a workaround in the specific module created since this second event was removed from this issue. But yes, having the second event would fix the problem.

I can add back in the second event that was triggered after the build(). The other option is to edit the Drupal\Core\Layout\LayoutDefault::build() to add into the render array any properties passed into it. That's the biggest problem causing the existing event not to be as useful since it currently can only affect individual regions and not the overall section properties.

Also, I don't see a way to change the merge request to be against 9.4.x.

@claudiu.cristea can you rebase the MR on 9.4 please? Or at least change the target of the MR? As you created it, I think you have to change it.

@heddn, done

Fixed for drupal 9.4.x

Fixed for drupal 9.4.x

Fixing failed test of patch #50.

Fixed hunk for LayoutBuilderEntityViewDisplay.php to drupal 9.4.x

Added the `SectionBuildRenderArrayEvent` event that will allow us to alter the section's build render array while we still have access to the section's third party settings. e.g use case, add configurable CSS classes to the section's container div.

Fixed Drupal CS issues of patch #55.

Re-roll for 10.1 and a 9.5 patch for applying on _those_ sites still.

Make drupalci happy.

+ @trigger_error('Calling LayoutBuilderContextTrait::getPopulatedContexts() without the $delta argument is deprecated in drupal:9.3.0 and the $delta argument will be required in drupal:10.0.0. See https://www.drupal.org/node/3210520', E_USER_DEPRECATED);
1. This will have to be updated for 10.1 and 11

Not needed but would be useful to have a tests-only.patch

I've rebased and pushed to issue/3062862-layout-3rd. Still need to reply to #61.

It looks like it doesn't work without specifying event name for SectionBuildRenderArrayEvent (at least in Drupal 10.2), such as

  const SECTION_BUILD_RENDER_ARRAY = 'section.build.render_array';

I will update the MR shortly based on this.

#64: Event names are not required. You can use the event's class name when subscribing.
e.g

$events[SectionBuildRenderArrayEvent::class] = ['onSectionBuildRender'];

...

rebased onto the main branch

Ok, all green and #60 addressed. Also hidden all previous branches so there is no confusion. Back to review.

Since Lucas just reviewed and I addressed his feedback (deprecation msg update), moving this to RTBC.

Some comments on the MR.

Thank you, all addressed. We have 2 tests failing now but both look unrelated (both from Drupal\FunctionalTests\DefaultContent\ContentExportTest).
Moving back to RTBC.
@godotislate pls check changing your BubbleableMetadata to CacheableMetadata just in case - added a comment on the MR.

Glancing at the last changes I am not sure this should have been set back to RTBC. Typically a committer will explicitly say that is OK.

I also don't see any review of the change record and the last time it was updated was in 2022. So that needs a review as well. If nothing else the branch and version numbers needs an update.

Coming back to be more explicit.
The change record needs work because, at least, the version and branch are incorrect. Also, a change record should lead with the change. and the two code blocks, if still relevant, should have some text to explain what they are for. For reference, https://www.drupal.org/community/contributor-guide/task/write-a-change-record-for-a-drupal-core-issue

There were quite a few questions and suggestions by @godotislate. They seem to be answered but this is not my area and the GitLab UI is not showing some of the changes made so I can't confidently resolve 'easy' threads. One of the suggestions was to use BubbleableMetadataach, which does have a reply but has not been reviewed. At least, that needs review before being passed to the committers.

I hope that helps.

I do wonder if this is a better fit for an alter hook.

We typically do alters for stuff like this and we'd be able to expose it for themes. An event subscriber won't work in themes.

Here's a hook solution (MR 15723), also updated the change record. There may be test failures but I tested current main and unfortunately those were pre-existing so someone is breaking things on main.
Moving to review, if any minor issues (phpstan baseline sort for example), I'll update on the fly.

re #79: We should consider though that LayoutBuilderEvents::SECTION_COMPONENT_BUILD_RENDER_ARRAY is already a thing in almost the same space. Offering one via event subscriber and the other in hook alter seems confusing.

True but I agree a hook will fit better here, that’s why I did it. We can have a follow up to convert the other one to an event and I can handle it too, not a very difficult change.

Yeah, I think this is something that themes will want to interact with, the right answer is likely a hook and deprecating those other events to hooks as well.