Problem/Motivation

Out of the box Drupal provides theme hook suggestions of the form node--type and node--%nid for node templates.
It would be great if it also provided templates of the form node--view-mode and node--type--view-mode so you can customize the teaser and search results display more readily.

Proposed resolution

Add additional template suggestions during template_preprocess_node if the view mode variable is available.

Remaining tasks

Update documentation so that themers are aware that this is available to them by default.

User interface changes

None.

API changes

None, only needs changes to themer's notes.

Related

From contrib land: #1300818: Template suggestions for node.tpl.php are effective only after specifying content type. there is a need for this.

Files: 
CommentFileSizeAuthor
#46 node_suggestions_view_mode-1503464-38-reroll-46.patch6.36 KBjoelpittet
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,581 pass(es). View
#38 interdiff.txt3.61 KBplopesc
#38 node_suggestions_view_mode-1503464-38.patch6.39 KBplopesc
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch node_suggestions_view_mode-1503464-38.patch. Unable to apply patch. See the log in the details link for more information. View
#36 interdiff.txt1.03 KBplopesc
#36 node_suggestions_view_mode-1503464-36.patch2.77 KBplopesc
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,806 pass(es), 3 fail(s), and 0 exception(s). View
#33 node_suggestions_view_mode-1503464-33.patch2.74 KBplopesc
PASSED: [[SimpleTest]]: [MySQL] 59,674 pass(es). View
#19 node_suggestions_view_mode-1503464-19.patch608 bytesplopesc
PASSED: [[SimpleTest]]: [MySQL] 59,639 pass(es). View
#17 Theme-Hook-Suggestions-View-Mode-D7-backport-1503464-17.patch848 bytessvenaas
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch Theme-Hook-Suggestions-View-Mode-D7-backport-1503464-17.patch. Unable to apply patch. See the log in the details link for more information. View
#10 Theme-Hook-Suggestions-View-Mode-1503464-10.patch2.66 KBlarowlan
FAILED: [[SimpleTest]]: [MySQL] 42,816 pass(es), 1 fail(s), and 0 exception(s). View
#5 Theme-Hook-Suggestions-View-Mode-1503464-4.patch2.61 KBlarowlan
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch Theme-Hook-Suggestions-View-Mode-1503464-4_0.patch. Unable to apply patch. See the log in the details link for more information. View
#4 Theme-Hook-Suggestions-View-Mode-1503464-4.patch2.61 KBlarowlan
PASSED: [[SimpleTest]]: [MySQL] 36,576 pass(es). View
#1 Theme-Hook-Suggestions-View-Mode-1503464-1.patch708 byteslarowlan
PASSED: [[SimpleTest]]: [MySQL] 35,717 pass(es). View

Comments

larowlan’s picture

Status: Active » Needs review
FileSize
708 bytes
PASSED: [[SimpleTest]]: [MySQL] 35,717 pass(es). View

Here's the patch

jacobroufa’s picture

Status: Needs review » Reviewed & tested by the community

Works great for me! Simple and small; worth including in core imho. Thanks larowlan!

chx’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

The needs backport is a bit tentative, the needs tests is affirmative. Also, how do we know the view mode only contains underscores and letters? Is there such assumption elsewhere?

And, thanks for working on this, it is very useful.

larowlan’s picture

Status: Needs work » Needs review
FileSize
2.61 KB
PASSED: [[SimpleTest]]: [MySQL] 36,576 pass(es). View

Patch adds cleansing of hyphens to underscores in build modes to ensure theme suggestions work as intened.
Patch adds tests of suggestions.
Couldn't get the test to run locally, so lets see what the testbot says.

larowlan’s picture

FileSize
2.61 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch Theme-Hook-Suggestions-View-Mode-1503464-4_0.patch. Unable to apply patch. See the log in the details link for more information. View

Last one postponed because of branch issues. D8 looks clean again (except for code review), trying again.

tstoeckler’s picture

I think the output of strstr() should be put in a variable, so that we don't have to call it twice.
With array_unshift() and array_push() the two ifs could be merged into one, but I don't know if that's preferred or not.

cweagans’s picture

tim.plunkett’s picture

Status: Needs review » Needs work
Issue tags: +Needs tests, +needs backport to D7

The last submitted patch, Theme-Hook-Suggestions-View-Mode-1503464-4.patch, failed testing.

larowlan’s picture

Status: Needs work » Needs review
FileSize
2.66 KB
FAILED: [[SimpleTest]]: [MySQL] 42,816 pass(es), 1 fail(s), and 0 exception(s). View

New patch after PSR-0 added.
Also fixes comments at #6.

larowlan’s picture

Issue tags: -Needs tests

removing tests tag

andypost’s picture

Status: Needs review » Needs work
Issue tags: +needs backport to D7

