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

Separated from meta with attributes.class added

+++ b/core/themes/stark/templates/views/views-view.html.twigundefined
@@ -0,0 +1,92 @@
+  ¶

Be carefull wth trailing whitespaces. There are 2 of them

Tagging.

Thanks for cleaning up my extra spaces, is there anything else you changed? @tsi

Nope, just that.

+++ b/core/themes/stark/templates/views/views-view.html.twig
@@ -0,0 +1,92 @@
+ * - css_name: A css-safe version of the view name.
+ * - css_class: The user-specified classes names, if any
+ * - header: The view header
+ * - footer: The view footer
+ * - rows: The results of the view query, if any
+ * - empty: The empty text to display if the view is empty
+ * - pager: The pager next/prev links to display, if any
+ * - exposed: Exposed widget form/info to display
+ * - feed_icon: Feed icon to display, if any

Just a little nitpicking, otherwise good to go I think.
Documentation lines should end with a period.
css_class description should read 'class' not 'classes.

Excellent @tsi, this is complete now AFAICT. Not marking RTBC though, as I have no idea how to test manually. Leave that for someone else.

Committed. My apologies for messing up the attribution #8, was trying to reconcile a large number of patches and I had a stupid.

Moving to core queue

Moved over to the right place for core, replaced all the is defined with just {% if var %} because that should be closer and strict_variables is turned off anyway so I don't think 'is defined' does much.

I don't think this will pass all tests and may need to change some tests but it will get this party started...

#14: twig-views-view-1843744-14.patch queued for re-testing.

removed the length bit and cleaned up some docs.

There are two fails I can see. They are to do with TwigReference with an empty string not evaluating as empty.

Ok @Cottser helped me, not sure how this fixed things... but it worked!

+++ b/core/modules/views/templates/views-view.html.twigundefined
@@ -0,0 +1,92 @@
+ * Main view template.

Should be probably "Default theme implementation for main view template."

+++ b/core/modules/views/templates/views-view.html.twigundefined
@@ -0,0 +1,92 @@
+ *    through CSS.
+ *    Which includes:
+ *     .view
+ *     .view-[css_name]
+ *     .view-id-[view_name]
+ *     .view-display-id-[display_name]
+ *     .view-dom-id-[dom_id]

The indentation seems to be too much. I guess 3 spaces could be better.

+++ b/core/modules/views/templates/views-view.html.twigundefined
@@ -0,0 +1,92 @@
+  {% if rows is not empty %}

I'm wondering when we should use "if foo is not empty" and when just "if foo". Would be cool just that I can understand it.

To be honest I'm sad about the variables conversions in this issues, as this causes bike-shedding, rerolling of other patches etc.

Can we convert core/modules/views/templates/views-view--frontpage.tpl.php here as well? Seems rather similar and I don't think we have an issue covering that one. If it doesn't make sense to do as part of this issue we can create a new issue for it.

I think for the $vars -> $variables we could probably leave those, but if we're changing the lined up assignments and so on, I'm not sure where we draw the line.

@dawehner Thanks for the doc help first.

The biggest difference for twig if's is:
{% if rows is not empty %} is different with 0 and '0' being TRUE and those would be FALSE with {% if rows %} so in this case your right, it's not needed because that is usually an array or empty array and will be treated the same for both cases.

So if you expect a 0/'0' then check with 'is not empty':)

Can you point to the issues that are causing re-rolls due to $vars to $variables conversion? Should create issues for these fixes? Why would this be a bikeshed if $variables is the standard for the rest of core? I don't mind reverting these, just wonder where people are having issues. I sympathize with @Cottser, the line is gray.

Added doc changes and not empty changes from #23 and views-view--frontpage from #24

The biggest difference for twig if's is:
{% if rows is not empty %} is different with 0 and '0' being TRUE and those would be FALSE with {% if rows %} so in this case your right, it's not needed because that is usually an array or empty array and will be treated the same for both cases.

So if you expect a 0/'0' then check with 'is not empty':)

That's cool. Thanks for guiding me in twig!

