No results text appears twice

I think might be a duplicate of #2570895: FieldPluginBase can duplicate a suffix

I had the same problem working on my current project.

From \Drupal\views\Plugin\views\display\DisplayPluginBase::elementPreRender() when a view is empty we add the #empty property but also we add the #empty property when building the form:

    if ($view->hasFormElements()) {
      // Only render row output if there are rows. Otherwise, render the empty
      // region.
      if (!empty($element['#rows'])) {
        $output = $element['#rows'];
      }
      else {
        $output = $element['#empty'];
      }

      $form_object = ViewsForm::create(\Drupal::getContainer(), $view->storage->id(), $view->current_display, $view->args);
      $form = \Drupal::formBuilder()->getForm($form_object, $view, $output);
      // The form is requesting that all non-essential views elements be hidden,
      // usually because the rendered step is not a view result.
      if ($form['show_view_elements']['#value'] == FALSE) {
        $element['#header'] = array();
        $element['#exposed'] = array();
        $element['#pager'] = array();
        $element['#footer'] = array();
        $element['#more'] = array();
        $element['#feed_icons'] = array();
      }

      $element['#rows'] = $form;
    }

If we have a form we don't need to add the value #empty again to the output but let's see the result of the tests.

Assigned to @joyceg 1 year ago... I'm going to work on it. Needs tests.

Let's try again.

#16 works like a charm.

The text appears duplicated before the patch, and it deletes one of them.

I'll work in the test.

Sorry for the status change. I'll do the test.

@joe_carvajal Great thanks!

Anyway I think we could verify that the empty behaviors are empty. Let's try this patch.

Feel free to work on the test then. We need to parse and verify that the *Empty text* only appears once.

I tried it, but I don't know how to perform this test. I won't work in this issue from now on.

I talked with marthinal and I'll try again :)

To reproduce the issue you only need to add at least an exposed filter.

I added a view that reproduces the error, but the test isn't created yet.

I've worked on this issue and when the patch is not applied, the test also receive the text message once. If you access to the view through the navigator, you can see the message twice but only once in the test.

I attach the patch without the testing view created by @joe_carvajal (also thanks for that view). I have created the test getting directly the path /admin/content.

Thanks to @marthinal for his support. I really appreciate it. You are awsome!

