Problem/Motivation

\Drupal\layout_builder\Entity\LayoutEntityDisplayInterface::isOverridable()
\Drupal\layout_builder\Entity\LayoutEntityDisplayInterface::setOverridable()
\Drupal\layout_builder\DefaultsSectionStorageInterface::isOverridable()
\Drupal\layout_builder\DefaultsSectionStorageInterface::setOverridable()

Proposed resolution

Make a shared OverridableInterface

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork drupal-2985362

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

tim.plunkett created an issue. See original summary.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

tim.plunkett’s picture

Issue tags: +Blocks-Layouts

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

DanielVeza made their first commit to this issue’s fork.

danielveza’s picture

Opened a MR for this change

  • Adds a new Interface LayoutBuilderOverridableInterface
  • Adds this new interface to DefaultsSectionStorageInterface & LayoutEntityDisplayInterface
  • Removes the overridable functions from DefaultsSectionStorageInterface & LayoutEntityDisplayInterface as they're now part of the interface
mstrelan’s picture

Status: Active » Reviewed & tested by the community

This is straight forward and does as described. Only thing I'd say is we can just call it OverridableInterface instead of LayoutBuilderOverridableInterface as per the proposed resolution, but this is consistent with LayoutBuilderEnabledInterface so I'm not going to hold it up.

I also wondered if we need a change record, but if you're implementing one of the existing interfaces you'll automatically be implementing the new interface already, so you don't need to declare the implementation in your class.

danielveza’s picture

Only thing I'd say is we can just call it OverridableInterface instead of LayoutBuilderOverridableInterface as per the proposed resolution, but this is consistent with LayoutBuilderEnabledInterface

Yup - I had the same thoughts while developing this. I figured keeping it consistent with LayoutBuilderEnabledInterface was the right call, but I don't feel strongly either way

I also wondered if we need a change record, but if you're implementing one of the existing interfaces you'll automatically be implementing the new interface already, so you don't need to declare the implementation in your class.

Good question. I guess it could be worth it since it's a new interface, but I'm not sure if it's one that will be used on it's own very often. I'm happy to write up a quick one if needed

quietone’s picture

Status: Reviewed & tested by the community » Needs work

I'm triaging RTBC issues. I read the IS, the comments ant the MR. I didn't find any unanswered questions.

Regarding the name, there are no existing interfaces in core using the word 'overridable' plus 'overridable' is not in the American English dictionaries recommended by Drupal.org, and it is in the list of misspelled words used by cspell. I think it we should conform to current patterns and change it.

There are existing interfaces in core using the word 'override';

$ find . -iname \*override\*interface\*  -not -path '*tests/*' -not -path '*vendor*'
./core/lib/Drupal/Core/Config/ConfigFactoryOverrideInterface.php
./core/lib/Drupal/Core/Menu/StaticMenuLinkOverridesInterface.php
./core/modules/layout_builder/src/OverridesSectionStorageInterface.php
./core/modules/language/src/Config/LanguageConfigFactoryOverrideInterface.php

Will LayoutBuilderOverrideInterface convey the same meaning?

Setting to NW for naming. At least, we should not be using, 'overridable'

mstrelan’s picture

In that case we need to rename isOverridable method to canBeOverridden. I'd have to disagree with this. We have things like Stringable, I think this is fine as is.

acbramley’s picture

Status: Needs work » Reviewed & tested by the community

The word already has precedent in the method name isOverridable so I'm not sure how it could be against cspell? It definitely is a word in the english language, do we need to expand the dictionary?

Changing the semantics of this and having to do BC for changing the method name seems like unnecessary busy work.

kim.pepper’s picture

I agree with #19 and #20. There is a strong precedent in Symfony and the PHP language itself of appending the "-able" suffix to an interface name. I don't think it needs to be in the dictionary.

longwave’s picture

Version: 11.x-dev » 10.3.x-dev
Status: Reviewed & tested by the community » Fixed

Agree that renaming the existing methods is out of scope here, we just want to add a shared interface to the hierarchy.

Committed and pushed a99e0b5763 to 11.x and 277cf4a8bb to 10.3.x. Thanks!

  • longwave committed 277cf4a8 on 10.3.x
    Issue #2985362 by DanielVeza, tim.plunkett, mstrelan, quietone, kim....

  • longwave committed a99e0b57 on 11.x
    Issue #2985362 by DanielVeza, tim.plunkett, mstrelan, quietone, kim....

Status: Fixed » Closed (fixed)

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