The last submitted patch, Theme-Hook-Suggestions-View-Mode-1503464-10.patch, failed testing.

andypost’s picture

Suppose this should be done for all entities, related #1043198: Convert view modes to ConfigEntity

svenaas’s picture

I needed this in D7 and have backported the patch locally, but I have two questions about standard practices before I proceed:

  1. Should a D7 backport patch get submitted here or on a D7 core issue?
  2. What are the expectations regarding associated tests on D7? I want to follow standard practices here, but the existing tests in D7's module/node/tests do not cover theme hook suggestions.
andypost’s picture

@svenaas You can file a patch here for D7. But this needs to be fixed in D8 first

svenaas’s picture

FileSize
848 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch Theme-Hook-Suggestions-View-Mode-D7-backport-1503464-17.patch. Unable to apply patch. See the log in the details link for more information. View

The attached patch adds the theme hook suggestions to the current dev version of Drupal 7.

svenaas’s picture

The patch I posted in #17 still applies for me against 7.20.

plopesc’s picture

Issue summary: View changes
Status: Needs work » Needs review
FileSize
608 bytes
PASSED: [[SimpleTest]]: [MySQL] 59,639 pass(es). View

Re-rolling this patch. IMHO, this is a really useful feature.

Regards.

plopesc’s picture

Status: Needs review » Needs work

This patch needs work, I also forgot to include the tests.
Sorry for the noise.

plopesc’s picture

Status: Needs work » Needs review
FileSize
2.8 KB
PASSED: [[SimpleTest]]: [MySQL] 59,786 pass(es). View

Attaching new patch, including tests and view_mode name sanitization.

Regards.

larowlan’s picture

  1. +++ b/core/modules/node/lib/Drupal/node/Tests/NodeTemplateSuggestionsTest.php
    @@ -0,0 +1,47 @@
    +}
    

    Missing newline

  2. +++ b/core/modules/node/lib/Drupal/node/Tests/NodeTemplateSuggestionsTest.php
    @@ -0,0 +1,47 @@
    +    $this->assertEqual($suggestions, array('node__full', 'node__page', 'node__page__full', 'node__' . $node->id(), 'node__' . $node->id() . '__full'), t('Found expected node suggestions.'));
    

    Shouldn't use t() in assert messages, see the 'assert t?' factoid on irc -same for later assert too.

Other than that, RTBC

plopesc’s picture

FileSize
2.77 KB
PASSED: [[SimpleTest]]: [MySQL] 60,121 pass(es). View
1.71 KB

Addressing suggestions ins #22.

Thanks for your review.

Regards

jibran’s picture

  1. +++ b/core/modules/node/lib/Drupal/node/Tests/NodeTemplateSuggestionsTest.php
    @@ -0,0 +1,47 @@
    +   * Test if template_preprocess_node() handles the suggestions right.
    

    Tests

  2. +++ b/core/modules/node/lib/Drupal/node/Tests/NodeTemplateSuggestionsTest.php
    @@ -0,0 +1,47 @@
    +  }
    +}
    

    There should be a blank line before the closing bracket of the class.

Other then that RTBC.

larowlan’s picture

FileSize
973 bytes
2.78 KB
PASSED: [[SimpleTest]]: [MySQL] 60,088 pass(es). View

Fixes issues identified at #24

jibran’s picture

Status: Needs review » Reviewed & tested by the community

Thank you for the fixes.

