Problem/Motivation

It is not possible to filter the inline blocks available for placement by the section to which a block is being added.

#2973615: Add Section delta as extra data on filtered block definitions added the section delta as $extra data on filtered block definitions for non-inline blocks. However, that patch failed to analogously add the section delta for inline blocks. The intent of #2973615 clearly was to affect all blocks (not just non-inline blocks), but that was not the end result so I categorized this as a bug report.

Steps to reproduce

Try to filter the inline block list using hook_plugin_filter_TYPE__CONSUMER_alter by the section delta, it does not exist in $extra

Proposed resolution

Add the delta for inline blocks too.

Remaining tasks

Add tests
Review patch.

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#35 3038981-nr-bot_qin_7bib.txt90 bytesneeds-review-queue-bot
#32 3038981-nr-bot.txt90 bytesneeds-review-queue-bot
#19 Screenshot from 2021-11-09 10-37-54.png154.27 KBvikashsoni
#15 Screenshot from 2021-06-08 12-12-49.png134.81 KBrinku jacob 13
#13 3038981-13.patch814 byteskapilv
#11 3038981-3038981-inline-blocks-missing.patch996 bytesdpi
#2 drupal-inline-blocks-delta-3038981-1.patch1002 bytesjdleonard

Issue fork drupal-3038981

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:

About issue forks

Comments

jdleonard created an issue. See original summary.

jdleonard’s picture

jdleonard
he/him
Primary language English
Location Austin, Texas
commented
Assigned: jdleonard » Unassigned
Status: Active » Needs review
StatusFileSize
new drupal-inline-blocks-delta-3038981-1.patch1002 bytes
jdleonard’s picture

jdleonard
he/him
Primary language English
Location Austin, Texas
commented
Version: 8.7.x-dev » 8.8.x-dev

Does this need to be fixed in 8.8 before 8.7?

jdleonard’s picture

jdleonard
he/him
Primary language English
Location Austin, Texas
commented
Version: 8.8.x-dev » 8.7.x-dev
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Version: 8.7.x-dev » 8.8.x-dev
Status: Needs review » Needs work
Issue tags: +Blocks-Layouts, +Needs tests

Thanks for the patch!
This will need automated tests.
And yes, it will have to land in 8.8 first

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Issue tags: +sprint

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.

dpi’s picture

dpi
Location Perth, Australia
commented
StatusFileSize
new 3038981-3038981-inline-blocks-missing.patch996 bytes

Rerolling for D9

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented 
+++ b/core/modules/layout_builder/src/Controller/ChooseBlockController.php
@@ -173,8 +173,13 @@ class ChooseBlockController implements ContainerInjectionInterface {
   public function inlineBlockList(SectionStorageInterface $section_storage, int $delta, $region) {
+    // @todo Explicitly cast delta to an integer, remove this in
+    //   https://www.drupal.org/project/drupal/issues/2984509.
+    $delta = (int) $delta;

This cast isn't needed anymore since the method now specifies int as a type

kapilv’s picture

kapilv commented
Status: Needs work » Needs review
StatusFileSize
new 3038981-13.patch814 bytes

Address #12.

kim.pepper’s picture

kim.pepper
Primary language English
Location 🏄‍♂️🇦🇺Sydney, Australia
commented
Issue tags: +#pnx-sprint
rinku jacob 13’s picture

rinku jacob 13 commented
StatusFileSize
new Screenshot from 2021-06-08 12-12-49.png134.81 KB

patch #11 successfully applied for drupal 9.3.x dev

dpi opened merge request !1319

dpi’s picture

dpi
Location Perth, Australia
commented

Created MR from previous patches, reroll (merge) to 9.3.x

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

vikashsoni’s picture

vikashsoni commented
StatusFileSize
new Screenshot from 2021-11-09 10-37-54.png154.27 KB

@dpi and @jdleonard Patch not applying in drupal-9.3.x-dev

dpi’s picture

dpi
Location Perth, Australia
commented

@vikashsoni I've checked that the patch generated from the MR applies to HEAD of both 9.3 (drupal/core#871c1b7) and 9.4 (drupal/core#1ae88e0) cleanly as of this writing.

Don't use the patch files from this issue, per your screenshot.

Check the doc for how to use patches from MR's.

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.

smustgrave’s picture

smustgrave commented
Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs Review Queue Initiative

This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request as a guide.

For the the tests requested in #5

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.

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

acbramley’s picture

acbramley commented

Rebased the MR against 11.x. Still needs tests.

The previous issue added these in LayoutBuilderTest::testLayoutBuilderChooseBlocksAlter

acbramley’s picture

acbramley commented
Assigned: Unassigned » acbramley

will give the tests a stab

acbramley’s picture

acbramley commented
Assigned: acbramley » Unassigned
Status: Needs work » Needs review
Issue tags: -Needs tests

Extended the existing test coverage (since moved to LayoutBuilderBlocksTest)

Also replaced a few unnecessary HTTP requests with API and accessing the manage layout url directly.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work
Issue tags: +Needs steps to reproduce

Sorry not super clear what are the steps to trigger this issue?

acbramley’s picture

acbramley commented
Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs steps to reproduce
needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Needs review » Needs work
StatusFileSize
new 3038981-nr-bot.txt90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

acbramley’s picture

acbramley commented
Status: Needs work » Needs review
danielveza’s picture

danielveza
he/him
Location Brisbane, AU
commented
Status: Needs review » Reviewed & tested by the community

This makes sense to have. Done a review, code changes & test coverage look good. Happy to RTBC.

needs-review-queue-bot’s picture

needs-review-queue-bot commented
Status: Reviewed & tested by the community » Needs work
StatusFileSize
new 3038981-nr-bot_qin_7bib.txt90 bytes

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

acbramley’s picture

acbramley commented
Status: Needs work » Reviewed & tested by the community

bad bot.

quietone’s picture

quietone commented

I read the comments and the MR (not a code review) and didn't see anything amiss. I have updated credit.

  • catch committed cca42017 on 11.x 
    Issue #3038981 by jdleonard, dpi, yogeshmpawar, acbramley, tim.plunkett...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 11.x, thanks!

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

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

Maintainers, please credit people who helped resolve this issue.

catch closed merge request !1319

Status: Fixed » Closed (fixed)

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