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

not sure about:

class="{{ row_classes[row_count]|join(' ')}}" should probably be converted to something like
class="{{ row.attributes.class }}"{{ row.attributes }}?

Tagging.

Moving to core queue

Tried to dive into this.
There's more going on here that I think I am read for re-twig/drupal.
going to switch over to some reviewing.

Getting some better results from....

<table{{ attributes }}>
  {% if title %}
    <caption>{{ title }}</caption>
  {% endif %}
  {% if header %}
    <thead>
      <tr>
        {% for field, label in header %}
          <th {% if header_classes[field] %}{{ header_classes[field] }}{% endif %}>
            {{ label }}
          </th>
        {% endfor %}
      </tr>
    </thead>
  {% endif %}
  <tbody>
    {% for row_count, row in rows %}
      <tr{% if row_classes[row_count] %}{{ row_classes[row_count] }}{% endif %}>
        {% for field, content in row %}
          <td {% if field_classes[field][row_count] %}{{ field_classes[field][row_count] }}{% endif %}>
            {{ content }}
          </td>
        {% endfor %}
      </tr>
    {% endfor %}
  </tbody>
</table>

joelpittet,
I am getting this a little better, going to try to keep banging on it.

@webthingee sorry I was giving this a crack and didn't see the message:( This is my crack.

I got most of the attributes on the objects except for row_classes. And fixed the broken summary attribute. But there is still an issue with the colspan on empty issues.

+++ b/core/modules/views/templates/views-view-table.html.twigundefined
@@ -0,0 +1,56 @@
+ * Template to display a view as a table.

I guess it should be Default ...

+++ b/core/modules/views/templates/views-view-table.html.twigundefined
@@ -0,0 +1,56 @@
+ * - header.label: Header labels.
+ * - header.attributes: Header HTML classes keyed by field id.

I'm wondering whether this is the way to document an array of FOO, containing all label and attributes. This feels not right for me.

+++ b/core/modules/views/templates/views-view-table.html.twigundefined
@@ -0,0 +1,56 @@
+ * - field.classes: HTML Classes to apply to each field, indexed by

"classes" instead of "Classes" here.

