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

  1. Install / enable group, group_notity.
  2. 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

  1. Decide the right course of action.
  2. Implement.
  3. Reviews / refinements.
  4. 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

CommentFileSizeAuthor
#4 3165399.3_4.interdiff.txt1.58 KBdww
#4 3165399-4.patch896 bytesdww
#3 3165399-3.patch2.54 KBdww

Comments

dww created an issue. See original summary.

dww’s picture

Status: Active » Needs review

I 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

dww’s picture

StatusFileSize
new2.54 KB

Uploading the patch would help. 😉

dww’s picture

Title: Consider ripping out twig template overrides » hook_theme_suggestions_HOOK_alter() prevents themes from overriding twig templates for the group_notify_email view mode
Category: Task » Bug report
Issue summary: View changes
StatusFileSize
new896 bytes
new1.58 KB

Okay, 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:

  • I re-tested and confirmed that if the theme is doing nothing, the module templates are still picked up with this patch attached.
  • But if the theme defines its own version of node--group-notify-email.html.twig that gets used.
  • And of course, if a theme defines something more specific (e.g. node--[node-type]--group-notify-email.html.twig) that gets used.

  • gregcube committed 5577172 on 8.x-1.x authored by dww
    Issue #3165399 by dww: hook_theme_suggestions_HOOK_alter() prevents...
gregcube’s picture

Looks good to me, too. Thanks!

gregcube’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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