Automatically add theme hook suggestion for node view mode

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

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.

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.

Fixing tags per http://drupal.org/node/1517250

#5: Theme-Hook-Suggestions-View-Mode-1503464-4.patch queued for re-testing.

#10: Theme-Hook-Suggestions-View-Mode-1503464-10.patch queued for re-testing.

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

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.

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

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

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

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

Regards.

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

Attaching new patch, including tests and view_mode name sanitization.

Regards.

Addressing suggestions ins #22.

Thanks for your review.

Regards

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.

Thank you for the fixes.

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.

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?

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

@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.

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

New patch

Created #2170289: Entity Display modes form allows to create machine names including dots related to Display modes UI

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() {

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

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' -->

Thank you for reviewing it Cottser ;)

It looks better now?

Regards

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

Fixing that test.

Looks great to me, thanks!

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!++

@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.

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

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

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

RTBC++ on this issue:)

Rerolled, back to RTBC

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

Created follow-up issue #2270883: Automatically add theme hook suggestions for all entity types once this is in.