Convert views/templates/views-view-grouping.tpl.php to twig

Tagging.

Committed, thanks!

Moving to core queue

Ok for real this time:) Seemed to pass related tests locally, let's open this baby up.

Left some doc todos in there if someone want's to step up to the plate?

title update

Hm - I think I included this in the patch here: http://drupal.org/node/1912606

Yes, we ended up with a dupe. Closing this one since #1912606: Remove theme_views_view_grouping function is RTBC.

Resurrected. Only tpl converted. Grabbed pieces from #1912606: Remove theme_views_view_grouping function which has been stripped down to only the theme function removal.

tagging

weird, tagging again

nitpicks:

+ * - view: The View object.

We should avoid calling it an "object" in the template.

+ * - grouping_level: Integer indicating the hierarchical level of the grouping.

I know "integer" means something in math, not just as a PHP data type, but can we call it "number" instead?

I'm assigning myself to do some manual testing.

- * It is not actually used in default Views, as this is registered as a theme
- * function which has better performance. For single overrides, the template is
- * perfectly okay.

Why on earth did we get rid of this? it's still true. I just spent 15 minutes trying to figure out why the template in the patch wasn't doing anything.

Anyway, steps to test:

1. Copy the twig template into Bartik so that it actually gets used
2. Create 2 articles
3. Edit front page view to have grouping on *at least two fields* - grouping on one field won't trigger the view-grouping markup.
4. View the front page.

Note that the Pre-patch markup for this theme function is totally broken (it prints Array for content) and throws errors: "Notice: Array to string conversion in theme_views_view_grouping()"

Pre-patch markup (whitespace edited for easier diffing):

 <div class="view-grouping">
 <div class="view-grouping-header">Title: <a href="/d8html/node/2">l;asjdkf l;akdj f;</a>
 </div>
 <div class="view-grouping-content">Array</div>
 </div>
 <div class="view-grouping">
 <div class="view-grouping-header">Title: <a href="/d8html/node/1">laskjdfl;asf</a>
 </div>
 <div class="view-grouping-content">Array</div>
 </div>

Post patch markup (not broken):

<div class="view-grouping">
<div class="view-grouping-header">Title: <a href="/d8html/node/2">l;asjdkf l;akdj f;</a>
</div>
<div class="view-grouping-content">
<div class="item-list">
<h3>Author uid: <a href="/d8html/node/2">1</a>
</h3>
<ol class="custom-list-class">
<li  class="views-row views-row-1 views-row-odd views-row-first views-row-last custom-row-class">
<div class="views-field views-field-title">
<span class="views-label views-label-title">Title: </span>
<span class="field-content">
<a href="/d8html/node/2">l;asjdkf l;akdj f;</a>
</span>
</div>
<div class="views-field views-field-uid">
<span class="views-label views-label-uid">Author uid: </span>
<span class="field-content">
<a href="/d8html/node/2">1</a>
</span>
</div>
</li>
</ol>
</div>
</div>
</div>
<div class="view-grouping">
<div class="view-grouping-header">Title: <a href="/d8html/node/1">laskjdfl;asf</a>
</div>
<div class="view-grouping-content">
<div class="item-list">
<h3>Author uid: <a href="/d8html/node/1">1</a>
</h3>
<ol class="custom-list-class">
<li  class="views-row views-row-1 views-row-odd views-row-first views-row-last custom-row-class">
<div class="views-field views-field-title">
<span class="views-label views-label-title">Title: </span>
<span class="field-content">
<a href="/d8html/node/1">laskjdfl;asf</a>
</span>
</div>
<div class="views-field views-field-uid">
<span class="views-label views-label-uid">Author uid: </span>
<span class="field-content">
<a href="/d8html/node/1">1</a>
</span>
</div>
</li>
</ol>
</div>
</div>
</div>

@Cottser suggested in IRC that if I use the tpl.php file in HEAD it might fix the Array printing problem + errors, but it doesn't help at all. HEAD is broken however you look at it :(

 This is set via aggregation settings
+ *     on the view.

I'm not sure this is right since I managed to trigger the template without turning aggregation on at all.

Thanks for all your work on this @thedavidmeister!

This is a novice task and 'needs work' for #14 and #15.

The fact that the functionality doesn't work properly or as documented is a separate issue altogether and should be handled separately from the conversion. It looks like Twig makes things less broken though :)

Tagging for profiling.

#1912606: Remove theme_views_view_grouping function May have something to do with it, but now doesn't exist in head. So re: #15 it is now the case that it is always used.

#14 should be done in this.

Don't forget the weird comment in #19.

I'm going to manually try #15 again but the template was definitely not getting called when I was testing on the 11th of May, which is a few days after when that issue was committed on the 8th.

Cool, I can confirm #22 in that this template is now always called. I must have not pulled or something before I did the testing in #16.

So it's just the weird comment about aggregation settings now.

@thedavidmeister not sure what #19 is trying to say, could you clarify?

Also, you're not crazy, that fix just got in yesterday into core so it would have been 'broken' when you tested.

+ *   - content: The content to be grouped. This is set via aggregation settings
+ *     on the view.