Can you point to the issues that are causing re-rolls due to $vars to $variables conversion? Should create issues for these fixes? Why would this be a bikeshed if $variables is the standard for the rest of core? I don't mind reverting these, just wonder where people are having issues. I sympathize with @Cottser, the line is gray.

That has been just a general statement, but yeah let's better just do it here, and if something needs a rerole, it needs a rerole. Let's be pragmatic here.

+++ b/core/modules/views/templates/views-view.html.twigundefined
--- /dev/null
+++ b/core/modules/views/tests/views_test_data/templates/views-view--frontpage.html.twigundefined

Good catch with the template file! So i'm wondering about the following: doesn't twig support inheritance of template files? Would this allow us here to pust reference the other file, and that's it? This would reduce the maintenance overhead a bit.

Tagging.

@dawehner - I think the answer to your question "doesn't twig support inheritance of template files [in core]" from #26 is: Maybe later :)

@Cottser @dawehner I've moved this issue from our sandbox to the core queue: #1783184: [meta] Use Twig template inheritance Hopefully we can't forget to revisit inheritance before D8 ships. :)

I also added a note about test modules, but I'm not sure that's a good use case for inheritance. Codeblocks for the sake of testing seem, overkill.

Also, testing this one manually for you.

It looks like the only difference here is that we add four extra new-lines in the Twig version. But either way, there are way too many new-lines :) I'm going to clean some of these out and do a quick re-roll. Other than this, this file looks great!

Before:

<div  class="view view-frontpage view-id-frontpage view-display-id-page_1 view-dom-id-751819de2280edcf420d556d5918ffc2dd4986b8f5ba059b6c5d7cf3352611cb">
        
  
  
      <div class="view-content">
        <div  class="views-row views-row-1 views-row-odd views-row-first">
      
  <div class="views-field views-field-title">        <span class="field-content"><a href="/node/4">Also an article</a></span>  </div>  </div>
  <div  class="views-row views-row-2 views-row-even views-row-last">
      
  <div class="views-field views-field-title">        <span class="field-content"><a href="/node/1">I am an artcle</a></span>  </div>  </div>
    </div>
  
  
  
  
  
  
</div>

After:

<div class="view view-frontpage view-id-frontpage view-display-id-page_1 view-dom-id-d9c2e45678482bc2b791322cfcbfa1c9c68cce825d8ae3cd597ed4d7a7a116c5">

  
    

  
  
  
      <div class="view-content">
        <div  class="views-row views-row-1 views-row-odd views-row-first">
      
  <div class="views-field views-field-title">        <span class="field-content"><a href="/node/4">Also an article</a></span>  </div>  </div>
  <div  class="views-row views-row-2 views-row-even views-row-last">
      
  <div class="views-field views-field-title">        <span class="field-content"><a href="/node/1">I am an artcle</a></span>  </div>  </div>

    </div>
  
  
  
  
  
  
</div>

A few other notes:

• It looks like other views issues are holding back on the changing from $vars to $variables because that should be handled by #1963942: Change all instances of $vars to $variables. So I rolled those back so the patch there wouldn't fail.
• I also added preprocess doc blocks as per #1913208: [policy] Standardize template preprocess function documentation
• I also removed a few lines from the output, as intended above.

Now after, with a few less lines:

<div class="view view-frontpage view-id-frontpage view-display-id-page_1 view-dom-id-7267e255137fb10270840dceb928d2f8601bc27e72b63cd0407a5184e04a4bed">
  
    
      
      <div class="view-content">
        <div  class="views-row views-row-1 views-row-odd views-row-first">
      
  <div class="views-field views-field-title">        <span class="field-content"><a href="/node/4">Also an article</a></span>  </div>  </div>
  <div  class="views-row views-row-2 views-row-even views-row-last">
      
  <div class="views-field views-field-title">        <span class="field-content"><a href="/node/1">I am an artcle</a></span>  </div>  </div>

    </div>
  
          </div>

Thanks! These docs tweaks apply to both Twig templates in the patch:

+++ b/core/modules/views/templates/views-view.html.twigundefined
@@ -0,0 +1,82 @@
+ * - attributes.class: HTML classes that can be used to style contextually
+ *    through CSS. Which includes:
+ *    .view
+ *    .view-[css_name]
+ *    .view-id-[view_name]
+ *    .view-display-id-[display_name]
+ *    .view-dom-id-[dom_id]

Combine the first two sentences, the second is a fragment. "through CSS, including:"

Make each class a bullet in a list and remove the dots from the classes since we're talking about markup here. Also 'through' on the second line should not be indented by one space - see http://drupal.org/node/1354#lists, it should match the indent level.

+++ b/core/modules/views/templates/views-view.html.twigundefined
@@ -0,0 +1,82 @@
+ * - css_name: A css-safe version of the view name.

Capitalize CSS here, capitalize 'View' (there are other spots where it makes sense to capitalize 'View' as well).

I am going to work on this issue.

Attached patch.

Fixed whitespace issue.

Wow, great job, looking really close. I think you meant this to be a comma instead of a period:

+ * - attributes.class: HTML classes that can be used to style contextually
+ *    through CSS. including:

how about "style contextually through CSS, including:"

Other than this one small change, this looks RTBC to me!

Fixed small grammer.

Thanks @kgoel! The lists of classes need a bit of touch-up (check the indent and make sure there is a space after the bullet). See the example in http://drupal.org/node/1354#lists for reference. The template in views_test_data is closer to being correct.

Patch attached.

Thanks @kgoel! An interdiff is helpful to show the changes made between patches as well :)

core/modules/views/tests/views_test_data/templates/views-view--frontpage.html.twig:

+++ b/core/modules/views/tests/views_test_data/templates/views-view--frontpage.html.twigundefined
@@ -0,0 +1,77 @@
+ * - attributes.class: HTML classes that can be used to style contextually
+ *   through CSS, including:
+ *   - view
+ *   - view-[css_name]
+ *   - view-id-[view_name]
+ *   - view-display-id-[display_name]
+ *   - view-dom-id-[dom_id]

This one is perfect.

core/modules/views/templates/views-view.html.twig:

+++ b/core/modules/views/templates/views-view.html.twigundefined
@@ -0,0 +1,82 @@
+ * - attributes.class: HTML classes that can be used to style contextually
+ *    through CSS, including:
+ *   - view
+ *   - view-[css_name]
+ *   - view-id-[view_name]
+ *   - view-display-id-[display_name]
+ *   - view-dom-id-[dom_id]

The bullets on this one need to be indented by one more space, and the second line "through CSS" is indented by one space too many, it should line up with the previous line.

Thanks Cottser!

Changed status

+ * - view: The view object.
views module.
+ * - directory: The location of the views module.
"Views" is capital V

+ * Prepares variables for container templates.

For views_ui_preprocess_views_view(), this is a module implementing hook_preprocess_views_view() so shouldn't the docs be "Implements hook_preprocess_views_view()."?

+ * Prepares variables for view templates.

View templates -> capital V

+ {% if header is not empty %}

There's a bunch of "is not empty" checks that seem superfluous, {% if foo %} should be enough.

+ * @ingroup views_templates

We're not using this in views template docs anymore. We *are* using @ingroup themeable though so that needs to be added.

Attributes being used in the template but not documented: attachment_after, attachment_before, title, title_prefix

@ thedavidmeister- thanks for the feedback. Updated patch based on comments in #44.

Updated status

