Problem/Motivation
I just sunk ~1 hour last night fighting with Twig templates for the emails being sent by group_notify. I've got all the twig debugging stuff enabled on the site. Although the active theme being used for the notifications has a node--private-audio-stream--group-notify-email.html.twig template, which is more specific and should be getting picked up, instead the module's own node--group-notify-email.html.twig keeps being used:
<!-- THEME DEBUG -->
<!-- THEME HOOK: 'node' -->
<!-- FILE NAME SUGGESTIONS:
x node--group-notify-email.html.twig
* node--1825--group-notify-email.html.twig
* node--1825.html.twig
* node--private-audio-stream--group-notify-email.html.twig
* node--private-audio-stream.html.twig
x node--group-notify-email.html.twig
* node.html.twig
-->
<!-- BEGIN OUTPUT from 'modules/contrib/group_notify/templates/node--group-notify-email.html.twig' -->
If I disable group_notify_theme_suggestions_node_alter(), it works as expected:
<!-- THEME DEBUG -->
<!-- THEME HOOK: 'node' -->
<!-- FILE NAME SUGGESTIONS:
* node--1825--group-notify-email.html.twig
* node--1825.html.twig
x node--private-audio-stream--group-notify-email.html.twig
* node--private-audio-stream.html.twig
* node--group-notify-email.html.twig
* node.html.twig
-->
<!-- BEGIN OUTPUT from 'themes/custom/.../templates/node/node--private-audio-stream--group-notify-email.html.twig' -->
Steps to reproduce
- Install / enable group, group_notity.
- Try to override the node--group-notify-email.twig.html file in a theme.
Proposed resolution
Stop implementing hook_theme_suggestions_HOOK_alter() since those are unnecessary for the templates we're providing to be picked up, and they prevent themes from overriding those templates if needed.
Consider ripping out all of the plumbing that's trying to mess with these node twig template theme suggestions. If folks need to customize these templates, they can rely on core's existing theme suggestions. We shouldn't try to mess with it, since the way we're doing it now prevents themes from customizing these at all.
The downside is that we'd no longer be providing an example template that does the customizations in the templates provided by this module. Then again, I'm not sure we even need those. From what I can tell, we're just hard-coding to hide a few fields, which I'm not sure we should even be doing. As with #3165158: Stop hard-coding any content in the email, rely on the 'group_notify_email' view mode I think this module should stop being opinionated about these things, and let the view mode configuration control everything.
I'm not sure there's a good way for a module to provide a template for this without breaking themes. Google isn't coming up with anything good. I'll try asking around, too. But again, maybe we don't want to be providing these at all.
Remaining tasks
Decide the right course of action.Implement.- Reviews / refinements.
- Commit.
User interface changes
Theme template overrides for the group-notify-email view modes will start working.
API changes
N/A
Data model changes
N/A
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 3165399-4.patch | 896 bytes | dww |
Comments
Comment #2
dwwI think this patch, plus maybe some documentation, is the best course of action. ;) Less code for us to worry about / maintain. Makes theming normal again for this templates.
Thoughts?
Thanks!
-Derek
Comment #3
dwwUploading the patch would help. 😉
Comment #4
dwwOkay, cool. I got some insights from @larowlan in Slack. Turns out our
hook_theme()implementation is sufficient. That lets the module provide templates for node + comment for the new view mode.The bug is that our
hook_theme_suggestions_HOOK_alter()implementations prevent themes from overriding twig templates for the group_notify_email view mode. They're also totally unnecessary. Ripping those out seems to make everyone happy:node--group-notify-email.html.twigthat gets used.node--[node-type]--group-notify-email.html.twig) that gets used.Comment #6
gregcube commentedLooks good to me, too. Thanks!
Comment #7
gregcube commented