When a header (or footer) are added to a View this Twig logic, {% if header %}, seems to always be satisfied, regardless of whether the header (or footer) should appear or not. The if I'm looking at is in views-view.html.twig.

See attached screenshots.

The extra spacing before and after the text is likely being caused by some whitespacing in the Twig template, but that is all contained within the if.

Files: 
CommentFileSizeAuthor
#26 header_and_footer_are-2547811-26.patch2.65 KBnlisgo
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 111,666 pass(es). View
#26 interdiff-2547811-19-26.txt2.41 KBnlisgo
#19 header_and_footer_are-2547811-19.patch2.64 KBnlisgo
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 110,861 pass(es). View
#19 interdiff-2547811-12-19.txt1.92 KBnlisgo
#19 header_and_footer_are-2547811-19-testonly.patch1.92 KBnlisgo
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 110,836 pass(es), 2 fail(s), and 0 exception(s). View
#12 header_and_footer_are-2547811-12.patch735 bytesdavidhernandez
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 110,551 pass(es). View
#6 header_and_footer_are-2547811-6.patch4.21 KBborisson_
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 101,592 pass(es), 5 fail(s), and 0 exception(s). View

Comments

davidhernandez created an issue. See original summary.

borisson_’s picture

FileSize
94.8 KB

I can't seem to reproduce this issue:

davidhernandez’s picture

Try this to reproduce. Create a View, add a header and/or footer. Add conditions to the View so that no results are returned. You should get nothing, or empty text if configured, but should not get a header or footer div in the markup.

borisson_’s picture

Status: Active » Needs review
FileSize
639 bytes
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 101,539 pass(es), 9 fail(s), and 0 exception(s). View

Attached patch should resolve this issue. Phpunit tests all seem to pass, let's see what the rest of the tests think.

Status: Needs review » Needs work

The last submitted patch, 4: header_and_footer_are-2547811-4.patch, failed testing.

borisson_’s picture

Status: Needs work » Needs review
FileSize
3.58 KB
4.21 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 101,592 pass(es), 5 fail(s), and 0 exception(s). View

Fixes the Drupal\views_ui\Tests\PreviewTest test and adds a new scenario that specifically tests for this (testHeaderFooterText).

Status: Needs review » Needs work

The last submitted patch, 6: header_and_footer_are-2547811-6.patch, failed testing.

dawehner’s picture

+++ b/core/themes/classy/templates/views/views-view.html.twig
@@ -45,7 +45,7 @@
-  {% if header %}
+  {% if header and rows %}

@@ -82,7 +82,7 @@
-  {% if footer %}
+  {% if footer and rows %}

Mh I don't get the fix. Header/footer are visible much like the empty text potentially on an empty result.

Could it be the case that the empty logic currently comes to early and so is executed in the context of the initial render step and not the final deep tree one?
(simple render array vs. the entire rendering)

davidhernandez’s picture

Yeah, the solution doesn't seem right. The header and footer should not resolve true with no content. I wonder if this is a similar problem to the region checking problem. Also, I'm wondering if there is side case where header/footer can legitimately have content when there are no rows. Maybe with no content but there are attachments or something?

dawehner’s picture

Well, the configuration allows you to do it.

davidhernandez’s picture

This is what I get when dumping the header. First when the View has no result, then with a result.

array(1) {
  ["area_text_custom"]=>
  array(0) {
  }
}

array(1) {
  ["area_text_custom"]=>
  array(1) {
    ["#markup"]=>
    string(20) "This is header text."
  }
}

For comparison, this is what I get for the empty text.

array(0) {
}

array(1) {
  ["area"]=>
  array(3) {
    ["#type"]=>
    string(14) "processed_text"
    ["#text"]=>
    string(33) "This is empty text for this view."
    ["#format"]=>
    string(10) "basic_html"
  }
}
davidhernandez’s picture

Status: Needs work » Needs review
FileSize
735 bytes
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 110,551 pass(es). View

How about this?

andypost’s picture

+1 to #12

Manuel Garcia’s picture

+1 also to #12.

davidhernandez’s picture

That's two +1. Anyone care to rtbc?

joelpittet’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests

I think the solution is appropriate, or else we need to change the output of the render method on each which extends AreaPluginBase.

This does need tests though to ensure we don't break it again.

Should be able to assert that the .view-footer don't exist if the areas aren't used. Or create a custom template with some silly text inside those and assert it doesn't exist when that template is loaded. There is a test theme for views that may be a good candidate for that add a template to views_test_theme

Example:
/modules/views/tests/themes/views_test_theme/templates/views-view-fields.html.twig

nlisgo’s picture

Assigned: Unassigned » nlisgo

Preparing a test

nlisgo’s picture

Status: Needs work » Needs review
FileSize
1.92 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 110,836 pass(es), 2 fail(s), and 0 exception(s). View
1.92 KB
2.64 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 110,861 pass(es). View
nlisgo’s picture

Assigned: nlisgo » Unassigned

The last submitted patch, 19: header_and_footer_are-2547811-19-testonly.patch, failed testing.

The last submitted patch, 19: header_and_footer_are-2547811-19-testonly.patch, failed testing.

mansspams’s picture

Status: Needs review » Reviewed & tested by the community

#19 works as advertised.

mansspams’s picture

+++ b/core/modules/views/src/Tests/Handler/AreaTest.php
@@ -116,6 +116,44 @@ public function testRenderArea() {
+    $xpath = '//div[contains(@class, :class)]';

$xpath has been set on line 136 already. Should it be twice?

mansspams’s picture

Status: Reviewed & tested by the community » Needs review
nlisgo’s picture

FileSize
2.41 KB
2.65 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 111,666 pass(es). View

@mansspams good spot. I have tidied up the test a bit too and added some comments.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests

Nice!

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed d997d39 and pushed to 8.0.x. Thanks!

  • alexpott committed d997d39 on 8.0.x
    Issue #2547811 by nlisgo, borisson_, davidhernandez, dawehner, mansspams...

Status: Fixed » Closed (fixed)

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