+++ b/core/modules/views/views.theme.incundefined
@@ -72,10 +79,10 @@ function template_preprocess_views_view(&$vars) {
+  $empty_message = empty($view->result);
+  $vars['header'] = $view->display_handler->renderArea('header', $empty_message);
+  $vars['footer'] = $view->display_handler->renderArea('footer', $empty_message);
+  $vars['empty_message'] = $empty_message ? $view->display_handler->renderArea('empty', $empty_message) : FALSE;

Do we really have to rename the variable. I don't like the new name, because it doesn't respect the fact that empty is an area like header/footer, so it also contains things others then just a simple text.

@dawehner yes because it a name conflict with 'is empty'. Would you rather change them all to header_area/footer_area/empty_area?

I don't see how $empty conflicts with "is empty". I think $empty_area is fine, no need to change footer/header.

One thing this patch does is change "view" to "View", which is not something we do anywhere in the Views module. I'd rather see it left as is.
I see @Cottser suggested this in #33, but AFAIK it's not something we plan on doing anywhere else, and having it different on the theme layer is weird.

Technically speaking it's also wrong. It's the ViewExecutable object, not the View storage object.

I agree that empty_area is a better name

I'm not sure where the 'capitalize View' thing first came up, personally I'd rather it be lowercase. Thoughts?

Also:

+++ b/core/modules/views/views_ui/views_ui.moduleundefined
@@ -232,7 +232,16 @@ function views_ui_cache_set(ViewUI $view) {
+ * @param array $variables
...
 function views_ui_preprocess_views_view(&$vars) {

@param array $vars

It's possible I suggested capitalizing View in the first place. Either way, my thought is that we don't capitalize node, taxonomy, or block, so why should a view be any different?

#51 - We don't have to capitalize the "V" - although I do find it weird that the module "Views" always has a capital "V" but a "view" wouldn't.

We should pick something and write it somewhere to be referenced though, because there's quite a few patches that will need a re-roll if we swap to lowercase.

#45: twig-views-view-1843744-45.patch queued for re-testing.

Tagging for reroll.

mm

Merge conflict here:

/**
<<<<<<< HEAD:core/modules/views_ui/views_ui.module
 * Theme preprocess for views-view.tpl.php.
=======
 * Specialized cache function to add a flag to our view, include an appropriate
 * include, and cache more easily.
 */
function views_ui_cache_set(ViewUI $view) {
  if (isset($view->locked) && is_object($view->locked) && $view->locked->owner != $GLOBALS['user']->uid) {
    drupal_set_message(t('Changes cannot be made to a locked view.'), 'error');
    return;
  }

  $view->changed = TRUE; // let any future object know that this view has changed.

  $executable = $view->get('executable');
  if (isset($executable->current_display)) {
    // Add the knowledge of the changed display, too.
    $view->changed_display[$executable->current_display] = TRUE;
    unset($executable->current_display);
  }

  // Unset handlers; we don't want to write these into the cache
  unset($executable->display_handler);
  unset($executable->default_display);
  $executable->query = NULL;
  unset($executable->displayHandlers);
  drupal_container()->get('user.tempstore')->get('views')->set($view->id(), $view);
}

/**
 * Implements hook_preprocess_views_view().
 *
 * Prepares variables for container templates.
 *
 * Default template: views-view.html.twig.
 *
 * @param array $variables
 *   An associative array containing:
 *   - view: The view object.
 *   - directory: The location of The views module.
>>>>>>> Patch #45:core/modules/views/views_ui/views_ui.module
 */

Decided to keep both hunks. Not sure if that's correct or not.

views_ui_cache_set was removed in #1919700: [Change notice[ Replace views_ui_cache_set with a method on the ViewUI object so should not be added back here. The * Theme preprocess for views-view.tpl.php. line should be removed from the patch here as well.

Changes as requested in #58.

• Remove views_ui_cache function
• Remove template preprocess comment

+++ b/core/modules/views/views.theme.incundefined
@@ -831,7 +838,7 @@ function template_preprocess_views_view_unformatted(&$vars) {
+ * Display the View as an HTML list element

This still needs to have the case reverted. Throughout most of the file.

Changes capitalization of view/views as per IRC discussion.

+++ b/core/modules/views/templates/views-view.html.twigundefined
@@ -0,0 +1,82 @@
+ * Default theme implementation for main View template.
...
+ * - css_name: A CSS-safe version of the View name.
...
+ * - header: The View header.
+ * - footer: The View footer.
+ * - rows: The results of the View query, if any.
+ * - empty_message: The empty text to display if the View is empty.

+++ b/core/modules/views/tests/views_test_data/templates/views-view--frontpage.html.twigundefined
@@ -0,0 +1,77 @@
+ * Default theme implementation for main View template.
...
+ * - css_name: A CSS-safe version of the View name.
...
+ * - header: The View header.
+ * - footer: The View footer.
+ * - rows: The results of the View query, if any.
+ * - empty_message: The empty text to display if the View is empty.

+++ b/core/modules/views_ui/views_ui.moduleundefined
@@ -298,7 +307,7 @@ function views_ui_view_preview_section_display_category_links(ViewExecutable $vi
- * Returns all contextual links for the main content part of the view.
+ * Returns all contextual links for the main content part of the View.

@@ -318,7 +327,7 @@ function views_ui_view_preview_section_rows_links(ViewExecutable $view) {
-  // for editing and performing other contextual actions on the view).
+  // for editing and performing other contextual actions on the View).

These are still backwards, should be lowercase

+++ b/core/modules/views/views.theme.incundefined
@@ -13,7 +13,7 @@
  * @param $hook
...
  * @param $display

might as well put string and string|null in here as well

+++ b/core/modules/views/views.theme.incundefined
@@ -13,7 +13,7 @@
+ * @param Drupal\views\ViewExecutable $view

@param \Drupal\views\ViewExecutable $view

+++ b/core/modules/views/views.theme.incundefined
@@ -40,14 +40,21 @@ function _views_theme_functions($hook, ViewExecutable $view, $display = NULL) {
+ * @param array $variables

This is still $vars

+++ b/core/modules/views/views.theme.incundefined
@@ -40,14 +40,21 @@ function _views_theme_functions($hook, ViewExecutable $view, $display = NULL) {
+ *   - directory: The location of the Views module.

What is this directory? Why is it the location of the Views module?

Reverted empty_message back to empty because I tested it in twig proper and it seemed to work as a var, V to v and touchups from #65

nitpicks:

+ * - attributes.class: HTML classes that can be used to style contextually
+ *   through CSS, including:

unnecessary comma.

+ * - empty: The empty text to display if the view is has no rows.

"if the view is has no rows" - weird

+ * - more: A link to view more, if any.

view more what? if this is the views "more" link we could do better here to explain what it is. The "full" explanation from the Views UI is:

This will add a more link to the bottom of this view, which will link to the page view. If you have more than one page view, the link will point to the display specified in 'Link display' section under advanced. You can override the url at the link display setting.

I don't think we need all of that, but a bit more background info on what "more" means could be useful here - maybe explain that it's part of the pagination functionality and that it links to the page display for this view. What does "if any" mean here? how could I have "any view mores"?

+ * - feed_icon: Feed icon to display, if any.

the "if any" thing is awkward here too. "if any" really only makes sense if you're talking about a list of things that might be empty, not a singular thing that may or may not exist.

+ * @ingroup views_templates

@ingroup themeable

+ {{ title_prefix }}
+ {{ title }}
+ {{ title_suffix }}
+ {{ attachment_before }}
+ {{ attachment_after }}

undocumented variables in the Twig templates.

Tagging for profiling.

My take on the doc cleanup from #68 and #69

There's still a few undocumented variables in the Twig template as per #69.

#73: twig-views-view-1843744-73.patch queued for re-testing.

code looks good. I'm going to re-tag for manual testing because we had a merge conflict and changed some functions around since #32

i'll test this.

HEAD:

 <div  class="view view-frontpage view-id-frontpage view-display-id-page_1 view-dom-id-355cd322c0354cb1f5be81f65757128f2edadf8c1a56e47c2e3468c46835250a">
 <div class="view-content">
 <div  class="views-row views-row-1 views-row-odd views-row-first views-row-last">
 <article id="node-1" class="node node-article promoted view-mode-teaser contextual-region clearfix" role="article" data-edit-entity="node/1" about="/d8html/node/1" typeof="sioc:Item foaf:Document" role="article">
 <header>
 <h2 property="dc:title" datatype="">
 <a href="/d8html/node/1">asdfasdf</a>
 </h2>
 <ul class="contextual-links">
 <li class="node-edit odd first">
 <a href="/d8html/node/1/edit?destination=node">Edit</a>
 </li>
 <li class="node-delete even last">
 <a href="/d8html/node/1/delete?destination=node">Delete</a>
 </li>
 </ul>
 <div class="meta submitted">
 <article class="profile" class="user" typeof="sioc:UserAccount" about="/d8html/user/1">
 </article>
 <span property="dc:date dc:created" content="2013-05-14T19:52:55+10:00" datatype="xsd:dateTime" rel="sioc:has_creator">Submitted by <a href="/d8html/user/1" rel="author" title="View user profile." class="username" lang="" about="/d8html/user/1" typeof="sioc:UserAccount" property="foaf:name" datatype="">admin</a> on Tue, 05/14/2013 - 19:52</span>
 </div>
 </header>
 <div class="content clearfix">
 <div class="field field-name-body field-type-text-with-summary field-label-hidden" data-edit-id="node/1/body/und/teaser">
 <div class="field-items">
 <div class="field-item even" property="content:encoded">
 <p>asdf</p>
 </div>
 </div>
 </div>
 </div>
 <footer class="link-wrapper">
 <ul class="links inline">
 <li class="node-readmore odd first last">
 <a href="/d8html/node/1" rel="tag" title="asdfasdf">Read more<span class="element-invisible"> about asdfasdf</span>
 </a>
 </li>
 </ul>
 </footer>
 </article>
 </div>
 </div>
 </div>

#73:

<div class="view view-frontpage view-id-frontpage view-display-id-page_1 view-dom-id-a32457e56e7301cde8785f9831e530d09a782bcd69d5960b8c5552383c946ddf">
<div class="view-content">
<div  class="views-row views-row-1 views-row-odd views-row-first views-row-last">
<article id="node-1" class="node node-article promoted view-mode-teaser contextual-region clearfix" role="article" data-edit-entity="node/1" about="/d8html/node/1" typeof="sioc:Item foaf:Document" role="article">
<header>
<h2 property="dc:title" datatype="">
<a href="/d8html/node/1">asdfasdf</a>
</h2>
<ul class="contextual-links">
<li class="node-edit odd first">
<a href="/d8html/node/1/edit?destination=node">Edit</a>
</li>
<li class="node-delete even last">
<a href="/d8html/node/1/delete?destination=node">Delete</a>
</li>
</ul>
<div class="meta submitted">
<article class="profile" class="user" typeof="sioc:UserAccount" about="/d8html/user/1">
</article>
<span property="dc:date dc:created" content="2013-05-14T19:52:55+10:00" datatype="xsd:dateTime" rel="sioc:has_creator">Submitted by <a href="/d8html/user/1" rel="author" title="View user profile." class="username" lang="" about="/d8html/user/1" typeof="sioc:UserAccount" property="foaf:name" datatype="">admin</a> on Tue, 05/14/2013 - 19:52</span>
</div>
</header>
<div class="content clearfix">
<div class="field field-name-body field-type-text-with-summary field-label-hidden" data-edit-id="node/1/body/und/teaser">
<div class="field-items">
<div class="field-item even" property="content:encoded">
<p>asdf</p>
</div>
</div>
</div>
</div>
<footer class="link-wrapper">
<ul class="links inline">
<li class="node-readmore odd first last">
<a href="/d8html/node/1" rel="tag" title="asdfasdf">Read more<span class="element-invisible"> about asdfasdf</span>
</a>
</li>
</ul>
</footer>
</article>
</div>
</div>
</div>

Diff:

--- /Users/thedavidmeister/tmp/diff/original-mod 
+++ /Users/thedavidmeister/tmp/diff/new-mod 
@@ -1,4 +1,4 @@
-<div  class="view view-frontpage view-id-frontpage view-display-id-page_1 view-dom-id-355cd322c0354cb1f5be81f65757128f2edadf8c1a56e47c2e3468c46835250a">
+<div class="view view-frontpage view-id-frontpage view-display-id-page_1 view-dom-id-a32457e56e7301cde8785f9831e530d09a782bcd69d5960b8c5552383c946ddf">
 <div class="view-content">
 <div  class="views-row views-row-1 views-row-odd views-row-first views-row-last">
 <article id="node-1" class="node node-article promoted view-mode-teaser contextual-region clearfix" role="article" data-edit-entity="node/1" about="/d8html/node/1" typeof="sioc:Item foaf:Document" role="article">

Interestingly... the view-dom-id-HASH changes on every page load with and without the template. That seems bad...

Updated issue summary.

Profiled with a views block containing one article on the front page:

=== 8.x..8.x compared (519afeb71bb40..519aff4d961e6):

ct  : 37,111|37,111|0|0.0%
wt  : 206,932|206,875|-57|-0.0%
cpu : 204,012|200,013|-3,999|-2.0%
mu  : 12,738,208|12,738,208|0|0.0%
pmu : 12,852,520|12,852,520|0|0.0%

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

=== 8.x..1843744-views-view compared (519afeb71bb40..519b001c90026):

ct  : 37,111|37,180|69|0.2%
wt  : 206,932|207,854|922|0.4%
cpu : 204,012|204,012|0|0.0%
mu  : 12,738,208|12,786,176|47,968|0.4%
pmu : 12,852,520|12,900,560|48,040|0.4%

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

+++ b/core/modules/views/views.theme.incundefined
@@ -11,11 +11,11 @@
+ * @param Drupal\views\ViewExecutable $view

Just to be nitpicky ... should be @param \Drupal\views...

Profiled with one views block listing one basic page on the front page.

=== 8.x..8.x compared (519b0b3c13638..519b1195ac799):

ct  : 52,887|52,887|0|0.0%
wt  : 507,889|508,062|173|0.0%
cpu : 493,368|493,789|421|0.1%
mu  : 61,060,312|61,060,312|0|0.0%
pmu : 61,155,976|61,155,976|0|0.0%

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

=== 8.x..twig-views-view-1843744-73.patch compared (519b0b3c13638..519b11e0177db):

ct  : 52,887|52,984|97|0.2%
wt  : 507,889|508,250|361|0.1%
cpu : 493,368|494,288|920|0.2%
mu  : 61,060,312|61,144,544|84,232|0.1%
pmu : 61,155,976|61,241,216|85,240|0.1%

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

Just needs work on #82. Profiling looks good :)

Re-rolled for #82

Sorry, needs more doc tweaks, profiling looks great though. Can someone please roll a new patch and interdiff with the following changes:

+++ b/core/modules/views/templates/views-view.html.twigundefined
@@ -0,0 +1,89 @@
+ * - attributes: Remaining HTML attributes for the element.
+ *   - class: HTML classes that can be used to style contextually

Per http://drupal.org/node/1354#lists when starting a sub-list we need to end the line before in a colon, e.g. "Remaining HTML attributes for the element, including:"

+++ b/core/modules/views/tests/views_test_data/templates/views-view--frontpage.html.twigundefined
@@ -0,0 +1,77 @@
+ * Available variables:
+ * - attributes: Remaining HTML attributes for the element.
+ * - attributes.class: HTML classes that can be used to style contextually
+ *   through CSS, including:
+ *   - view
+ *   - view-[css_name]
+ *   - view-id-[view_name]
+ *   - view-display-id-[display_name]
+ *   - view-dom-id-[dom_id]
+ * - css_name: A CSS-safe version of the view name.
+ * - css_class: The user-specified class names, if any.
+ * - header: The view header.
+ * - footer: The view footer.
+ * - rows: The results of the view query, if any.
+ * - empty: The empty text to display if the view is empty.
+ * - pager: The pager next/prev links to display, if any.
+ * - exposed: Exposed widget form/info to display.
+ * - feed_icon: Feed icon to display, if any.
+ * - more: A link to view more, if any.

I think this whole section of the docblock in views-view--frontpage.html.twig should be replaced with the one in views-view.html.twig assuming the frontpage one doesn't have any more variables documented than the default views-view one.

+++ b/core/modules/views_ui/views_ui.moduleundefined
@@ -220,7 +220,15 @@ function views_ui_library_info() {
- * Theme preprocess for views-view.tpl.php.
+ * Implements hook_preprocess_views_view().
+ *
+ * Prepares variables for container templates.
+ *
+ * Default template: views-view.html.twig.
+ *
+ * @param array $vars
+ *   An associative array containing:
+ *   - view: The view object.

We discussed in #1913208: [policy] Standardize template preprocess function documentation (starting around comment #37) that this should probably just be:

Implements hook_preprocess_HOOK() for views-view.html.twig.

We don't need to re-document the vars here. Just the one line should be good.

So let's reroll this!

#88: twig-views-view-1843744-88.patch queued for re-testing.

Thanks @Hydra, you rock!

+++ b/core/modules/views/templates/views-view.html.twigundefined
@@ -30,7 +30,7 @@
- * @ingroup themeable
+ * @ingroup views_templates

This should be @ingroup themeable.

It also looks like we lost css_name and css_class docs, but they still look to be added in preprocess, can we add those back please (to both templates):

+++ b/core/modules/views/tests/views_test_data/templates/views-view--frontpage.html.twigundefined
@@ -1,27 +1,34 @@
- * - css_name: A CSS-safe version of the view name.
- * - css_class: The user-specified class names, if any.

... it's you rocking here, I'm just your patching slave! You find those tweaks like a machine, I'm admireing that!

Just a couple more doc nitpicks.

+++ b/core/modules/views/templates/views-view.html.twig
@@ -0,0 +1,91 @@
+ *     - view-display-id-[display_name]
+ *     - view-dom-id-[dom_id]
+ * - $css_name: A css-safe version of the view name.
+ * - $css_class: The user-specified classes names, if any.

Can you remove the $'s from the twig docblock?

+++ b/core/modules/views/templates/views-view.html.twig
@@ -0,0 +1,91 @@
+ * - attachment_before: An optional attachment view to be displayed before the
+ *   view content.
+ * - attachment_after: An optional attachment view to be displayed after the
+ *   view content.
+ *
+ * @ingroup themeable

Please add the @see template_preprocess() and @see template_preprocess_views_view() to the twig docblock.

Oh gosh, I have defenetly go to bed, this is embrassing, sry for that -.-" Thx for the quick review :)

#95: twig-views-view-1843744-95.patch queued for re-testing.

+++ b/core/modules/views/tests/views_test_data/templates/views-view--frontpage.html.twigundefined
@@ -0,0 +1,89 @@
+ * @ingroup views_templates

I think I missed this one before, again this should be @ingroup themeable.

Then ready :)

Thanks!

:)

Found one more thing:

+++ b/core/modules/views/templates/views-view.html.twigundefined
@@ -0,0 +1,94 @@
+ * - title: Title of the view, only used when displaying in the admin preview.
+ *   title_prefix: Additional output populated by modules, intended to be
+ *   displayed in front of the view title.
+ *   title_prefix: Additional output populated by modules, intended to be
+ *   displayed after the view title.
+ * - attachment_before: An optional attachment view to be displayed before the

title_prefix is here twice (second should be called title_suffix) and both variables are missing a '-' bullet point.

Please fix in both .html.twig template files :)

Next round...

profiling it!

Does not need further profiling, good profiling results in #83 and only changes since then have been documentation.

Thanks everyone!

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

Just adding back title_suffix var docs, part of the megapatch at #1987510: [meta] Convert all core *.tpl.php templates to Twig as singular patch.

Needs a re-roll

Working on it!

I'll review 108.

I can confirm that the patch from #108 applies cleanly and works as described.

RTBC!

And also green now! Thanks a million @socketwench and @tlattimore (and everyone who worked on this issue) :)

Committed 3cdae05 and pushed to 8.x. Thanks!