Convert views/templates/views-view-list.tpl.php to twig

Not sure how to translate this one:

    {% for id,row in rows %}
      <li {{ row_classes[id] }}>{{ row }}</li>
    {% endfor %}

I would assume move towards this:

    {% for row in rows %}
      <li class="{{ row.attributes.class }}"{{ row.attributes }}>{{ row.content }}</li>
    {% endfor %}

Tagging.

Moving to core queue

Ok did the simple way to convert this, would prefer to rework the preprocessor to be closer to the second option in #1 but want to see if this will pass.

There are quite a few fails in the views module locally on my d8clean.dev as well but no obvious ones for this conversion so here goes nothing.

title update

Tagging.

@joelpittet
Do you have any preferences, whether all this twig issues or the improvements of the variables should get in first?

Review for #4:

wrapper_prefix, wrapper_suffix, rows and row_classes are variables used in the Twig template but not documented in the template.

 /**
  * Display the view as an HTML list element
  */

This is not how we document preprocess functions. See #1913208: [policy] Standardize template preprocess function documentation.

+    $variables['wrapper_prefix'] = '<div' . $wrapper_class . '>';
+    $variables['wrapper_suffix'] = '</div>';

We don't need these prefix/suffix variables, just the class. The div markup should be markup in the template.

+  $variables['list_type_prefix'] = '<' . $handler->options['type'] . $attributes . '>';
+  $variables['list_type_suffix'] = '</' . $handler->options['type'] . '>';

Same for this. Surely we don't need prefix/suffix but just <{{ list_type}} {{ attributes }}> ... </{{ list_type}}> in the Twig template.

-  template_preprocess_views_view_unformatted($vars);
+  template_preprocess_views_view_unformatted($variables);

Is it ok to call one preprocess directly from within another?

@dawehner re #14 I do not have a preference... same work to me either way.

Reworked the html strings out of the template. Had to add the wrapper if because it seems to only show up if there are classes to apply to it.

@thedavidmeister on your last point, I am not sure... that is how it was. @dawehner you may be able to answer that one?

Although I think that fail above is fake, I did notice a couple whitespace issues.

We need to avoid generating a HTML tag by printing a variable in a template like this:

+  <{{ list.type }}{{ list.attributes }}>
+    {% for id, row in rows %}
+      <li{{ row_classes[id] }}>{{ row }}</li>
+    {% endfor %}
+  </{{ list.type }}>

We ran into this in the item-list template, too. See this comment for a workaround.

Related: #1842034: Remove dynamic generation of HTML tags

Durrr...Friday afternoon coding

#18 - Don't worry, I think the Twig "is" not working as "==" is going to trip lots of people up when D8 gets released. I've done the same thing in my patches too...

+++ b/core/modules/views/templates/views-view-list.html.twigundefined
@@ -0,0 +1,44 @@
+  {% if list.type == 'ul' %}
+    <ul{{ list.attributes }}>
+  {% else %}
+    <ol{{ list.attributes }}>
+  {% endif %}
...
+  {% if list.type == 'ul' %}
+    </ul>
+  {% else %}
+    </ol>
+  {% endif %}

I'm wondering whether this is okay, as it makes the template pretty complicated.

#20 - @dawehner, see comment #14. You're now just saying the opposite of @jenlampton.

Ah I'm sorry. I'm not that much into twig.