This says the content is set via aggregation settings on the view, but if you look at the manual testing steps it's possible to trigger this template without aggregation enabled at all.

Hmm... ok I removed that note. I would have thought you would have needed aggregation on for that but I guess that makes sense.

+ *   - content: The content to be grouped. This is set via aggregation settings
+ *     on the view.

missed one, in the preprocess comment.

Assigning Issue from COH

+++ b/core/modules/views/templates/views-view-grouping.html.twigundefined
@@ -0,0 +1,23 @@
+ * - grouping: The grouping instruction.

I've been using views for years but "grouping instruction" has zero meaning to me, and this is particularly confusing in the template, because the variable isn't used or demonstrated how it could be used.

Granted, this is the exact pre-existing documentation but, while eyeballs are on this, would it be too much to ask for some clarifications about what exactly is a grouping instruction?

Also, "The View object", could use some attention per comment #14, though, i presume that the view object is used in other templates from views, so make this a follow-up doc cleanup issue so all templates are documented consistently.

Feel free to shoot me for throwing up road blocks at the last minute. I'd be happy to open these two things as follow-up issues.

#30 - jwilson3, would you mind sticking the grouping variable in the template and seeing what comes out the other end?

Interesting, in my test the {{ grouping }} variable is always empty, (and using {{ dump(grouping) }} causes warnings.

And what's more, the {{ grouping_level }} variable is always zero.

Should these be filed as separate follow-up issues, or be fixed here?

Followup for sure. These docs were pulled from tpl.php directly. If the variables are empty that's a problem but it doesn't have anything to do with the Twig template conversion.

@jwilson3 if you wouldn't mind creating a followup issue, postponing it and linking to it from here and the Twig conversion meta issue that would be great.

So we just need profiling and doc updates as per #28 and #14.

I'm unassigning this just because it's been a few days with no patch. Feel free to reassign @deanbowles if you're still working on this.

Follow-up added: #1997866: Erroneous Views grouping and grouping_level template variables.

Though, I see no reason to mark it postponed...

After testing against the tpl.php, it's clear that the grouping field is *not* empty for PHPTemplate, but is empty for Twig template only. In PHPTemplate it spits out the following information:

Grouping: Array
(
    [field] => created
    [rendered] => 0
    [rendered_strip] => 1
)

So this seems like at least the empty grouping variable is a bug introduced in this conversion code above.

#34 - mmk, well that's something we should fix here then.

Re-rolled per #33

tpl.php doesn't show any content in the view. I also get following notice: Notice: Array to string conversion in include() (line 23 of core/themes/bartik/templates/views-view-grouping.tpl.php).

The twig version shows the view as it should be.

{{ dump(grouping) }} :

object(Drupal\Core\Template\TwigReference)#901 (2) {
  ["writableRef":protected]=>
  &array(3) {
    ["field"]=>
    string(10) "field_tags"
    ["rendered"]=>
    string(1) "1"
    ["rendered_strip"]=>
    string(1) "0"
  }
  ["storage":"ArrayObject":private]=>
  array(3) {
    ["field"]=>
    string(10) "field_tags"
    ["rendered"]=>
    string(1) "1"
    ["rendered_strip"]=>
    string(1) "0"
  }
}

{{ dump(grouping_level) }} :
int(0)

var_dump($grouping) :

array(3) {
  ["field"]=>
  string(10) "field_tags"
  ["rendered"]=>
  string(1) "1"
  ["rendered_strip"]=>
  string(1) "0"
}

var_dump($grouping_level) :
int(0)

Screenshot of the source from tpl.php version

#37 - I'm not sure why this was set to "needs work"? What exactly are we doing here if the grouping field contains the same data in Twig and tpl.php?

@intergalactic o... That is saying to me that the PHP template version is broken, not the twig version... so could you open a followup for that PHP template issue with VDC tags and since this one is printing out the expected output it should be fine.

#36: 1843752-27-twig-views-grouping.patch queued for re-testing.

Couldn't find the reference in #14 @thedavidmeister. But did the other one #28

@Sean Charles sorry something happened with the patch, it didn't apply and testbot failed and there was no interdiff.

Looks good!

Profiling.

=== 8.x..8.x compared (519c03b86b1ec..519c045f200ea):

ct  : 154,347|154,347|0|0.0%
wt  : 1,007,555|1,005,183|-2,372|-0.2%
cpu : 940,129|939,055|-1,074|-0.1%
mu  : 9,726,456|9,726,456|0|0.0%
pmu : 10,013,924|10,013,924|0|0.0%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519c03b86b1ec&...

=== 8.x..1843752 compared (519c03b86b1ec..519c04ab45451):

ct  : 154,347|154,586|239|0.2%
wt  : 1,007,555|1,008,179|624|0.1%
cpu : 940,129|941,658|1,529|0.2%
mu  : 9,726,456|9,739,080|12,624|0.1%
pmu : 10,013,924|10,030,596|16,672|0.2%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519c03b86b1ec&...

+1. Ready for #1987510: [meta] Convert all core *.tpl.php templates to Twig as singular patch

added manual testing steps

Committed ed042be and pushed to 8.x. Thanks!