I have noticed that the fluid layouts always render the columns even if these are empty, the reason of that is that currently the code is checking if left or right are empty, which they are not because they are object.

Current code:

{#
...
 * - footer: content of footer region
 */
#}

{% if (left and not right) or (right and not left) %}
  {% set layout_class = 'group-one-column' %}
{% endif %}

Instead the code should check if the render of those objects will be empty, something like this (followed example):

{#
...
 * - footer: content of footer region
 */
#}

{% set left = left|render %}
{% set right = right|render %}

{% if (left and not right) or (right and not left) %}
  {% set layout_class = 'group-one-column' %}
{% endif %}

Another problem that I have run into with emptiness is the dynamic fields. If you create a field block (view's block in my scenario) the block renders code even if the content is empty and you have specified in the view to now render anything in this case.

Please take a look to the attached screen shot for more details.

Thank you so much for your work.

Members fund testing for the Drupal project. Drupal Association Learn more

Comments

altrugon created an issue. See original summary.

altrugon’s picture

Issue summary: View changes
floydm’s picture

floydm’s picture

This patch also checks if the render array returned from $block->build() is empty, which I believe addresses your second scenario.

aspilicious’s picture

Version: 8.x-2.2 » 8.x-2.x-dev
Status: Active » Needs work
Issue tags: +Needs tests

Thank you!
I approve this patch but I can't commit it at the moment.

Changed the version, in order to hopefully fire the test bot.
This also needs a test...

1) Select a fluid layout
2) Leave one region empty
3) Check with xpath if the classes are set

There are examples enough in the tests.
So it should be doable.

Bonus points for the person/team that can finish this patch. :D

floydm’s picture

Adding test to verify that with a fluid layout the empty columns drop out.

Setting to needs review so the testbot will fire. Leaving the "needs tests" issue tag because I'm not certain this is adequate.

aspilicious’s picture

The test is perfect.

2 small problems left looking at the patch file
- in you test CSS there is a tab
- the ds test layout info file needs a newline at the end.

floydm’s picture

aspilicious’s picture

Looking great, need to verify this myaelf during the weekend.

Thnx for all the effort!

altrugon’s picture

aspilicious’s picture

Status: Needs review » Reviewed & tested by the community

Code looks good, I'll test this today.

aspilicious’s picture

Status: Reviewed & tested by the community » Fixed

Thnx floydm for all those wonderful patches WITH tests :D

I can use more people like you in the queue :)

  • aspilicious committed 07bc6e2 on 8.x-2.x authored by floydm
    Issue #2649208 by floydm: Fluid layouts render empty columns
    

Status: Fixed » Closed (fixed)

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