Problem/Motivation

From #1876964-34: Improve DX when writing style plugins by adding a renderRowGroup() function

<?php
     
//What if I don't want to use theme_views_view_grouping in my style plugin.
   
$theme_functions = views_theme_functions('views_view_grouping', $this->view, $this->view->display_handler->display);
    foreach (
$sets as $set) {
     
$row = reset($set['rows']);
     
// Render as a grouping set.
     
if (is_array($row) && isset($row['group'])) {
       
$output[] = array(
         
'#theme' => $theme_functions,
         
'#view' => $this->view,
         
'#grouping' => $this->options['grouping'][$level],
         
'#grouping_level' => $level,
         
'#rows' => $set['rows'],
         
'#title' => $set['group'],
        );
      }
?>

Proposed resolution

Replace views_view_grouping with $this->grouping_theme.
Or move $theme_functions = views_theme_functions('views_view_grouping', $this->view, $this->view->display_handler->display); to helper function.

Remaining tasks

Better Title.
Finalize the approach.
Create a patch.

User interface changes

None.

API changes

None

#1876964: Improve DX when writing style plugins by adding a renderRowGroup() function
#1961702: Rename renderSingleGroup to renderRowGroup

Files: 
CommentFileSizeAuthor
#10 1963420-10.patch1.34 KBjibran
PASSED: [[SimpleTest]]: [MySQL] 55,915 pass(es).
[ View ]
#10 interdiff.txt883 bytesjibran
#7 1963420-7.patch1.37 KBjibran
PASSED: [[SimpleTest]]: [MySQL] 56,245 pass(es).
[ View ]
#7 interdiff.txt763 bytesjibran
#2 1963420-2.patch1.37 KBjibran
PASSED: [[SimpleTest]]: [MySQL] 54,140 pass(es).
[ View ]

Comments

xjm’s picture

If we create new methods, we should camelCase() them. (Soon we will activate a big meta to rename all the methods in the codebase that are still under_scores.)

jibran’s picture

Status:Active» Needs review
StatusFileSize
new1.37 KB
PASSED: [[SimpleTest]]: [MySQL] 54,140 pass(es).
[ View ]

Thanks @xjm for explaining that.
Here is the patch using $this->groupingTheme approach.

dawehner’s picture

For sure this patch looks pretty fine

+++ b/core/modules/views/lib/Drupal/views/Plugin/views/style/StylePluginBase.phpundefined
@@ -83,6 +83,18 @@
+   *
+   * @see \Drupal\views\Plugin\views\style\StylePluginBase::render_grouping_sets()

@see should be always after everything else. See http://drupal.org/node/1354

jibran’s picture

Title:Improve DX when writing style plugins» Improve DX when writing style plugins by adding a groupingTheme variable

Thanks @dawehner for having a look on the patch.
I went through the http://drupal.org/node/1354#see and http://drupal.org/node/1354#var. While creating the patch I copied

  /**
    * Stores the rendered field values, keyed by the row index and field name.
    *
    * @see \Drupal\views\Plugin\views\style\StylePluginBase::render_fields()
    * @see \Drupal\views\Plugin\views\style\StylePluginBase::get_field()
    *
    * @var array|null
    */
  protected $rendered_fields;

@see should be always after everything else

It is true for methods but is it true for attributes?

dawehner’s picture

No idea, but I guess it might be better to be consistent.

jhodgdon’s picture

http://drupal.org/node/1354#order gives the order of elements.

jibran’s picture

StatusFileSize
new763 bytes
new1.37 KB
PASSED: [[SimpleTest]]: [MySQL] 56,245 pass(es).
[ View ]

Thank you @jhodgdon for pointing that out. I should have read it last time but it is damn big page :).

dawehner’s picture

Status:Needs review» Reviewed & tested by the community

Thank you.

jhodgdon’s picture

Yes, it is a big page. That is why we now have a nice table of contents at the top so you can find the section you need, hopefully! :)

A couple of notes here:
- You can just refer to @see StylePluginBase::render_grouping_sets() and leave out the namespace, since this is on the same class, if you want.
- I don't think the paragraph of explanation for the @var docs really tells us much that isn't in the first line, but I'm not really against having it there.
- The first line could use a little grammatical cleanup. I suggest:
The theme function used to render the grouping set.

jibran’s picture

StatusFileSize
new883 bytes
new1.34 KB
PASSED: [[SimpleTest]]: [MySQL] 55,915 pass(es).
[ View ]

Thanks @jhodgdon for the suggestions I have updated the patch accordingly

jhodgdon’s picture

Looks good to me, thanks!

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

Committed e614787 and pushed to 8.x. Thanks!

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