+++ b/core/modules/views/views.theme.incundefined
@@ -841,20 +847,16 @@ function template_preprocess_views_view_list(&$vars) {
+    $variables['wrapper_attributes'] = new Attribute(array('class' => $wrapper_class));
...
+  $variables['list']['attributes'] = new Attribute(array('class' => $class));

Instantiating the Attribute objects is not needed here, they can just be arrays now that #1982024: Lazy-load Attribute objects later in the rendering process only if needed is in.

Sorry, wasn't paying enough attention when I posted #23. #1982024: Lazy-load Attribute objects later in the rendering process only if needed only works for 'attributes', 'title_attributes', and 'content_attributes'. Carry on :)

Sorry, I can't see where the manual testing was done for this issue. Feel free to RTBC and remove the tag or do the manual testing.

i'll do it.

post-patch markup:

      <div class="item-list">
      <h3>Title: <a href="/d8html/node/2">l;asjdkf l;akdj f;</a></h3>
  
      <ul class="custom-list-class">
  
          <li class="views-row views-row-1 views-row-odd views-row-first views-row-last custom-row-class">  
  <div class="views-field views-field-title">    <span class="views-label views-label-title">Title: </span>    <span class="field-content"><a href="/d8html/node/2">l;asjdkf l;akdj f;</a></span>  </div></li>
    
      </ul>
  </div>
<div class="item-list">
      <h3>Title: <a href="/d8html/node/1">laskjdfl;asf</a></h3>
  
      <ul class="custom-list-class">
  
          <li class="views-row views-row-1 views-row-odd views-row-first views-row-last custom-row-class">  
  <div class="views-field views-field-title">    <span class="views-label views-label-title">Title: </span>    <span class="field-content"><a href="/d8html/node/1">laskjdfl;asf</a></span>  </div></li>
    
      </ul>
  </div>

pre-patch markup:

<div class="item-list">      <h3>Title: <a href="/d8html/node/2">l;asjdkf l;akdj f;</a></h3>
    <ul class="custom-list-class">          <li  class="views-row views-row-1 views-row-odd views-row-first views-row-last custom-row-class">  
  <div class="views-field views-field-title">    <span class="views-label views-label-title">Title: </span>    <span class="field-content"><a href="/d8html/node/2">l;asjdkf l;akdj f;</a></span>  </div></li>
      </ul></div><div class="item-list">      <h3>Title: <a href="/d8html/node/1">laskjdfl;asf</a></h3>
    <ul class="custom-list-class">          <li  class="views-row views-row-1 views-row-odd views-row-first views-row-last custom-row-class">  
  <div class="views-field views-field-title">    <span class="views-label views-label-title">Title: </span>    <span class="field-content"><a href="/d8html/node/1">laskjdfl;asf</a></span>  </div></li>
      </ul></div>

with "normalised" whitespace for easy diffing:

pre:

<div class="item-list">
<h3>Title: <a href="/d8html/node/2">l;asjdkf l;akdj f;</a>
</h3>
<ul class="custom-list-class">
<li  class="views-row views-row-1 views-row-odd views-row-first views-row-last custom-row-class">
<div class="views-field views-field-title">
<span class="views-label views-label-title">Title: </span>
<span class="field-content">
<a href="/d8html/node/2">l;asjdkf l;akdj f;</a>
</span>
</div>
</li>
</ul>
</div>
<div class="item-list">
<h3>Title: <a href="/d8html/node/1">laskjdfl;asf</a>
</h3>
<ul class="custom-list-class">
<li  class="views-row views-row-1 views-row-odd views-row-first views-row-last custom-row-class">
<div class="views-field views-field-title">
<span class="views-label views-label-title">Title: </span>
<span class="field-content">
<a href="/d8html/node/1">laskjdfl;asf</a>
</span>
</div>
</li>
</ul>
</div>

post:

 <div class="item-list">
 <h3>Title: <a href="/d8html/node/2">l;asjdkf l;akdj f;</a>
 </h3>
 <ul class="custom-list-class">
 <li class="views-row views-row-1 views-row-odd views-row-first views-row-last custom-row-class">
 <div class="views-field views-field-title">
 <span class="views-label views-label-title">Title: </span>
 <span class="field-content">
 <a href="/d8html/node/2">l;asjdkf l;akdj f;</a>
 </span>
 </div>
 </li>
 </ul>
 </div>
 <div class="item-list">
 <h3>Title: <a href="/d8html/node/1">laskjdfl;asf</a>
 </h3>
 <ul class="custom-list-class">
 <li class="views-row views-row-1 views-row-odd views-row-first views-row-last custom-row-class">
 <div class="views-field views-field-title">
 <span class="views-label views-label-title">Title: </span>
 <span class="field-content">
 <a href="/d8html/node/1">laskjdfl;asf</a>
 </span>
 </div>
 </li>
 </ul>
 </div>

Updated issue summary.

Great, thanks!

Tagging for profiling.

Assigning to myself for profiling.

3 nodes on front page, rendered through views-view-list:

=== 8.x..8.x compared (519af7f3cd409..519af8b10af92):

ct  : 42,302|42,302|0|0.0%
wt  : 236,471|236,636|165|0.1%
cpu : 232,014|232,016|2|0.0%
mu  : 13,583,280|13,583,280|0|0.0%
pmu : 13,693,800|13,693,800|0|0.0%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519af7f3cd409&...

=== 8.x..1843754-views-view-list compared (519af7f3cd409..519af8e64a59b):

ct  : 42,302|42,521|219|0.5%
wt  : 236,471|237,893|1,422|0.6%
cpu : 232,014|232,014|0|0.0%
mu  : 13,583,280|13,606,832|23,552|0.2%
pmu : 13,693,800|13,715,720|21,920|0.2%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519af7f3cd409&...

edit: updated run.
edit2: another update, previous run was with twig_debug still enabled.

+1. Ready for #1987510: [meta] Convert all core *.tpl.php templates to Twig as singular patch

Committed 5717861 and pushed to 8.x. Thanks!