Improve BP Emails templating

[imath]

So i had ​this idea of using the Site Icon as a logo inside the Email's header, as it's too late to talk about it for core, i've built this ​plugin and realized i had to add a new entry into the BuddyPress templates stack and to completely override the single-bp-email.php template just to add an image before the Site name. So it might be possible to improve the templating for this in a future release or for 2.5.0.

I've put this ticket on the 2.5.0 milestone because there's 2 things i think at least we really need to fix:

• A string is not translated : Hi {{recipient.name}},
• Some style is missing for the customizer preview in case we choose a 'refresh' transport for the custom setting.

Then ideally, i think we should have some filterable template tags for the email template.

[DJPaul]

Good catch with the hardcoded string. :)

i had to add a new entry into the BuddyPress templates stack

If you're trying to do this from inside a plugin, yes.

completely override the single-bp-email.php template just to add an image before the Site name

Yes, theme templates normally work like this. ;)

bp_email_the_site_name()

Are you doing this JUST to filter it?

bp_email_the_salutation() + bp_email_the_unsubsribe_mention()

Why should these be filters? The strings are/will be run through i18n functions, so someone can use the gettext filters to change them if they're really desperate (or just copy/paste the template into their theme).

body.wp-customizer-unloading stuff

This is another good find. What happens right now if this isn't present? How broken is it?
Also, I think if we find we need to add CSS to fix the display in the Customiser, this should be in a new stylesheet -- not inside the actual markup sent in the email.

[imath]

Yes, theme templates normally work like this. ;)
Are you doing this JUST to filter it?

1) Absolutely, imho it seems really a huge task to:

• register a new template dir
• make sure this template dir will only be added to the stack if the email template is required to avoid adding 3 new entries in the stack each time bp_get_template_part() is used
• do a copy paste of the single email template inside the plugin's template dir

*JUST* to add a logo before the site name, while a filter would avoid all this :)

2) All other template parts are using template tags including filters, so it's a bit weird to not have the same possibility for this particular template.

I agree, Themes can add their template, and plugins can also do that like i've described in point 1. So i really don't mind not having template tags there, although i see no good reason not to have them..

But: the untranslated string should be fixed.

What happens right now if this isn't present?

It's not broken, the page reload. But the css should be included to inform the user the page is reloading in case a setting using the refresh transport method has been added by a plugin (like the plugin I've built is doing). To me, Not including the css means we haven't thought of supporting the refresh transport method for a customizer setting.

[DJPaul]

You're the third person to suggest the template should have a site logo -- @timersys and @hnla beat you to it, and I thought about it during original development. Your plugin idea is the first idea i've seen that suggests using the Site Icon (which WP recommends to NOT use for a large site logo) as a small logo near the email title, that's pretty interesting.

I wanted to keep the first version of the template as simple as possible and see what feedback we get. It is easier to add stuff than remove it. :)

All other template parts are using template tags including filter

Yes... but those all use a template loop, and the email doesn't. Rather than filter the site name, how about we add actions before/after it, and also in other sections of the email? Like the rest of BuddyPress' templates?

I agree with your idea to add a function for the email salutation, though.

Not including the css means we haven't thought of supporting the refresh transport method for a customizer setting.

Well, that's true. I didn't bother to test that during original development. :) Out of interest, do you know why doesn't WordPress have a generic style here? Are we not loading something that's normally loaded in the normal Customizer?

If you say we need new styles, OK, but those styles need to be in a new CSS sheet, not hardcoded into the email template, because they are for the Customiser, not the email. Hook the enqueue to the wp_footer action and it should work (though we'll need to check if we're in the Customiser, etc).

[hnla]

I actually wrote a patch to include a simple media upload ability for email header before it being pointed out the ability did exist. My take was a pretty simple facility to allow the user to select an image from media library or upload, similar in fashion to that which might have been used to set the sites header logo, a logo/image quite often that would surfice to be added to the email template at some top position, does this need to be any more complex than that?

[imath]

I wasn't trying to beat hnla or timersys :)

My plugin is working for my need, so i'm fine with whatever decision you will take on this ticket.

Out of interest, do you know why doesn't WordPress have a generic style here?

It has one, but it's hooked to wp_head ​https://core.trac.wordpress.org/browser/trunk/src/wp-includes/class-wp-customize-manager.php#L783 As the email template is not using this hook, i guess that's why it's not loaded.

[DJPaul]

Interesting. So, most of the Customizer pieces came from @timersys' Email Template plugin -- actually, he patched it into BuddyPress for us, doing a lot of work (​https://github.com/paulgibbs/BuddyPress/pull/2/files) so big thanks Damian. :)

His plugin removes all hooked actions and filters, see ​https://github.com/paulgibbs/BuddyPress/issues/4#issuecomment-156811155 and ​https://github.com/paulgibbs/BuddyPress/issues/4#issuecomment-156851463

• We needed wp_footer for the Customizer JS/CSS to work. But not the header.
• I decided to not un-register all hooked actions from wp_footer because I wanted to see what plugins would break this. Apparently Damian found plugins that broke stuff, but I haven't yet, so there's a little risk here.

I am not sure about adding wp_head() this late, although we could. But Damian didn't find a need for it, and I haven't, until now. I'm not sure. What do people think?

An alternate would be to hook customize_preview_loading_style into wp_footer.

[imath]

An alternate would be to hook customize_preview_loading_style into wp_footer.

I like it :) Just tested it and it's working great. See 6896.cssloading.patch

[DJPaul]

6896.cssloading.patch looks good - thanks @imath (feel free to commit that if you beat me to it).

[djpaul]

In 10596:

Emails, Customizer: hook WP CSS to support the refresh transport.

See #6896

Props imath

[djpaul]

In 10607:

Emails: add actions to email template for plugin authors.

See #6896

[imath]

6896.salutation.patch is my suggestion to fix the translation issue.

[imath]

In 10609:

BP Email Template: fix the email salutation i18n issue

See #6896