Problem/Motivation

#1875974: Abstract 'component type' specific code out of EntityDisplay sought to entirely refactor how fields are handled for EntityDisplay objects.
This is still desirable, but is a huge effort with tricky BC concerns.

Proposed resolution

In the meantime, the iterative step is to add native support for placing block plugins within an entity display.
Block plugins are a \Drupal\Core\Block concept, separate from the block.module

Remaining tasks

Decide if this is actually desirable
Figure out the UX

User interface changes

Yes

API changes

API additions only

Data model changes

Yes

Comments

tim.plunkett created an issue. See original summary.

tim.plunkett’s picture

Status: Active » Needs review
FileSize
42.17 KB
jonathanshaw’s picture

Thanks Tim! Obviously a huge win for sitebuilders here. Goodbye to a whole class of clunky contrib modules to achieve a very common use case.

swentel’s picture

Awesome. Tested it and works like a charm!

Only quickly scanned the code, looks fine to me.

+++ b/core/modules/field_ui/src/Form/EntityViewDisplayEditForm.php
@@ -83,6 +94,242 @@ protected function buildExtraFieldRow($field_id, $extra_field) {
+      // the new settings fro the next rebuild.

nitpick: missing 'm' in fro

Don't want to set it to needs work for now for missing things (e.g. config dependencies, post update hook to update entity displays) so more people can look at it.

dawehner’s picture

Just some quick comments ...

  1. +++ b/core/lib/Drupal/Core/Entity/Display/EntityViewDisplayInterface.php
    @@ -46,4 +46,45 @@ public function build(FieldableEntityInterface $entity);
    +   * @return array|null
    +   *   The array of display options, keyed by block name.
    ...
    +   *
    +   * @return \Drupal\Core\Block\BlockPluginInterface
    +   *   The instantiated block plugin.
    +   */
    +  public function getBlock($name);
    

    These methods names are weird ... given that one returns an object and the other one an array of configuration.

  2. +++ b/core/lib/Drupal/Core/Entity/Entity/EntityViewDisplay.php
    @@ -263,6 +308,23 @@ public function buildMultiple(array $entities) {
    +
    +        if ($block instanceof ContextAwarePluginInterface) {
    +          $contexts = \Drupal::service('context.repository')->getRuntimeContexts($block->getContextMapping());
    +          \Drupal::service('context.handler')->applyContextMapping($block, $contexts);
    +        }
    

    I'm wondering whether we could pass in the current rendered entity as context. With that we would have to rely less on block runtime contexts

groovedork’s picture

This is great, but as a sitebuilder I would prefer a solution described here, as it has a number of advantages:
- It offers a clean workaround for the problem that the title is not a field.
- It allows for fields to be placed in any of the theme's regions. This would be extremely valuable, and would allow much more flexibilty than this solution offers.

jonathanshaw’s picture

#7 no reason not to have both? They're not way duplicates AFAIK.

tim.plunkett’s picture

That other issue is an interesting idea, but as jonathanshaw points out it is not conflicting with this one.

groovedork’s picture

It may not conflict technically, but they do overlap functionally. Both systems could be used to place blocks in between fields.

Energy to work on Drupal may not be infinite. If energy could be redirected, then I propose to redirect it towards the solution that solves more problems (title does not behave like a field) and offers greater flexibility (mix blocks and fields into any region).

This solution, while wonderful in its own right, could lead to lethargy and slowdown in the development of a solution that I believe would be better, and also covers this use case.

tim.plunkett’s picture

Please comment on the other issue to encourage others to work on it, and stop derailing my efforts here. Thanks!

groovedork’s picture

Ok, sorry.