Result summary Area plugin not displayed when there is no result.

Thank you for working on this issue! The fix itself looks totally fine.

Do you mind writing a test for this bug as well?

+++ b/core/modules/views/src/Plugin/views/area/Result.php
@@ -98,6 +97,10 @@ public function render($empty = FALSE) {
+    // Special treatment if the view is empty.
+    if (empty($total)) {
+      $start = 0;
+    }

Ideally this comment should answer "why" we need this. The code itself already kinda explains that :)

The tests are looking great. Here is a quick comment about the comment.

1. +++ b/core/modules/views/src/Plugin/views/area/Result.php
   @@ -98,6 +97,10 @@ public function render($empty = FALSE) {
   +    // If the view is empty, start = 1 instead of 0.
   +    if (empty($total)) {
   +      $start = 0;
   +    }
   

   Is there a reason why this comment doesn't match to the code? Note: it would be even better, if the comment would describe why we need to use a different start.

2. +++ b/core/modules/views/src/Plugin/views/area/Result.php
   @@ -108,14 +111,14 @@ public function render($empty = FALSE) {
   -    // Send the output.
   -    if (!empty($total)) {
   -      $output .= Xss::filterAdmin(str_replace(array_keys($replacements), array_values($replacements), $format));
   -    }
        // Return as render array.
   -    return array(
   -      '#markup' => $output,
   -    );
   +    if (!empty($total) || !empty($this->options['empty'])) {
   +      return array(
   +        '#markup' => Xss::filterAdmin(str_replace(array_keys($replacements), array_values($replacements), $format)),
   +      );
   +    }
   +
   +    return array();
   

   its good to fix that!

3. +++ b/core/modules/views/tests/src/Kernel/Handler/AreaResultTest.php
   @@ -0,0 +1,62 @@
   +    $output = \Drupal::service('renderer')->renderRoot($output);
   +    $this->setRawContent($output);
   +    $this->assertText('start: 0 | end: 0 | total: 0 | label: test_area_result | per page: 0 | current page: 1 | current record count: 0 | page count: 1');
   +  }
   

   Nice!

Well mini pagers are special :)

I think we need to add to the testResultEmpty test case. We need to test when the view has the empty option set to FALSE - to check nothing displays.

@Grimreaper do you mind posting an interdiff for it?

@Grimreaper
The problem is not a missing core committer, its about a missing reviewer

+++ b/core/modules/views/src/Plugin/views/area/Result.php
@@ -83,9 +82,11 @@ public function render($empty = FALSE) {
+    // Offset for start calculation.
+    $start_offset = empty($total) ? 0 : 1;

It would be nice to explain why this line is needed. The current documentation is about what is done.

Needs a reroll for #2776975: March 3, 2017: Convert core to array syntax coding standards for Drupal 8.3.x RC phase

some other things:

1. +++ b/core/modules/views/tests/src/Kernel/Handler/AreaResultTest.php
   @@ -0,0 +1,86 @@
   +  /**
   +   * Views used by this test.
   +   *
   +   * @var array
   +   */
   

   This can just be an {@inheritdoc}

2. +++ b/core/modules/views/tests/src/Kernel/Handler/AreaResultTest.php
   @@ -0,0 +1,86 @@
   +  /**
   +   * Tests the messages area handler.
   +   */
   

   Copy/paste leftover, needs to be updated to the current use case.

3. +++ b/core/modules/views/tests/src/Kernel/Handler/AreaResultTest.php
   @@ -0,0 +1,86 @@
   +  /**
   +   * Tests the messages area handler.
   +   */
   

   Copy/paste leftover, needs an update.

4. +++ b/core/modules/views/tests/src/Kernel/Handler/AreaResultTest.php
   @@ -0,0 +1,86 @@
   +    // Add a filtering to have no result.
   

   'Add a filter that will make the result set empty.' Something like that?

5. +++ b/core/modules/views/tests/src/Kernel/Handler/AreaResultTest.php
   @@ -0,0 +1,86 @@
   +    $view->setDisplay('page_1');
   +
   +    // Add a filtering to have no result.
   +    $view->displayHandlers->get('page_1')->overrideOption('filters', array(
   +      'name' => array(
   +        'id' => 'name',
   +        'table' => 'views_test_data',
   +        'field' => 'name',
   +        'relationship' => 'none',
   +        'operator' => '=',
   +        'value' => 'non-existing-name',
   +      ),
   +    ));
   

   Since the page_1 display doesn't have overridden filters, adding this again shouldn't be necessary right? It should already have this filter from adding it to the default display?

I can't reproduce the issue on the 8.4.x branch. Does this have been fixed in any way with 8.4 ?

I also remembered to convert array in short array syntax.

Good catch! This looks good now.

Manually tested this in 8.4.x again and still fails to show the summary for me without the patch applied. Don't know what's different for @Haza

Ok, got it. I added the wrong "field" type for the header (I added "Global : Text area")

Well, at least it made clear that the issue is very specific to the "issue summary" field.

Manually tested the patch with 8.x and using the correct field (this time), I can confirm it solves the issue.

Overlaps with #2569381: Drupal\views\Plugin\views\area\Result does an unnecessary XSS::adminFilter() , if that lands first we need to combine the tests from that with the tests added here.

Can we have some screenshots about this fix for making it easier for core committer to review?

Nice, fix, tests and screenshots everyone. Just one minor point around BC...

+++ b/core/modules/views/src/Plugin/views/area/Result.php
@@ -74,7 +74,6 @@ public function render($empty = FALSE) {
-    $output = '';

@@ -108,14 +111,14 @@ public function render($empty = FALSE) {
-    // Send the output.
-    if (!empty($total)) {
-      $output .= Xss::filterAdmin(str_replace(array_keys($replacements), array_values($replacements), $format));
-    }
     // Return as render array.
-    return [
-      '#markup' => $output,
-    ];
+    if (!empty($total) || !empty($this->options['empty'])) {
+      return [
+        '#markup' => Xss::filterAdmin(str_replace(array_keys($replacements), array_values($replacements), $format)),
+      ];
+    }
+
+    return [];

Whilst adding || !empty($this->options['empty'] is required for the fix. I'm not sure that we should change the return for empty result from ['#markup' => ''] to []. As far as I can see it's not necessary for the fix and potentially could introduce PHP notices/errors in custom code where people are relying on #markup being set even when there is nothing to render.

@alexpotts feedback has been addressed, still looks good, tests are still green.

Committed 2be2910 and pushed to 8.4.x. Thanks!

As we're in the run up to 8.3.0 setting to patch to be ported for commit to 8.3.1 as I think this is BC safe.

Committed b32e30b and pushed to 8.3.x. Thanks!