+++ b/core/modules/views/src/Plugin/views/display/DisplayPluginBase.php
@@ -2195,6 +2195,9 @@ public function elementPreRender(array $element) {
+      if (!empty($element['#view']->empty)) {
+        $element['#empty'] = [];
+      }

It'd be kinda nice to have some documentation about what is going on here :)

Hi @dawehner i have made the improvement as per your suggestion . Please find the patch for the same

Step to test patch #31
1 - Install the minimal profile.
2 - Enable Views
3 - Go to admin/content and create content type and add new content.
4 - Go to admin/content page and filter "Published status" and found "No content available." text display in two times.
5 - Apply patch #31 and again display same page and found "No content available." text display in one time.

Issues is resolve and patch working fine

As discussed with @dawehner, the test should fail and we cannot reproduce the problem from the default theme (classy). We change the theme from the setup and I added logic to the tpl to only render the empty results when rows are empty.

+++ b/core/modules/views/templates/views-view.html.twig
@@ -51,8 +51,12 @@
-  {{ empty }}
+  {% if rows %}
+      {{ rows }}
+  {% elseif empty %}
+      {{ empty }}
+  {% endif %}
+

The only problem I have with that is that this doesn't fix the problem with custom templates. I assume this is fine.

The template in Classy already has this logic so it doesn't need to be updated.

+++ b/core/modules/views/templates/views-view.html.twig
@@ -51,8 +51,12 @@
-  {{ rows }}
-  {{ empty }}
+  {% if rows %}
+      {{ rows }}
+  {% elseif empty %}
+      {{ empty }}
+  {% endif %}

This is a case where we should actually update Stable too since it's not changing markup but fixing a bug with the display logic.

Also, the {{ rows }} and {{ empty }} are indented by two spaces too many. Looks to be indented four spaces instead of two.

Added the same logic to Stable views-view.html.twig and fixed the indents.

Only question is whether there needs to be a specific test for Stable.

I've tested the patch from #37.

This works as shown by marthinal's image in #8.

I removed the fix and ran the test and got this:

$ php core/scripts/run-tests.sh --verbose --class "Drupal\views\Tests\Plugin\ViewsNoResultsBehaviorTest"

Drupal test run
---------------

Tests to be run:
  - Drupal\views\Tests\Plugin\ViewsNoResultsBehaviorTest

Test run started:
  Saturday, May 13, 2017 - 14:27

Test summary
------------

Drupal\views\Tests\Plugin\ViewsNoResultsBehaviorTest          10 passes   1 fails                  3 messages
- Found database prefix 'test41674918' for test ID 4.

Test run duration: 33 sec

Detailed test results
---------------------


---- Drupal\views\Tests\Plugin\ViewsNoResultsBehaviorTest ----


Status    Group      Filename          Line Function                            
--------------------------------------------------------------------------------
⋮
Fail      Other      ViewsNoResultsBeh   42 Drupal\views\Tests\Plugin\ViewsNoRe
    Only one message should be present
    Value 1 is equal to value 2.

With the fix re-instated:

$ php core/scripts/run-tests.sh --verbose --class "Drupal\views\Tests\Plugin\ViewsNoResultsBehaviorTest"

Drupal test run
---------------

Tests to be run:
  - Drupal\views\Tests\Plugin\ViewsNoResultsBehaviorTest

Test run started:
  Saturday, May 13, 2017 - 14:28

Test summary
------------

Drupal\views\Tests\Plugin\ViewsNoResultsBehaviorTest          11 passes                            3 messages

Test run duration: 33 sec

Detailed test results
---------------------


---- Drupal\views\Tests\Plugin\ViewsNoResultsBehaviorTest ----


Status    Group      Filename          Line Function                            
--------------------------------------------------------------------------------
⋮
Pass      Other      ViewsNoResultsBeh   42 Drupal\views\Tests\Plugin\ViewsNoRe
    Only one message should be present

(NB: I've removed passes that aren't applicable.)

This has a fix in Stable as requested by Cottser in #36.

This all looks good and is RTBC.

1. +++ b/core/modules/views/src/Tests/Plugin/ViewsNoResultsBehaviorTest.php
   @@ -0,0 +1,45 @@
   +class ViewsNoResultsBehaviorTest extends ViewTestBase {
   

   This should be \Drupal\Tests\views\Functional\ViewTestBase

2. +++ b/core/modules/views/templates/views-view.html.twig
   @@ -51,8 +51,12 @@
   +  {% if rows %}
   +    {{ rows }}
   +  {% elseif empty %}
   +    {{ empty }}
   +  {% endif %}
   

   Also I'm concerned about introducing extra spaces with the new templates? Should we be doing something like

     {% if rows -%}
       {{ rows }}
     {%- elseif empty -%}
       {{ empty }}
     {%- endif %}
   

   ?

3. +++ b/core/modules/views/templates/views-view.html.twig
   @@ -51,8 +51,12 @@
   +
   

   How come the new line here?

+++ b/core/modules/views/templates/views-view.html.twig
@@ -51,8 +51,12 @@
-  {{ rows }}
-  {{ empty }}
+  {% if rows %}
+    {{ rows }}
+  {% elseif empty %}
+    {{ empty }}
+  {% endif %}

+++ b/core/themes/stable/templates/views/views-view.html.twig
@@ -49,8 +49,11 @@
-  {{ rows }}
-  {{ empty }}
+  {% if rows %}
+    {{ rows }}
+  {% elseif empty %}
+    {{ empty }}
+  {% endif %}

Per @alexpott's point, to get this to output the same amount of whitespace the following should work:

{% if rows %}
  {{- rows }}
{% elseif empty %}
  {{- empty }}
{% endif %}

or

{% if rows -%}
{{ rows }}
{% elseif empty -%}
{{ empty }}
{% endif %}

I think the if's themselves don't need to be "trimmed", just the part before the variable is printed.

From alexpott's comment in #40 I've:

1. changed the test to inherit from Drupal\Tests\views\Functional\ViewTestBase
2. added the triming using Cottser's option 2 in #41
3. removed the extra empty line

I have applied the patch mentioned in comment #42 and it works as expected.
PFA before and after patch screengrab.
Looks like RTBC.

Changing the status to RTBC

Thank you, everyone.

+++ b/core/modules/views/src/Tests/Plugin/ViewsNoResultsBehaviorTest.php
@@ -0,0 +1,45 @@
+class ViewsNoResultsBehaviorTest extends ViewTestBase {

This test seems out of place in the test hierarchy. Should it be somewhere in Drupal\Tests\views\Functional?

+++ b/core/modules/views/src/Tests/Plugin/ViewsNoResultsBehaviorTest.php
@@ -0,0 +1,45 @@
+  /**
+   * Setup test.
+   */

In other test classes, if setUp() has a Docblock at all, it's an {@inheritdoc}.

Patch #42 worked for me also.

#45 I added the docblock and it changed the test view to /functional namespace

Patch #49 works for me. Thank you!

Issue can be reproduced by setting "Stark" as admin theme in standard profile install.

Patch works #49 and resolves issue.

Committed 4284388 and pushed to 8.7.x. Thanks!

Is this something that could be backported to 8.6.x?