1119336 - Add css classes to html emails, such as the comment block

Status: RESOLVED FIXED

(Bugzilla :: Email Notifications, enhancement)

Description

[Albert Ting]

css classes should be added to the various sections in the html emails, such as bz_comment_text/bz_comment_head for the comment header/text.  Same goes for the table.

This way, one can add a bugmail-start.html.tmpl hook to define custom styling.

Comment 1

[Albert Ting]

Attachment: v1

Suggested patch that add classes to the comment section and tables.

Comment 2

[Dylan Hardison [:dylan] (he/him)]

Comment on attachment 8546129 [details] [diff] [review]
v1

Review of attachment 8546129 [details] [diff] [review]:
-----------------------------------------------------------------

the patch doesn't apply to trunk.

Comment 3

[Albert Ting]

Attachment: v2

Update patch, with some changes already deployed in bug 1142365

Comment 4

[:glob ✱]

Comment on attachment 8582575 [details] [diff] [review]
v2

Review of attachment 8582575 [details] [diff] [review]:
-----------------------------------------------------------------

::: template/en/default/email/bugmail.html.tmpl~
@@ +27,4 @@
>                {comment_num => comment.count, full_url => 1, user => to_user}) FILTER none %]
>                on [% "$terms.bug $bug.id" FILTER bug_link(bug, { full_url => 1, user => to_user }) FILTER none %]
> +              from [% INCLUDE global/user.html.tmpl user = to_user, who = comment.author %]
> +            </div>

you've changed this block from a <b> to a <div>.  we need to retain the existing style, please revert that change.

because email client have very spotty support for class-driven styles, this should remain a <b>, not <div> with a style defined elsewhere.

@@ +71,5 @@
> +  [% IF isnew %]
> +    [% class_name = "bz_new_table" %]
> +  [% ELSE %]
> +    [% class_name = "bz_diff_table" %]
> +  [% END %]

[% class_name = isnew ? "bz_new_table" : "bz_diff_table" %]

Comment 5

[Albert Ting]

Byron, do you suggest wrapping a <div class="xyz"> around the <b>?

Comment 6

[:glob ✱]

(In reply to Albert Ting from comment #5)
> Byron, do you suggest wrapping a <div class="xyz"> around the <b>?

<b class="xyz">

the stylesheet can set a font-weight to override the weight set by <b>.

Comment 7

[Albert Ting]

Attachment: v3

Updated version based on feedback in comment #4

Comment 8

[:glob ✱]

Comment on attachment 8591149 [details] [diff] [review]
v3

Review of attachment 8591149 [details] [diff] [review]:
-----------------------------------------------------------------

r=glob

i'll fix the issues below when i commit this patch

::: template/en/default/email/bugmail.html.tmpl~
@@ +68,4 @@
>  
>  [% BLOCK generate_diffs %]
>    [% SET in_table = 0 %]
> +    [% class_name = isnew ? "bz_new_table" : "bz_diff_table" %]

nit: weird indentation

@@ +93,4 @@
>          [% END %]
>            <br>
>            [% IF in_table == 0 %]
> +             <table border="1" cellspacing="1" class="[% class_name FILTER none %]">

you cannot remove any of the existing formatting when adding classes.  i'll restore cellpadding and cellspacing values on commit.

@@ +105,4 @@
>  
>        [% PROCESS "email/bugmail-common.txt.tmpl" %]
>        [% IF in_table == 0 %]
> +         <table border="1" cellspacing="1" class="[% class_name FILTER none %]">

same here

Comment 9

[:glob ✱]

To ssh://[e-mail]/bugzilla/bugzilla.git
   7b85e3b..88628a5  master -> master

Comment 10

[:glob ✱]

> Flags: approval?

oops :)