Problem/Motivation

We have two identical templates in core, views_view_summary.html.twig and item_list.html.twig. There's no reason for two. More code to maintain, and less consistent markup.

Proposed resolution

Remove views_view_summary.html.twig, replace usage with item_list.html.twig instead

Remaining tasks

update all usage of theme('views_view_summary') with theme('item_list__views_view_summary')
remove the views_view_summary template file
update the views_view_summary preprocess function to provide variables as expected by item-list.html.twig

User interface changes

None.

How to reproduce

Create a view with a contextual filter.
Check "Display a summary" when "When the filter value is NOT in the URL".

API changes

Removal of 'views_view_summary.html.twig + preprocess

#1813426: [meta] Consolidate all item list templates and add theme_hook_suggestions
#1939064: Convert theme_links() to Twig

Files: 
CommentFileSizeAuthor
#29 2032695-working-twig-suggestions.png48.92 KBakalata
#29 2032695-29-reroll-changes.txt1.12 KBakalata
#29 2032695-29.patch4.23 KBakalata
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,179 pass(es), 197 fail(s), and 2,612 exception(s). View
#27 2032695-27.patch5.82 KBpiyuesh23
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,045 pass(es). View
#21 2032695-21.patch4.18 KBlokapujya
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 95,174 pass(es). View
#17 remove-2032695-17.patch2.99 KBAjitS
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch remove-2032695-17.patch. Unable to apply patch. See the log in the details link for more information. View
#15 remove-2032695-15.patch1.74 KBAjitS
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,515 pass(es). View

Comments

jenlampton’s picture

Issue tags: +Template consolidation

tagging

dead_arm’s picture

Assigned: Unassigned » dead_arm

Prague sprint!

dead_arm’s picture

Assigned: dead_arm » Unassigned
Status: Active » Postponed

After looking at this, postponing on #1751194: Introduce hook_theme_suggestions[_HOOK]() and hook_theme_suggestions[_HOOK]_alter()

  • item list hasn't been converted yet, so item_list.html.twig isn't a file we have to use at this time.
  • The views-view-summary.html.twig template adds a class to the ul, "views-summary" that is used in views/ajax_view.js

Follow up:

There is also an unformatted list template "views-view-summary-unformatted.html.twig" that should probably be converted/considered.

mgifford’s picture

harshil.maradiya’s picture

FileSize
3.7 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 86,786 pass(es). View

changing name convention as per instruction in issue

harshil.maradiya’s picture

Status: Active » Needs review
dawehner’s picture

The patch at least does not match with the issue summary ... is that my intention?

lauriii’s picture

FileSize
2.97 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 86,839 pass(es). View
harshil.maradiya’s picture

Hi dawehnr,

I have created patch .as per the instructions only.

Remaining tasks

update all usage of theme('views_view_summary') with theme('item_list__views_view_summary')
remove the views_view_summary template file
remove the 'views_view_summary preprocess function

akalata’s picture

Status: Needs review » Needs work

Manually testing #8:

Error: [code]Template "core/modules/views/templates/item-list__views-view-summary.html.twig" is not defined (). [/code]

Also wondering your thoughts on removing/consolidating views-view-summary-unformatted.html.twig, mentioned in #3.

mitokens’s picture

mgifford’s picture

Issue tags: +Needs reroll

doesn't apply in SimplyTest.me any more.

AjitS’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
3.2 MB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,500 pass(es), 3 fail(s), and 0 exception(s). View

Rerolling

AjitS’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

Realised that the reroll was not correct.
Sorry about that! Giving it another try soon.

AjitS’s picture

Status: Needs work » Needs review
Issue tags: -Needs reroll
FileSize
1.74 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 94,515 pass(es). View

Rerolling again. Hope it is right this time.

The last submitted patch, 13: remove-2032695-13.patch, failed testing.

AjitS’s picture

FileSize
2.99 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch remove-2032695-17.patch. Unable to apply patch. See the log in the details link for more information. View

I missed to remove the template file while rerolling.

lokapujya’s picture

Assigned: Unassigned » lokapujya

lokapujya queued 17: remove-2032695-17.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 17: remove-2032695-17.patch, failed testing.

lokapujya’s picture

FileSize
4.18 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 95,174 pass(es). View

Re-rolled + removing respective .twig file from classy theme.

lokapujya’s picture

Assigned: lokapujya » Unassigned
Status: Needs work » Needs review
akalata’s picture

Status: Needs review » Needs work
Issue tags: +Needs reroll

We keep rerolling this without making sure that the patch is solving the issue.

Personally, I'm stuck because I can't figure out where views_view_summary is being declared as a theme function -- it isn't listed in views_theme(). Without that change, none of the changes in the attached patches will have an effect other than breaking the summary display completely.

jjcarrion’s picture

Assigned: Unassigned » jjcarrion

I'm going to work on this today.

jjcarrion’s picture

Issue summary: View changes
jjcarrion’s picture

@akalata the theme function is generated dynamically from the annotation on the DefaultSummary class. So we need just to make the reroll. Thanks for your help at the DrupalCon Barcelona!

piyuesh23’s picture

Status: Needs work » Needs review
FileSize
5.82 KB
PASSED: [[SimpleTest]]: [PHP 5.5 MySQL] 114,045 pass(es). View

Re-rolled the patch.

Uploading the updated patch here.