+++ b/core/modules/views/views.theme.incundefined
@@ -578,89 +588,89 @@ function template_preprocess_views_view_table(&$vars) {
+    $variables['rows'][$num][$column]->content = $view->display_handler->renderArea('empty');
...
+    $variables['rows'][$num][$column]->attributes = new Attribute(array(

Sure that $num and $column is right here?

Re-titling.

Tagging.

Tagging.

There's already a review for this in #9.

Additional points:

There's mention of PHP data types in the Twig template docs which need to be removed.

There are variables being used in the Twig template that are not documented:

field.attributes, field.label

 /**
  * Display a view as a table style.
  */

This comment is not right for a preprocess function at all. Should be of the form "Prepares variables for ... templates.". Need to reference the template it's for and add an @param for $variables.

-  // We need the raw data for this grouping, which is passed in as $vars['rows'].
+  // We need the raw data for this grouping, which is passed in as $variables['rows'].
   // However, the template also needs to use for the rendered fields.  We
-  // therefore swap the raw data out to a new variable and reset $vars['rows']
+  // therefore swap the raw data out to a new variable and reset $variables['rows']

These refactored comments are now over 80 characters long so need to be wrapped/re-written appropriately.

Reference for the pending preprocess function standards: #1913208: [policy] Standardize template preprocess function documentation

Ok reverted the $vars=>$variables on this one cus it's big and makes it hard to see the changes. And added some doc cleanup.

yeah should have tested that one... I made that fix in a separate issue because I accidentally made a regression with the way the columns are built. Also, timplunkett was preferring arrays over objects so I made that change.

Ok back to straight conversion, we can fix the row_classes bits here #1968398: Convert Views $row_classes to $row['attributes']

1. Added content to a view for testing patch to ensure new view twig
2. Applied patch from #20 successfully
3. Cleared Cache
4. Confirmed that views tables are stilled rendered correctly with patch!

Tested as part of the Florida Drupal Camp code sprint 2013

Thanks aczietlow for manually testing the table.

Just some nitpicks.

+++ b/core/modules/views/templates/views-view-table.html.twigundefined
@@ -0,0 +1,55 @@
+ * Template to display a view as a table.

... Should be "Default theme implementation"..., see http://drupal.org/node/1823416

+++ b/core/modules/views/views.theme.incundefined
@@ -445,37 +445,44 @@ function template_preprocess_views_view_summary_unformatted(&$vars) {
+ * @param array $variables
+ *   An associative array containing:
+ *   - view: A View object.

I think documenting $vars['rows'] and $vars['result'] would be helpful as well. I think "A View object" should be better something like "A ViewExecutable object".

CNW for #23 and:

+++ b/core/modules/views/views.theme.incundefined
@@ -445,37 +445,44 @@ function template_preprocess_views_view_summary_unformatted(&$vars) {
+ * @param array $variables
...
 function template_preprocess_views_view_table(&$vars) {

@param array $vars for now.

@dawehner re #23 I added rows but $vars['results'] = $vars['rows'] and is documented inline, so I left that one out.

Did #24 as well.

nitpick:

+ *     - classes: HTML classes to apply to each field, indexed by
+ *       field id, then row number. This matches the index in rows.

capitalise id to ID and we don't need the second comma in this sentence.

looks RTBC to me other than that.

Tagging for profiling.

Removed an 80 liner and touched up docs to #26

Profiling this

Profiling for this is complete.

Scenario setup

1. Clean install
2. Enable Devel
3. Enable Devel Generate
4. Generate 50 nodes
5. Switch frontpage view to use a table style
6. Add Title field to display
7. Create block view, showing one full node. Place that block on the front page

Results

=== 8.x..8.x compared (519ab5e205edd..519ab6224a5d1):

ct  : 39,343|39,343|0|0.0%
wt  : 193,732|193,819|87|0.0%
cpu : 183,149|182,789|-360|-0.2%
mu  : 14,516,520|14,516,520|0|0.0%
pmu : 14,691,960|14,691,960|0|0.0%

=== 8.x..1843766-twig-views-view-table compared (519ab5e205edd..519ab64453886):

ct  : 39,343|39,742|399|1.0%
wt  : 193,732|194,054|322|0.2%
cpu : 183,149|182,878|-271|-0.1%
mu  : 14,516,520|14,564,024|47,504|0.3%
pmu : 14,691,960|14,738,368|46,408|0.3%

We just need to get an API key for @jerdavis so he can upload these results and the ones at #1843770: Convert views/templates/views-view-unformatted.tpl.php to twig for full review. Thanks @jerdavis!

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

Uploaded results.

=== 8.x..8.x compared (519ab5e205edd..519ab6224a5d1):

ct  : 39,343|39,343|0|0.0%
wt  : 193,732|193,819|87|0.0%
cpu : 183,149|182,789|-360|-0.2%
mu  : 14,516,520|14,516,520|0|0.0%
pmu : 14,691,960|14,691,960|0|0.0%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519ab5e205edd&run2=519ab6224a5d1&source=drupal-perf-drupalcon&extra=8.x..8.x

Run 519ab5e205edd uploaded successfully for drupal-perf-drupalcon.
Run 519ab64453886 uploaded successfully for drupal-perf-drupalcon.
=== 8.x..1843766-twig-views-view-table compared (519ab5e205edd..519ab64453886):

ct  : 39,343|39,742|399|1.0%
wt  : 193,732|194,054|322|0.2%
cpu : 183,149|182,878|-271|-0.1%
mu  : 14,516,520|14,564,024|47,504|0.3%
pmu : 14,691,960|14,738,368|46,408|0.3%

http://www.lionsad.de/xhprof-kit/xhprof/xhprof_html/?run1=519ab5e205edd&run2=519ab64453886&source=drupal-perf-drupalcon&extra=8.x..1843766-twig-views-view-table

Committed 6ac1d47 and pushed to 8.x. Thanks!