Remove theme_views_view_grouping function

Thanks for the patch!

+++ b/core/modules/views/templates/views-view-grouping.html.twigundefined
@@ -0,0 +1,22 @@
+ * - content: The content of this template.

We should explain what this content is about. (the rendered style of the grouped view, or something like that).

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

What about mention the ViewExecutable object somehow here?

+++ b/core/modules/views/templates/views-view-grouping.html.twigundefined
@@ -0,0 +1,22 @@
+ * @see template_preprocess
+ * @see template_preprocess_views_view_grouping

Maybe also @see for \Drupal\views\Plugin\views\style\StylePluginBase::render_grouping_sets()

Thanks @jastraat, looks great just added a few nitpicks.

+++ b/core/modules/views/templates/views-view-grouping.html.twig
@@ -0,0 +1,24 @@
+ * - $view: The view object
+ * - $grouping: The grouping instruction.

Can you drop the $'s in the twig docblock?

+++ b/core/modules/views/templates/views-view-grouping.html.twig
@@ -0,0 +1,24 @@
+ * @see \Drupal\views\Plugin\views\style\StylePluginBase::render_grouping_sets()

That probably shouldn't be in there.

+++ b/core/modules/views/templates/views-view-grouping.html.twigundefined
@@ -0,0 +1,24 @@
+ * @see \Drupal\views\Plugin\views\style\StylePluginBase::render_grouping_sets()

I'm not against putting this in here, because it has the potential for people to understand where this is called, but yeah I don't care much whether it should stick or not.

+++ b/core/modules/views/views.theme.incundefined
@@ -296,26 +296,17 @@ function template_preprocess_views_view_fields(&$vars) {
+ * @param array $variables
+ *   An associative array containing:
+ *   - element: An associative array containing the properties of the element.

I'm quite sure that this piece is wrong. The elements of variables are view, title, content, rows and grouping_level

@dawehner - If the $hook for views_view_grouping is no longer be necessary in views_theme with the removal of the theme function that was used instead of a template, ( See https://drupal.org/node/1843746#comment-7271612 ) then that change to views.module should probably be a part of this patch as well. Could you comment?

Thanks!

No it's still needed. Should we let users test this manually?

I tested it in an installation of Drupal 8, but it never hurts to have other users try it. I've attached an updated patch with the changes from #10 and #11

Thanks @jastraat! I think this is looking good. Minor documentation nitpicks ahead :)

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

This needs to be a complete sentence (end in a period) per http://drupal.org/node/1354#drupal.

+++ b/core/modules/views/templates/views-view-grouping.html.twigundefined
@@ -0,0 +1,23 @@
+ * - content: The content to be grouped. This is set via aggregation settings on the view.

Over 80 characters, please wrap (ref. same link as above).

@Cottser - Sorry I missed the 80 character limit: I'll fix that.
Almost none of the variable descriptions are full sentences though; some of them are just longer fragments.

@jastraat - thanks, good point! Feel free to tweak the variable documentation, please include an interdiff. A few more documentation updates I noticed:

1. +++ b/core/modules/views/templates/views-view-grouping.html.twigundefined
   @@ -0,0 +1,23 @@
   + * @see template_preprocess
   + * @see template_preprocess_views_view_grouping
   

   These need to end in parens per http://drupal.org/node/1354#see, i.e. @see template_preprocess().

2. +++ b/core/modules/views/views.theme.incundefined
   @@ -296,26 +296,16 @@ function template_preprocess_views_view_fields(&$vars) {
   + * Prepares variables for a single grouping within a view.
   

   Per #1913208: [policy] Standardize template preprocess function documentation this should end in "templates", but I'd recommend splitting up the description a bit, something like:

    * Prepares variables for Views grouping templates.
    *
    * This template displays a single grouping within a view.
   

3. +++ b/core/modules/views/views.theme.incundefined
   @@ -296,26 +296,16 @@ function template_preprocess_views_view_fields(&$vars) {
   + * @param array $variables
   + *   An associative array containing the properties of the element.
   + *   Properties used: #view, #title, #content, #rows, #grouping_level.
   

   I think maybe we need another example in #1913208: [policy] Standardize template preprocess function documentation, the current example there only shows a render element, whereas this one has many variables being passed in.

   In this case it should be listing all the same variables as the template does (in almost exactly the same format), minus content. #1898460-6: taxonomy.module - Convert PHPTemplate templates to Twig has a good example of this.

Thanks @jastraat, the changes look good and the interdiff was very helpful! A couple minor tweaks, the first to the Twig template and the second to the preprocess docblock:

+++ b/core/modules/views/templates/views-view-grouping.html.twigundefined
@@ -7,12 +7,13 @@
  * - view: The view object

This will still need a period at the end.

+++ b/core/modules/views/views.theme.incundefined
@@ -296,13 +296,18 @@ function template_preprocess_views_view_fields(&$vars) {
+ *   - grouping_level: Integer indicating the hierarchical level of the grouping.

This is *just* over 80 characters, please wrap "grouping." to the next line.

Removed trailing space.

This seems to be perfect.

Code looks good, I just closed #1843752: Convert views/templates/views-view-grouping.tpl.php to twig as a dupe.

I did manual testing and the markup didn't show up. Adding 'template' gets better results, but I'd like someone else to confirm the markup is as expected. I was getting errors like the following before I applied the patch, so the output was actually quite a bit different after applying the patch (more rows were output after patching):

Notice: Array to string conversion in theme_views_view_grouping() (line 308 of core/modules/views/views.theme.inc).

I'd attach a view export but I guess I haven't really kept up with how that works in D8 :)

I created a view of nodes displaying fields, kept the default title field and added a "published status" field, then set "published status" as the #1 grouping field and "title" as the #2 grouping field.

Also if I'm not mistaken we should remove the 'pattern' key like we did in #1843746: Convert views/templates/views-view-field.tpl.php to Twig , discussed in #22 and #23 there.

I did manually test the patch from #21, and I didn't get the error you mentioned. However, I also don't think the template is being picked up. I created a unformatted list view of nodes grouped by publication year, and it appears that it's using the views-view-unformatted.tpl.php template. This was the case even after I added 'template' and removed 'pattern' from the views_theme() function.

Does anyone know exactly when the grouping template gets called?

Is the way to export a view now to grab the .yml file from files/config_* ?
For example - see attachment of the view I mentioned in #26.

The error was actually before I applied the patch. As far as I can tell, you need to add two groupings for this template to be used.

Nitpick for #24 - Could we capitalise "V" for a View when you're referencing a View object?

Updated with comments from #25 and #29. Unfortunately I wasn't able to manually test this because the most up-to-date pull of Drupal 8 just crashes on my local installation.

@jastraat - for the manual testing/fixing broken Drupal 8 you might try removing sites/default/files/php and truncating all your cache tables, that often helps. If not, dump your database, copy default.settings.php over settings.php and re-install Drupal 8.

@Cottser - I'm afraid I've tried both those methods already. Drupal 8 installs, but then immediately logs out the first user. Every time I try to load the site, the connection is reset. Note - I have a Drupal 7 site on this same installation of WAMP and am NOT having connection reset problems. I also wasn't having this problem with the Drupal 8 install prior to this week.

Just by looking at the error message I would say that's fine, because you removed the code which produces the error message anyway.

Updated issue summary.

This is now just remove the theme function. The tpl conversion is this way -> #1843752: Convert views/templates/views-view-grouping.tpl.php to twig

Titling as this now just removes the theme_ function.

This looks perfect.

Committed 1304f13 and pushed to 8.x. Thanks!