dawehner’s picture

  1. +++ b/core/modules/views/src/Plugin/views/style/DefaultSummary.php
    @@ -19,7 +19,7 @@
    diff --git a/core/modules/views/templates/views-view-summary.html.twig b/core/modules/views/templates/views-view-summary.html.twig
    
    diff --git a/core/modules/views/templates/views-view-summary.html.twig b/core/modules/views/templates/views-view-summary.html.twig
    index 3dde427..c2eaac6 100644
    
    index 3dde427..c2eaac6 100644
    --- a/core/modules/views/templates/views-view-summary.html.twig
    
    --- a/core/modules/views/templates/views-view-summary.html.twig
    +++ b/core/modules/views/templates/views-view-summary.html.twig
    
    +++ b/core/modules/views/templates/views-view-summary.html.twig
    +++ b/core/modules/views/templates/views-view-summary.html.twig
    @@ -20,7 +20,8 @@
    
    @@ -20,7 +20,8 @@
      * @ingroup themeable
    

    Do we really still need this particular file?

  2. +++ b/core/modules/views/views.module
    @@ -99,8 +106,8 @@ function views_theme($existing, $type, $theme, $path) {
    -    'variables' => array('tags' => array(), 'quantity' => 9, 'element' => 0, 'parameters' => array()),
    -  );
    +      'variables' => array('tags' => array(), 'quantity' => 9, 'element' => 0, 'parameters' => array()),
    +    );
     
    
    @@ -132,12 +139,12 @@ function views_theme($existing, $type, $theme, $path) {
    -    'variables' => array('view' => NULL, 'field' => NULL, 'row' => NULL),
    -    'function' => 'theme_views_view_field',
    -  );
    +      'variables' => array('view' => NULL, 'field' => NULL, 'row' => NULL),
    +      'function' => 'theme_views_view_field',
    +    );
       $hooks['views_view_grouping'] = $base + array(
    -    'variables' => array('view' => NULL, 'grouping' => NULL, 'grouping_level' => NULL, 'rows' => NULL, 'title' => NULL),
    -  );
    +      'variables' => array('view' => NULL, 'grouping' => NULL, 'grouping_level' => NULL, 'rows' => NULL, 'title' => NULL),
    +    );
     
       // Only display, pager, row, and style plugins can provide theme hooks.
       $plugin_types = [
    @@ -220,12 +227,12 @@ function views_theme($existing, $type, $theme, $path) {
    
    @@ -220,12 +227,12 @@ function views_theme($existing, $type, $theme, $path) {
       }
     
       $hooks['views_form_views_form'] = $base + array(
    -    'render element' => 'form',
    -  );
    +      'render element' => 'form',
    +    );
     
       $hooks['views_exposed_form'] = $base + array(
    -    'render element' => 'form',
    -  );
    +      'render element' => 'form',
    +    );
     
       return $hooks;
    
    @@ -246,9 +253,9 @@ function views_preprocess_node(&$variables) {
    -        && $variables['page']
    -        && $variables['view_mode'] == 'full'
    -        && !$variables['view']->display_handler->hasPath()) {
    +      && $variables['page']
    +      && $variables['view_mode'] == 'full'
    +      && !$variables['view']->display_handler->hasPath()) {
           $variables['page'] = FALSE;
    

    these changes are out of scope. We use 4 spaces instead of 2

akalata’s picture

Issue summary: View changes
Status: Needs review » Needs work
Issue tags: -Needs reroll
FileSize
4.23 KB
FAILED: [[SimpleTest]]: [PHP 5.5 MySQL] 113,179 pass(es), 197 fail(s), and 2,612 exception(s). View
1.12 KB
48.92 KB

#28.1 - No, the patch for this issue should include the removal of views-view-summary.html.twig.

@jjcarrion - thanks for pointing that out!

I'm attaching a reroll with a few tweaks, so that the item-list template finally shows up in the template suggestions with twig.debug enabled (see below).

However, as you can see by the screenshot of the HTML, nothing shows up -- the preprocess function needs to be updated to return the values that item-list.html.twig expects.

akalata’s picture

Assigned: jjcarrion » Unassigned
AjitS’s picture

Status: Needs work » Needs review

If there is a patch needing review, everyone should know :-)

Status: Needs review » Needs work

The last submitted patch, 29: 2032695-29.patch, failed testing.

jjcarrion’s picture

Sorry for not updating what I did... I was with the reroll, but nothing appear in the view. As @akalata said, we have to change some variables in the preprocess to make it work with the item_list twig template, otherwise, the name of the variables are not matching.. this is my guess.

I'll try to take a look during the weekend if there is no solution yet.

The last submitted patch, 29: 2032695-29.patch, failed testing.

akalata’s picture

@AjitS I didn't set to "needs review" because the patch is incomplete - we're still missing a major piece of functionality.

akalata queued 29: 2032695-29.patch for re-testing.

The last submitted patch, 29: 2032695-29.patch, failed testing.

andypost’s picture

Version: 8.0.x-dev » 9.x-dev
Status: Needs work » Postponed

views_view_summary.html.twig is part of stable theme

catch’s picture

Title: Remove views_view_summary.html.twig, call use item-list.html.twig instead » Use item-list.html.twig instead of views_view_summary.html.twig
Version: 9.x-dev » 8.2.x-dev

We can do the consolidation of usages + deprecation in a minor release, just can't do the actual removal.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.