Problem/Motivation

While helping on #3542273: Remove EntityViewDisplay::getDisplayInfos , I have noticed our Entity View integration is maybe a bit too complicated:

Proposed resolution

TODO:

  • Remove duplicated DisplayBuilderEntityDisplayInterface::getDisplayBuilderOverrideField() implementations
  • Merge DisplayBuilderOverridableInterface into DisplayBuilderEntityDisplayInterface; because there are always implemented together
  • Replace <code>EntityViewDisplay::getDisplayInfos() by EntityViewOverride::getDisplayInfos()

Then, let's move import logic from DisplayBuilderEntityDisplayInterface::initialImport() to buildable plugin. It doesn't make sense anymore to have the logic split in 2 different place. Buildable plugin have already everything they need to carry those tasks.

This will also allow to split "Import from Layout Builder or Manage Display configuration." into:

  • Import from Layout Builder configuration.
  • Import from Manage Display configuration.
Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

pdureau created an issue. See original summary.

pdureau’s picture

Issue summary: View changes

pdureau’s picture

Status: Active » Needs review

it looks good so far...

pdureau’s picture

Assigned: pdureau » mogtofu33

Also tested locally, by clicking around in UI, and doing all kind of imports from Manage Display and Layout Builder.

pdureau’s picture

As a follow-up, we may be avle to totally remove the EntityViewDisplay VS LayoutBuilderEntityViewDisplay split (and hopefully the EntityViewDisplayForm VS LayoutBuilderEntityViewDisplayForm split).

We keep 2 different classes with explicit reference to Layout Builder because of the classes we are extending from Core:

  • LayoutBuilderEntityViewDisplay is extending Drupal\layout_builder\Entity\LayoutBuilderEntityViewDisplay instead of Drupal\Core\Entity\Entity\EntityViewDisplay
  • LayoutBuilderEntityViewDisplayForm is extending Drupal\layout_builder\Form\LayoutBuilderEntityViewDisplayForm instead of Drupal\field_ui\Form\EntityViewDisplayEditForm

Can we promote composition over inheritance?

  • EntityViewDisplay has already no logic of its own, just an empty shell for the EntityViewDisplayTrait
  • LayoutBuilderEntityViewDisplay is only implementing EntityInterface::preSave() which could be replaced by hook_entity_presave with conditions
  • EntityViewDisplayTrait is implementing/overriding:
    • EntityInterface::postSave() ???
    • EntityInterface::delete() could be replaced by hook_entity_delete?
    • ConfigEntityInterface::calculateDependencies() and ConfigEntityInterface::onDependencyRemoval() may be addressable with #3540069: Evaluate ObjectWithPluginCollectionInterface
    • EntityViewDisplayInterface::buildMultiple() could be replaced by hook_entity_view
    • DisplayBuilderEntityDisplayInterface::isDisplayBuilderEnabled() to move to buildable plugin
    • DisplayBuilderEntityDisplayInterface::getDisplayBuilderOverrideField() to move to buildable plugin
    • DisplayBuilderEntityDisplayInterface::getDisplayBuilderOverrideProfile() to move to buildable plugin
    • DisplayBuilderEntityDisplayInterface::isDisplayBuilderOverridable() to move to buildable plugin

We already implements hook_entity_delete, we can gather all hook implementations which is altering Entity View Display entity type into a single class which will look very similar to the current EntityViewDisplayTrait. Like an "entity type decorator"

For forms:

  • EntityViewDisplayForm has already no logic of its own, just an empty shell for the EntityViewDisplayFormTrait
  • DisplayBuildablePluginManager has just a little form alteration, which could be replaced by entiity_form_alter hook.
  • EntityViewDisplayFormTrait only overrides FormInterface::submitForm()

I will create the ticket once this one is merged.

  • mogtofu33 committed 9f30f64c on 1.0.x authored by pdureau
    task: #3593247 Move some entity view logic to buildable plugins
    
    By:...
mogtofu33’s picture

Assigned: mogtofu33 » Unassigned
Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

pdureau’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.