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.
Comment | File | Size | Author |
---|---|---|---|
#46 | node_suggestions_view_mode-1503464-38-reroll-46.patch | 6.36 KB | joelpittet |
#38 | interdiff.txt | 3.61 KB | plopesc |
#38 | node_suggestions_view_mode-1503464-38.patch | 6.39 KB | plopesc |
#36 | interdiff.txt | 1.03 KB | plopesc |
#36 | node_suggestions_view_mode-1503464-36.patch | 2.77 KB | plopesc |
Comments
Comment #1
larowlanHere's the patch
Comment #2
jacobroufa CreditAttribution: jacobroufa commentedWorks great for me! Simple and small; worth including in core imho. Thanks larowlan!
Comment #3
chx CreditAttribution: chx commentedThe 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.
Comment #4
larowlanPatch 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.
Comment #5
larowlanLast one postponed because of branch issues. D8 looks clean again (except for code review), trying again.
Comment #6
tstoecklerI 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.
Comment #7
cweagansFixing tags per http://drupal.org/node/1517250
Comment #8
tim.plunkett#5: Theme-Hook-Suggestions-View-Mode-1503464-4.patch queued for re-testing.
Comment #10
larowlanNew patch after PSR-0 added.
Also fixes comments at #6.
Comment #11
larowlanremoving tests tag
Comment #12
andypost#10: Theme-Hook-Suggestions-View-Mode-1503464-10.patch queued for re-testing.
Comment #14
andypostSuppose this should be done for all entities, related #1043198: Convert view modes to ConfigEntity
Comment #15
svenaas CreditAttribution: svenaas commentedI needed this in D7 and have backported the patch locally, but I have two questions about standard practices before I proceed:
Comment #16
andypost@svenaas You can file a patch here for D7. But this needs to be fixed in D8 first
Comment #17
svenaas CreditAttribution: svenaas commentedThe attached patch adds the theme hook suggestions to the current dev version of Drupal 7.
Comment #18
svenaas CreditAttribution: svenaas commentedThe patch I posted in #17 still applies for me against 7.20.
Comment #19
plopescRe-rolling this patch. IMHO, this is a really useful feature.
Regards.
Comment #20
plopescThis patch needs work, I also forgot to include the tests.
Sorry for the noise.
Comment #21
plopescAttaching new patch, including tests and view_mode name sanitization.
Regards.
Comment #22
larowlanMissing newline
Shouldn't use t() in assert messages, see the 'assert t?' factoid on irc -same for later assert too.
Other than that, RTBC
Comment #23
plopescAddressing suggestions ins #22.
Thanks for your review.
Regards
Comment #24
jibranTests
There should be a blank line before the closing bracket of the class.
Other then that RTBC.
Comment #25
larowlanFixes issues identified at #24
Comment #26
jibranThank you for the fixes.
Comment #28
swentel CreditAttribution: swentel commentedNitpick: I'd write this as '// Change the view mode'. No need to use a machine name here.
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.
Comment #29
plopescIt 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?
Comment #30
catchI'd rather fix the UI as swentel suggests, looks like a proper bug.
Comment #31
plopesc@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.
Comment #32
tstoecklerRight let's just strip any sanitation out of this patch and fix the view mode UI in a follow-up.
Comment #33
plopescNew patch
Comment #34
plopescCreated #2170289: Entity Display modes form allows to create machine names including dots related to Display modes UI
Comment #35
star-szrOops, just created a duplicate of this issue. Patch looks great to me, just a couple minor things before I can RTBC.
Need a blank line inside the class declaration.
This needs a space after the '//'.
Before:
After:
Comment #36
plopescThank you for reviewing it Cottser ;)
It looks better now?
Regards
Comment #38
plopescUps, I was not aware of the existence of
TwigDebugMarkupTest
...Fixing that test.
Comment #39
star-szrLooks great to me, thanks!
Comment #40
sunDo 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!++
Comment #41
plopesc@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 likein_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.
Comment #42
alexpottWe need a change record for this - patch looks good though!
Comment #43
plopescCreated change record https://drupal.org/node/2250487 and patch still applying.
Related tests pass locally, so marking as RTBC again.
Comment #44
joelpittet@plopesc totally, could you open follow up for entities as you mentioned in #41.
RTBC++ on this issue:)
Comment #46
joelpittetRerolled, back to RTBC
Comment #47
alexpottCommitted 3a699a6 and pushed to 8.x. Thanks!
Comment #49
plopescCreated follow-up issue #2270883: Automatically add theme hook suggestions for all entity types once this is in.