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

Need to deal with this in a nicer way:
{{ row_classes[id] }}

Can someone point me in the right direction?

Tagging.

Moving to core queue

Conversion and cleanup from #1, fixed $vars to $variables, extra whitespace between $var = declarations. And docblock cleanups.

it prints out an empty class on the links. This is the outputted code:

<div class="view-content">
  <div class="item-list">
    <ul class="views-summary">
      <li><a class="" href="/test-view/1">1</a>
        (2)
      </li>
    </ul>
  </div>
</div>

this is in the frontend, in the backend, in the views administration, I have a class views-ajax-processed-processed

When I change row_classes[id] to row_classes.id in views-view-summary.html.twig it seems to work. The frontend shows no class (not even an empty one). In the backend the views-ajax-processed-processed is still there.

@intergalactic o... thank you for reviewing:) Here's a couple changes that make it, look prettier from twig and html.

woops

Tagging.

This seems to be nearly perfect.

+++ b/core/modules/views/templates/views-view-summary.html.twigundefined
@@ -0,0 +1,22 @@
+ * ¶

Unneeded space

+++ b/core/modules/views/templates/views-view-summary.html.twigundefined
@@ -0,0 +1,22 @@
+ * @ingroup themeable

I guess we can reference the preprocess function.

Yup, as per #12 need to remove trailing spaces on the end of lines.

Twig templates should have @see referencing their preprocess functions, eg:

 * @see template_preprocess()
 * @see template_preprocess_views_view_summary()

I'd also like to see some more documentation around the contents of each row in the template, like what are row.url, row.attributes, row.link?
They're all used in the new template without explanation.

@thedavidmeister Thanks for the review! For the extra docs for the conversion we are doing @todo if we aren't sure, but I gave it a crack anyways... if there are gripes on what I put, I will re-roll it with @todos. As the extra documentation may need to happen after conversion and probably by people who know better what the variables actually represent.

Got the bits from #12 and #13
Thank you too @dawehner!

+ * Prepares variables for printing a single record from a row, with fields.

Elsewhere I saw Cottser saying these are supposed to end in "templates." so maybe this should read: "Prepares variables for Views row, single record templates." or something... It's hard to phrase that without being awkward.

+ * - view: The view object.

The View object (capital V).

@todo is fine by me for variables but they were completely absent before, I like what you did in #14.

Other than tweaks, we need manual testing instructions in this issue summary and for somebody to do the testing now.

Regarding #16, for the more complex descriptions it's fine to break up the description between the summary and a paragraph after. e.g.:

Prepares variables for Views summary templates.

The summary prints a single record from a row, with fields.

Default template: views-view-summary.html.twig.

Making the documentation tweaks noted here and creating a new patch and interdiff would make a good novice task.

If you'd like to work on this, please assign the issue to yourself. If you get stuck, drop by #drupal-twig on IRC or post your questions here. Thanks!

Edit: added context to docs example.

Fixes from #16 and #17 thanks @thedavidmeister and @Cottser

The markup before and after patch in #18 match. The Twig version works as expected in my manual testing.

Before:

<div class="item-list">
  <ul class="views-summary">
      <li><a href="/test/article" class="views-ajax-processed-processed">Article</a>
              (3)
      </li>
    </ul>
</div>

After:

<div class="item-list">
  <ul class="views-summary">
      <li><a href="/test/article" class="views-ajax-processed-processed">Article</a>
              (3)
      </li>
    </ul>
</div>

Docs nitpick:

+ * - options.count: A views option for whether to show the count.
+ * - rows: The rows contained in this view.

Capital V for View(s).

Docs fixed.

+ * - rows: The rows contained in this view.

Capital V for View.

+++ b/core/modules/views/templates/views-view-summary.html.twig
@@ -0,0 +1,30 @@
+ *   - row.attributes: The summary link HTML attributes.
+ *   - row.url: The summary link URL.
+ *   - row.link: The summary link text.
+ *   - row.count: The number of items under this grouping.

The "row." doesn't need to be there on these lines. The object "row" is not a key it's somewhat arbitrary inside the loop. Please remove these:) The indent does the talking.

The attached patch addresses the code comment changes requested in #22 and #23.

+1

+++ b/core/modules/views/views.theme.incundefined
@@ -344,12 +344,19 @@ function template_preprocess_views_view_field(&$vars) {
+ * @param array $variables
+ *   An associative array containing:
+ *   - view: The View object.

Same comment as before: there is rows and result in there, and the documetation of the view objection should be improved. (see #1843766: Convert views/templates/views-view-table.tpl.php to twig)

added main meta

Nitpick:

+ * - rows: The rows contained in this View.

We'd generally add something like "Each row contains:"

+ * - options.count: A Views option for whether to show the count.

What's the point of this being options.count when we could just have a "display_count" flag. Is the "options" variable doing anything else at all? If so, why isn't anything else in options documented?

Tagging for profiling.

@thedavidmeister re #26 Options.count is how they did it before, not sure I want to make that change in this conversion? Maybe you can open a separate issue for that one?

I reverted $variables => $vars to make the conversion more clear. Also I have split that one out here: #1963942: Change all instances of $vars to $variables

I reverted my $row_classes to $row['attributes'] conversion as well and separated that out here: #1968398: Convert Views $row_classes to $row['attributes']

And did the V to v on views.

#28 - that makes sense, I've now seen the options.foo variable around in a few other patches. This one is just the only one with a single option so it threw me a bit.

What about this, we extend the docs a bit to use the indented form rather than foo.bar syntax?

+ * - options.count: A views option for whether to show the count.

Could be something like:

  - options: Options for the view. This contains:
    - count: A flag indicating whether the count should be displayed.

as per #1843764: Convert views/templates/views-view-summary-unformatted.tpl.php to twig, what about "Flags indicating how the summary should be displayed."?

How does this work for #29 and #30?

looks good, was manually tested in #19

I will work on profiling this.

Performance benchmarks on this look pretty good in my opinion. Here are the results (with APC class loader not enabled):

Run 519c153c76aa0 uploaded successfully for drupal-perf-drupalcon.
Run 519c1639640b3 uploaded successfully for drupal-perf-drupalcon.
=== 8.x..8.x compared (519c153c76aa0..519c1639640b3):

ct  : 82,764|82,764|0|0.0%
wt  : 570,052|570,243|191|0.0%
cpu : 546,364|546,237|-127|-0.0%
mu  : 53,512,856|53,512,856|0|0.0%
pmu : 53,653,184|53,653,184|0|0.0%

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

=== 8.x..1843762-views-view-summary compared (519c153c76aa0..519c1681d41e5):

ct  : 82,764|83,229|465|0.6%
wt  : 570,052|573,769|3,717|0.7%
cpu : 546,364|549,207|2,843|0.5%
mu  : 53,512,856|53,570,088|57,232|0.1%
pmu : 53,653,184|53,711,096|57,912|0.1%

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

mmm, 0.7% is a little bit on the high side as far as these conversions go. They're usually sitting at 0.2-0.6%.

That's with APC cache off. I think this is fine.

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

Committed 80cb1e4 and pushed to 8.x. Thanks!