Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.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.
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | ds-fluid_layouts_empty_columns-2649208-8-D8.patch | 6.87 KB | floydm |
| #8 | interdiff-6-8.txt | 223 bytes | floydm |
| #6 | interdiff-4-6.txt | 5.43 KB | floydm |
| #6 | ds-fluid_layouts_empty_columns-2649208-6-D8.patch | 6.9 KB | floydm |
| #4 | ds-fluid_layouts_empty_columns-2649208-4-D8.patch | 1.84 KB | floydm |
Comments
Comment #2
altrugon CreditAttribution: altrugon commentedComment #3
floydm CreditAttribution: floydm at Affinity Bridge commentedA starting patch that does what the ticket description suggests.
Comment #4
floydm CreditAttribution: floydm at Affinity Bridge commentedThis patch also checks if the render array returned from $block->build() is empty, which I believe addresses your second scenario.
Comment #5
aspilicious CreditAttribution: aspilicious commentedThank 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
Comment #6
floydm CreditAttribution: floydm at Affinity Bridge commentedAdding 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.
Comment #7
aspilicious CreditAttribution: aspilicious commentedThe 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.
Comment #8
floydm CreditAttribution: floydm at Affinity Bridge commentedThank you for catching those. Fixed now.
Comment #9
aspilicious CreditAttribution: aspilicious commentedLooking great, need to verify this myaelf during the weekend.
Thnx for all the effort!
Comment #10
altrugon CreditAttribution: altrugon at Affinity Bridge commentedComment #11
aspilicious CreditAttribution: aspilicious commentedCode looks good, I'll test this today.
Comment #12
aspilicious CreditAttribution: aspilicious commentedThnx floydm for all those wonderful patches WITH tests :D
I can use more people like you in the queue :)