Allow multi-value field rendering to pass through Views rewrite only once

Patch no longer applies. It may be too late for 8.0, but perhaps allowed for 8.1?

If #2342287: Allow Twig in Views token replacement lands, we might be able to do this by providing a token for all the values of a multi-valued field and using Twig to style it as needed.

Pro: avoids clutter in the UI, allows even greater flexibility to site builders (eg: displays such as "item1, item2, and item3"), could add support for multi-value field tokens in contrib while waiting for acceptance into core.
Con: Less discoverable and requires knowledge of how to code Twig to make it work.

Thoughts?

+1 for adding this option. Have rerolled patch #2 against 8.0.x.

@kaare, exactly!

One issue: multi-valued fields run the rewrite on each value of that field, not once for the field itself. This makes sense when you're wanting to rewrite each term a page is tagged with, for example.

Using the terms-on-a-page example, the term handler would need to 1) provide a Views token of all the terms in a single item, and b) only execute the render code once for that field. I'm not sure the best way to handle the UI as the site builder would need to control it -- in code we don't know if they want to rewrite each value or rewrite all the values at once. I suppose it would have to be somewhere in the field handler...

(Edit: Duh... I just realized that's exactly where you proposed your change as well! At least we're clearly thinking along the same lines!)

I've got a feeling that this is going to be too much change, too late in the product. And there's no reason it can't be done in contrib.

I've been thinking about this idea for a while now -- I'm happy to fiddle with it and roll a patch, but I don't want to hijack your issue and change its scope. Let me know if you'd prefer this in a separate issue or not. (assuming I can make it work...)

OK, hijacking away! :P

The attached patch provides a "Render all items together" option when "Display all values in the same row" is checked. When enabled, multi-valued fields in a Views row will run renderAltered() once for each field rather than once for each value in each field. In addition, a foo_multiple is added so you can render the field using Twig.

To solve the initial use case -- rendering a single value (field_tags, in this case) as plain HTML and multiple values as a <ul> -- you would check the "Render all items together" option and use a rewrite similar to the following:

{% if field_tags_multiple|length > 1 %}
  <ul>
  {% for item in field_tags_multiple %}
    <li>{{ item }}</li>
  {% endfor %}
  </ul>
{% elseif field_tags_multiple|length == 1  %}
  {{ field_tags_multiple|first }}
{% else %}
  There are no tags for this item.
{% endif %}

The result is:

Things still to work on:

1. UI cleanup, could use a better explanation of what this new option really does. As was suggested by @kaare, there should be a doc page on d.o that could be linked to from the description.

2. I really want someone that knows Twig and security to tell me if there's a problem with the "unfiltering" I'm doing of admin-entered Twig:

    // Xss::filterAdmin encodes '>' and '<' to '&gt;' and '&lt;'. Since these
    // are both valid in Twig, we un-encode those that appear within block
    // delimiters ('{%' and '%}') before handing it back to Twig for processing.
    $text = preg_replace(
      ['/\{%([\s\S]*?)&gt;([\s\S]*?)%\}/', '/\{%(.*?)&lt;(.*?)%\}/'],
      ['{%$1>$2%}', '{%$1<$2%}'],
      $text
    );

I'm trying to keep the un-encoding as limited as possible, but you never know...

#2 is actually a more generic issue and, if this issue gets postponed it might be worth a separate issue for that. Otherwise something like {% if foo|length > 1 %} causes Twig to barf. And that seems like a crucial fix if #2342287: Allow Twig in Views token replacement is going to be worth anything...

Summary update. Hiding screenshots that are no longer valid with the new approach and (in case we return to the former fix) everything except the most recent patch of the previous approach that got a green from the testbot.

Hopefully fixes tests... Let's see what the testbot thinks!

Tagging for reroll.

Rerolled

Thanks @jyotisankar, a couple things I noticed…

1. +++ b/core/modules/field/config/schema/field.views.schema.yml
   diff --git a/core/modules/views/src/Plugin/views/PluginBase.php b/core/modules/views/src/Plugin/views/PluginBase.php
   old mode 100644
   new mode 100755
   
   +++ b/core/modules/views/src/Plugin/views/field/Field.php
   diff --git a/core/modules/views/src/Plugin/views/field/FieldPluginBase.php b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php
   old mode 100644
   new mode 100755
   

   Mode changes snuck in.

2. +++ b/core/modules/views/src/Plugin/views/PluginBase.php
   @@ -364,6 +364,15 @@ protected function viewsTokenReplace($text, $tokens) {
   +    ¶
   
   +++ b/core/modules/views/src/Plugin/views/field/Field.php
   @@ -522,7 +526,21 @@ function multiple_options_form(&$form, FormStateInterface $form_state) {
   +    ¶
   
   +++ b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php
   @@ -1145,7 +1151,20 @@ public function advancedRender(ResultRow $values) {
   +          ¶
   ...
   +          ¶
   

   Unwanted trailing whitespace snuck in.

For me this rebases cleanly following the steps at http://drupal.org/patch/reroll with no conflicts. Maybe give it another shot @jyotisankar?

While there are other eyes looking at this issue, I would love some feedback on what I'm doing as mentioned in #11 -- see item #2 in that comment.

Thanks!

Reroll.

The patch in #21 still applies. Forgot to remove the "needs reroll" tag...

Added child issue to deal with #11. If we can get that into core, we may be able to handle this in contrib...

... And that other issue is #2466931: Valid Twig syntax is incorrectly escaped in Views rewrites.

Since this is followup from #2342287: Allow Twig in Views token replacement.

Tagging for reroll.

Rerolled the patch.

#21 still applies for me. Running it past the testbot to be sure...

#27 seems to add a bunch of unrelated stuff to the Views field schema.

Ack... Nevermind, I was on an experimental branch. Will reroll #21 shortly.

Looks like the schema definition for views.field.field moved from field.views.schema.yml to views.field.schema.yml.

Yeah, that's not confusing!

Updating the issue summary to remove this stuff #2466931: Valid Twig syntax is incorrectly escaped in Views rewrites. Needs work for the test coverage.

Note: tests for this are potentially blocked on #2346615: Views doesn't have multi-valued field unit tests.

Following an IRC conversation with @dawener, he suggested refactoring advancedRender to wrap a new doAdvancedRender (or possibly doAdvancedRenderText to clarify what we're rendering) and push the code that does the multi-value parsing into Field.php.

That avoids the need to do things like:

+++ b/core/modules/views/src/Plugin/views/field/FieldPluginBase.php
@@ -1138,8 +1138,15 @@ public function advancedRender(ResultRow $values) {
+        if ($this instanceof Field && $this->options['group_rows_render_together'] && $this->options['alter']['alter_text']) {

Also the existing if ($this instanceof MultiItemsFieldHandlerInterface) in FieldPluginBase would be removed.