@berdir, @sgabe and me have been working in this functionality for mimemail and simplenews.
- #1349728: Possibility to configure the theme that will render the email
- #374222: Template files do not work when using admin theme
We all agree that this functionality will reside in a common mail module and as long as mimemail has already mailsystem as a dependency here is the patch to add this functionality.
It consist in altering the registry of every non-mail-theme to render using the mail theme configured.
We have been thinking of other aproaches, but finally decided to this one, any other points of view are welcome.
Any submodule that want to declare a theme registry to be rendered with the mail theme should add the key mail theme to its definition in hook_theme:
function hook_theme() {
$theme['example_message'] = array(
'variables' => array('key' => NULL, 'recipient' => NULL, 'subject' => NULL, 'body' => NULL),
'template' => 'example-message',
'pattern' => 'example_message__',
'mail theme' => TRUE,
);
return $theme;
}
Comment | File | Size | Author |
---|---|---|---|
#4 | 1382036-3.patch | 6.79 KB | jherencia |
#2 | 1382036-2.patch | 6.16 KB | jherencia |
#1 | 1382036.patch | 2.84 KB | jherencia |
Comments
Comment #1
jherencia CreditAttribution: jherencia commentedComment #2
jherencia CreditAttribution: jherencia commentedI forgot to add a file, thanks @Berdir for the warning ;).
Comment #3
BerdirYou will need to add an inner loop here and also look for everything that goes like $name . '__' to catch theme_hook_suggestions. See my patch.
you are using 'domain' for the options and 'domain_theme' here...
Comment #4
jherencia CreditAttribution: jherencia commentedThanks @Berdir, I added the changes you suggested.
Please see the refactor I made, maybe you would like to do it in simplenews too.
Comment #5
Schnitzel CreditAttribution: Schnitzel commented+1 for patch at #4, used it together with http://drupal.org/node/374222#comment-5399034 and worked perfectly.
Comment #6
miro_dietikerDid a code review, looks fine. We want to build Simplenews on this feature.
Would be great to see this feature in Mailsystem asap. :-)
Comment #7
sgabe CreditAttribution: sgabe commentedSecond that. We would also like to use this in Mime Mail.
Comment #8
rooby CreditAttribution: rooby commentedWorks for me along with the simplenews patch.
Seeing as mimemail also implements a similar option at its level, maybe an issue should also be opened there to change to using this setting?
Comment #9
BerdirThat issue exists: #1349728: Possibility to configure the theme that will render the email
Comment #10
pillarsdotnet CreditAttribution: pillarsdotnet commentedSorry for being offline so long. Will apply outstanding patches later today if possible.
Comment #11
miro_dietikerThat's cool man :-) Looking forward to hear success messages .-)
Comment #12
s_leu CreditAttribution: s_leu commented@pillarsdotnet
Did you already apply the patch?
Comment #13
miro_dietikerThis is blocking us from releasing beta in simplenews...
I'll open a ticket to add me as a co-maintainer for Mail System since this dependency is needed to make HTML rendering work reliably.
Comment #14
miro_dietikerReferring to my application for maintainer state for project Mail System
#1412592: Co-Maintaining project Mailsystem
Would be great to see this confirmed.
Comment #15
pillarsdotnet CreditAttribution: pillarsdotnet commentedSorry for being so unresponsive. I have added miro_dietiker as a maintainer.
Comment #16
pillarsdotnet CreditAttribution: pillarsdotnet commentedReviewing/applying the patch now.
Comment #17
pillarsdotnet CreditAttribution: pillarsdotnet commentedRemoving the chunk dealing with Domain Theme because that module is marked as abandoned/obsolete.Nevermind -- I see that domain_theme is a submodule of domain
Seems like a poor naming choice. Doesn't this somehow clash with the theme hook of the Domain module?
Comment #18
pillarsdotnet CreditAttribution: pillarsdotnet commentedAlso seems like this will make it difficult to set the theme on a per-email basis.
Comment #19
BerdirI don't think doing that is possible in any way.
Once a theme is loaded it's there. You can't switch. It's just plain impossible. theme() has a static cache for this and doesn't use drupal_static(). See http://api.drupal.org/api/drupal/includes--theme.inc/function/theme/7
That's why we have to inject it into the theme registry like this in the first place :) No other approach works.
Comment #20
pillarsdotnet CreditAttribution: pillarsdotnet commentedOkay; I've made cosmetic changes, applied the patch and pushed to the 7.x-2.x branch.
Setting to "needs work" because now I need to generate 6.x and 8.x versions. Will roll new releases in all three branches after testing.
Comment #21
pillarsdotnet CreditAttribution: pillarsdotnet commented@#19
That's why I wrote the Echo module.
Comment #22
pillarsdotnet CreditAttribution: pillarsdotnet commentedOkay, what is the D6 way of doing what the D7
hook_theme_registry_alter()
does?Comment #23
pillarsdotnet CreditAttribution: pillarsdotnet commentedPushed updates to 6.x-2.x and 8.x-2.x branches, but haven't tested yet. The 6.x version will certainly fail because the necessary hooks don't exist.
Comment #24
sammys CreditAttribution: sammys commented@pillarsdotnet: D6 hook_theme_registry_alter
Comment #25
pillarsdotnet CreditAttribution: pillarsdotnet commented@sammys -- I could have sworn the D6 hook didn't show up the last time I looked. Thanks.
Comment #26
pillarsdotnet CreditAttribution: pillarsdotnet commentedOkay, pushed new releases as follows:
Documentation and screenshots need updating.
Comment #26.0
pillarsdotnet CreditAttribution: pillarsdotnet commentedAdded hook_theme definition example.
Comment #28
Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer and at Appnovation for Pfizer, Inc. commentedThe 6.x branch is no longer supported.