Make StylePluginBase::renderFields public

The reason is that I need to access cached fields (in $rendered_fields) and before accessing them, I have to cache the fields because this is not done automatically before hook_pre_render is called. You may refer to https://www.drupal.org/node/2826755 to get more info.

@dawehner:

I think implementing a FieldAwareStylePluginInterface would be similar to use a nuke to kill a fly. The method is defined directly in StylePluginBase and any style plugin extending it can access it. I don't see any reason why we should put this method in an interface. If it was defined in PluginBase (which is extended by StylePluginBase), my answer would have been different.

Hi,

Before going forward, I just realized the proposed method $DisplayPluginBase::getExtender(id) is not needed. I was able to use another option.

Now, here is a quick wrap-up. I ported Views Merge Rows to Drupal 8 few months ago because the module maintainer doesn't work on the project anymore. Basically, VMR is run through hook_views_pre_render() and a display extender plugin is designed to define a merge option on a per-field basis. Currently, no style plugin is coded in the module.

Here is a sample of the relevant Drupal 7 code (hook_views_pre_render()):

function views_merge_rows_views_pre_render(&$view) {
  $options = $view->display_handler->extender['views_merge_rows']->get_options();
  if (!$options['merge_rows']) {
    return;
  }
  $rendered_fields = $view->style_plugin->render_fields($view->result);

  [Fields treatment here...]

  // Store the merged rows back to the view's style plugin.
  foreach ($merged_rows as $row_index => $merged_row) {
    foreach ($options['field_config'] as $field_name => $field_config) {
      switch ($field_config['merge_option']) {
        case 'merge':
        case 'merge_unique':
          $view->style_plugin->rendered_fields[$row_index][$field_name]
            = implode($field_config['separator'], $merged_row[$field_name]);
          break;

  [...]

}

My main concern was to keep the module behavior as is. So, here is a sample of the relevant Drupal 8 code (hook_views_pre_render()) as it is currently, assuming patches as they are currently proposed are applied:

function views_merge_rows_views_pre_render(ViewExecutable $view) {
  $items_per_page = get_items_per_page_for_current_display($view);

  $extender = $view->display_handler->getExtenders()['views_merge_rows'];
  //$extender = $view->display_handler->getExtender('views_merge_rows');
  if (isset($extender)) {
      $options = $extender->get_options();
  }
  else {
      $options = NULL;
  }

  if (!is_null($options) && $options['merge_rows'] && $items_per_page > 0) {
    $view->setItemsPerPage(0);
  }
  if (!is_null($options) && $options['merge_rows'] != FALSE) {
      $view->style_plugin->renderFields($view->result);
      $rendered_fields = $view->style_plugin->getRenderedFields();

  [Fields treatment here...]

        foreach ($options['field_config'] as $field_name => $field_config) {
          switch ($field_config['merge_option']) {
            case 'merge':
            case 'merge_unique':
              foreach ($merged_row[$field_name] as $field_index => $field_value) {
                if (empty($field_value)) {
                  unset($merged_row[$field_name][$field_index]);
                }
              }
                if ($field_config['exclude_first']) {
                array_shift($merged_row[$field_name]);
              }
              $value_count = count($merged_row[$field_name]);
              $i = 1;
              foreach ($merged_row[$field_name] as $field_index => $field_value) {
                  if ($i <> $value_count) {
                    $merged_row[$field_name][$field_index] = $field_config['prefix'] . $field_value . $field_config['separator'] . $field_config['suffix'];
                  }
                  else {
                    $merged_row[$field_name][$field_index] = $field_config['prefix'] . $field_value . $field_config['suffix'];
                  }
                  $i++;
              }
              unset($i);
              unset($value_count);
              $view->style_plugin->setRenderedField(implode($merged_row[$field_name]), $row_index, $field_name);
              break;

  [...]
}

Now, regarding @andypost suggestions:
"And the main question in why not inherit your style plugin from base one and alter discovery or views style plugins with own implementation?"

Duplicating current views style plugins would work (I tested the option), but as I pointed out in a prior comment, the module users would be constrained to remain within these and this is not the module's intent. It is intended to be used with whatever Views style the user decides to use. This also has the drawback to duplicate styles in the selection form.

Inheriting a style plugin from base was also looked at before proposing the patches. But as mentioned before, required properties/methods would not be available to default styles. I could alter discovery here (would need to know how to implement that), but once again it would go against the intents behind the module.

With regards to making renderFields() method public, I don't think there is an issue here. The only way I could get rid of that need would be to use another hook where $rendered_fields has been rendered, but I still need an access to that data to manipulate it.

I am really open to suggestions. I would really be happy to close the issues before version 8.4 is released. I am not comfortable to tell my users to patch the core to use the module with Drupal 8. Many thanks in advance.

I forgot, here is a sample of renderFields():

  protected function renderFields(array $result) {
    if (!$this->usesFields()) {
      return;
    }

    if (!isset($this->rendered_fields)) {
      $this->rendered_fields = [];
      $this->view->row_index = 0;
      $field_ids = array_keys($this->view->field);

      [...]
  }

The function is called at rendering. It is explicitely checking whether $rendered_fields already exists. I examined the code and my understanding is that such case can only happen if renderFields() is run prior to rendering the view. So I also understand there is an implicit assumption here that renderFields() can be run during the pre-rendering process.

So, there is evidence that this method was intended to be public.