Improve DX when writing style plugins by adding a groupingTheme variable [#1963420]

Problem/Motivation

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

      //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

Comment	File	Size	Author
#10	1963420-10.patch	1.34 KB	jibran
#10
#10	interdiff.txt	883 bytes	jibran
#7	1963420-7.patch	1.37 KB	jibran
#7
#7	interdiff.txt	763 bytes	jibran
#2	1963420-2.patch	1.37 KB	jibran
#2

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

xjm

she/her

English

CreditAttribution: xjm commented 7 April 2013 at 00:01

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.)

Comment #2

jibran

he/him

Urdu

Sydney, Australia

CreditAttribution: jibran commented 9 April 2013 at 06:09

Status:

Active

» Needs review

File	Size
1963420-2.patch	1.37 KB

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

Comment #3

dawehner

German

CreditAttribution: dawehner commented 19 May 2013 at 16:59

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

Comment #4

jibran

he/him

Urdu

Sydney, Australia

CreditAttribution: jibran commented 19 May 2013 at 17:40

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?

Comment #5

dawehner

German

CreditAttribution: dawehner commented 20 May 2013 at 00:28

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

Comment #6

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 22 May 2013 at 18:53

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

Comment #7

jibran

he/him

Urdu

Sydney, Australia

CreditAttribution: jibran commented 22 May 2013 at 19:47

File	Size
interdiff.txt	763 bytes
1963420-7.patch	1.37 KB

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

Comment #8

dawehner

German

CreditAttribution: dawehner commented 22 May 2013 at 21:43

Status:

Needs review

» Reviewed & tested by the community

Thank you.

Comment #9

jhodgdon

she/her

English

CreditAttribution: jhodgdon commented 23 May 2013 at 13:34

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.