Hello,

Empty blocks are shown when they are empty, I think buildRegions show ask for content before adding them to region.

            $build[$region][$block_id] = $block_render_array;

Attached is a patch.

CommentFileSizeAuthor
#8 buildregions_should-2660242-8.patch10.03 KBlegolasbo
#5 buildregions_should-2660242-5.patch1.82 KBm.abdulqader
#2 buildregions_should-2660242-2.patch1.44 KBm.abdulqader
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

m.abdulqader created an issue. See original summary.

m.abdulqader’s picture

m.abdulqader CreditAttribution: m.abdulqader at Vardot commented
FileSize
buildregions_should-2660242-2.patch1.44 KB
m.abdulqader’s picture

m.abdulqader CreditAttribution: m.abdulqader at Vardot commented
Assigned: m.abdulqader » Unassigned
samuel.mortenson’s picture

samuel.mortenson
he/him
Primary language English
CreditAttribution: samuel.mortenson at Acquia commented

A similar fix is going to be included in #2664002: Quickedit not working for Content Blocks rendered using the StandardDisplayBuilder, but I'll keep this issue open until this or that one is closed.

m.abdulqader’s picture

m.abdulqader CreditAttribution: m.abdulqader at Vardot commented
FileSize
buildregions_should-2660242-5.patch1.82 KB

Temporary solution until solving this issue.
This will check for empty blocks and empty regions as well.

Status: Needs review » Needs work

The last submitted patch, 5: buildregions_should-2660242-5.patch, failed testing.

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo for Acquia commented
Assigned: Unassigned » legolasbo

Working on this.

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo for Acquia commented
Status: Needs work » Needs review
FileSize
buildregions_should-2660242-8.patch10.03 KB

Attached patch is a new approach to this issue. It fixes the issue and refactors the build method into smaller methods to make this class more self documenting.

The gist of it is first build the regions and then add the prefix and suffix to the result of that build operation. Since only non-empty regions are added to the build, there will never be a prefix/suffix added to an empty region.

While working on this I also found out that the code was somewhat duplicated in InPlaceEditorDisplayBuilder, but differed in the attributes added to the wrapper. I therefor added a protected method calculateRegionWrapperAttributes which is now overridden by InPlaceEditorDisplayBuilder to add it's data attribute.

After this patch is committed there should be several follow-ups.

  1. Refactor StandardDisplayBuilderTest into smaller units to make them more readable and maintainable.
  2. Complete test coverage for StandardDisplayBuilder so we have our bases covered.
  3. Add test coverage for InPlaceEditorDisplayBuilder
  4. Clean upInPlaceEditorDisplayBuilder
legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo for Acquia commented
Assigned: legolasbo » Unassigned
japerry’s picture

japerry
Location KVUO
CreditAttribution: japerry at Drupal Association commented
Status: Needs review » Closed (fixed)

So this bug really was fixed in #2664002: Quickedit not working for Content Blocks rendered using the StandardDisplayBuilder so I'm marking this closed/fixed for now. If we want to refactor these other files, we should open up a different issue for that.