swentel’s picture

  1. +++ b/core/modules/node/lib/Drupal/node/Tests/NodeTemplateSuggestionsTest.php
    @@ -0,0 +1,48 @@
    +    //Change the view_mode.
    

    Nitpick: I'd write this as '// Change the view mode'. No need to use a machine name here.

  2. +++ b/core/modules/node/node.module
    @@ -639,9 +639,13 @@ function node_preprocess_block(&$variables) {
    +  $sanitized_view_mode = strtolower(preg_replace('/[^a-zA-Z0-9]+/', '_', $variables['elements']['#view_mode']));
    

    I'm not a big fan of this preg_replace here. Why don't we simply restrict the machine when we are creating the view mode in the UI. The UI is not even consistent anyway as it says that dots are not allowed, but it actually does, so I'd call that a bug ... and the strtolower shouldn't be needed either.

    Granted, someone can created this programmatically, but there are ways to catch violations against that.

plopesc’s picture

It was because of #3, but assuming that we will give support only for view_modes created through the UI or programmatically following that name constraints, we could convert preg_replace into strtr($view_mode, '.', '_').

What do you think?

catch’s picture

Status: Reviewed & tested by the community » Needs work

I'd rather fix the UI as swentel suggests, looks like a proper bug.

plopesc’s picture

@catch IMHO, the bug in the View modes creation page is out of the scope of this issue.

If @swentel is OK with the proposal in #29, I could create a new patch for this issue.

I think we should open a new issue, if it does not exist related to dots in view modes machine name.

Regards.

tstoeckler’s picture

Right let's just strip any sanitation out of this patch and fix the view mode UI in a follow-up.

plopesc’s picture

Status: Needs work » Needs review
FileSize
2.74 KB
PASSED: [[SimpleTest]]: [MySQL] 59,674 pass(es). View
2.09 KB

New patch

plopesc’s picture

Cottser’s picture

Status: Needs review » Needs work
Issue tags: +Twig
Related issues: +#2228479: Add template suggestions for node.html.twig based on the view mode

Oops, just created a duplicate of this issue. Patch looks great to me, just a couple minor things before I can RTBC.

  • +++ b/core/modules/node/lib/Drupal/node/Tests/NodeTemplateSuggestionsTest.php
    @@ -0,0 +1,48 @@
    +class NodeTemplateSuggestionsTest extends NodeTestBase {
    +  public static function getInfo() {
    

    Need a blank line inside the class declaration.

  • +++ b/core/modules/node/lib/Drupal/node/Tests/NodeTemplateSuggestionsTest.php
    @@ -0,0 +1,48 @@
    +    //Change the view_mode.
    

    This needs a space after the '//'.

  • Before:

    <!-- THEME DEBUG -->
    <!-- CALL: _theme('node') -->
    <!-- FILE NAME SUGGESTIONS:
       * node--1.html.twig
       * node--article.html.twig
       x node.html.twig
    -->
    <!-- BEGIN OUTPUT from 'core/themes/bartik/templates/node.html.twig' -->

    After:

    <!-- THEME DEBUG -->
    <!-- CALL: _theme('node') -->
    <!-- FILE NAME SUGGESTIONS:
       * node--1--full.html.twig
       * node--1.html.twig
       * node--article--full.html.twig
       * node--article.html.twig
       * node--full.html.twig
       x node.html.twig
    -->
    <!-- BEGIN OUTPUT from 'core/themes/bartik/templates/node.html.twig' -->
      plopesc’s picture

      Status: Needs work » Needs review
      FileSize
      2.77 KB
      FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 66,806 pass(es), 3 fail(s), and 0 exception(s). View
      1.03 KB

      Thank you for reviewing it Cottser ;)

      It looks better now?

      Regards

      Status: Needs review » Needs work

      The last submitted patch, 36: node_suggestions_view_mode-1503464-36.patch, failed testing.

      plopesc’s picture

      Status: Needs work » Needs review
      FileSize
      6.39 KB
      FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch node_suggestions_view_mode-1503464-38.patch. Unable to apply patch. See the log in the details link for more information. View
      3.61 KB

      Ups, I was not aware of the existence of TwigDebugMarkupTest...

      Fixing that test.

      Cottser’s picture

      Status: Needs review » Reviewed & tested by the community

      Looks great to me, thanks!

      sun’s picture

      Do we have a follow-up issue already to remove this code from Node module + make it generic for all entities?

      IMO, we should simply have: {entity_type}__{bundle/id}__{viewmode}

      → Consistency for themers.

      → Less work for developers.

      → Double-DX-win!++

      plopesc’s picture

      @sun totally agree.
      AFAIK, there is not currently a 'entity' hook available to work with in hook_theme_suggestions_HOOK.
      So we could try to create it or try to detect in _theme() function if the current $base_theme_hook belongs to an entity, could be something like in_array($base_theme_hook, array_keys(Drupal::entityManager()->getDefinitions())) and proceed to add the suggestions in that case.

      Should we open a new follow up once this will be committed or just start to work on it here?

      Regards.

      alexpott’s picture

      Status: Reviewed & tested by the community » Needs work
      Issue tags: +Needs change record

      We need a change record for this - patch looks good though!

      plopesc’s picture

      Status: Needs work » Reviewed & tested by the community
      Issue tags: -Needs change record

      Created change record https://drupal.org/node/2250487 and patch still applying.
      Related tests pass locally, so marking as RTBC again.

      joelpittet’s picture

      @plopesc totally, could you open follow up for entities as you mentioned in #41.

      RTBC++ on this issue:)

      Status: Reviewed & tested by the community » Needs work

      The last submitted patch, 38: node_suggestions_view_mode-1503464-38.patch, failed testing.

      joelpittet’s picture

      Status: Needs work » Reviewed & tested by the community
      FileSize
      6.36 KB
      PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 71,581 pass(es). View

      Rerolled, back to RTBC

      alexpott’s picture

      Status: Reviewed & tested by the community » Fixed

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

      • Commit 3a699a6 on 8.x by alexpott:
        Issue #1503464 by plopesc, larowlan, joelpittet, svenaas: Automatically...
      plopesc’s picture

      Status: Fixed » Closed (fixed)

      Automatically closed - issue fixed for 2 weeks with no activity.