Use svg icons headerPane's emailButton and in edit contacts panel

RESOLVED FIXED in Thunderbird 61.0

Status

enhancement
RESOLVED FIXED
Last year
Last year

People

(Reporter: Paenglab, Assigned: Paenglab)

Tracking

unspecified
Thunderbird 61.0

Thunderbird Tracking Flags

(thunderbird60 fixed, thunderbird61 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Assignee

Description

Last year
Like the FX bug 1441727, we should also use a SVG icon for the star in the address edit popup (the popup that opens when you click on the star on the right of the address in message header.
Assignee

Comment 1

Last year
Posted patch starIcon.patch (obsolete) — Splinter Review
Additionally to the icon exchange, I aligned the star better to the address in message header. Also the header in the popup is better aligned. The buttons end now aligned with the menulists too.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8959110 - Flags: review?(jorgk)
Assignee

Comment 2

Last year
Jörg, you can also test with my crazy colours WE-test-theme to see that it's coloured like it does on FX bookmark popup.

Comment 3

Last year
Comment on attachment 8959110 [details] [diff] [review]
starIcon.patch

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

Works for me.

(In reply to Richard Marti (:Paenglab) from comment #2)
> Jörg, you can also test with my crazy colours WE-test-theme to see that it's
> coloured like it does on FX bookmark popup.
I didn't test that. Which version of FF do I need? I only have the release version, but there, the bookmark star is blue like with this patch. Which theme are you referring to, the one from bug 1439134 or the one from bug 1435549 ("ugly colours"). It wouldn't be the one from bug 1443226.

I was wondering which comment conventions you use in CSS. Something like
/* this is a comment that starts on one line
   and ends on the next */
would not pass review.

::: mail/themes/shared/mail/messageHeader.css
@@ +141,5 @@
> +  color: HighlightText;
> +  background-color: Highlight;
> +}
> +
> +mail-emailaddress[selected="true"] > .emailDisplayButton{

space before {

@@ +144,5 @@
> +
> +mail-emailaddress[selected="true"] > .emailDisplayButton{
> +  /* when an email address context menu is selected,
> +    make sure that the email bubble stays displayed, and
> +    tweak the bottom to blend in more w/ the menu */

Ugly comment. OK, pre-existing, was moved.

@@ +161,5 @@
>  /* Because there's no star for newsgroups like there is for email addresses,
>   * pushing it back the same number of pixels as emailSeparator causes
>   * the comma to be drawn too close to the last letter, which looks bad, so
>   * using a separate rule here.
>   */

Here's a better looking comment.
Attachment #8959110 - Flags: review?(jorgk) → review+
Assignee

Comment 4

Last year
(In reply to Jorg K (GMT+1) from comment #3)
> Comment on attachment 8959110 [details] [diff] [review]
> starIcon.patch
> 
> Review of attachment 8959110 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Works for me.
> 
> (In reply to Richard Marti (:Paenglab) from comment #2)
> > Jörg, you can also test with my crazy colours WE-test-theme to see that it's
> > coloured like it does on FX bookmark popup.
> I didn't test that. Which version of FF do I need? I only have the release
> version, but there, the bookmark star is blue like with this patch. Which
> theme are you referring to, the one from bug 1439134 or the one from bug
> 1435549 ("ugly colours"). It wouldn't be the one from bug 1443226.

FX 60. Sorry, none of the extensions I uploaded there. It was a later m-c change of colouring arrow popups I have in the extension which needed no change in TB. I'll attach it therefore you can test it if you like.

> I was wondering which comment conventions you use in CSS. Something like
> /* this is a comment that starts on one line
>    and ends on the next */
> would not pass review.

This is a standard CSS comment. Everything after a /* is commented out until a */ appears. It doesn't end at line end.

> ::: mail/themes/shared/mail/messageHeader.css
> @@ +141,5 @@
> > +  color: HighlightText;
> > +  background-color: Highlight;
> > +}
> > +
> > +mail-emailaddress[selected="true"] > .emailDisplayButton{
> 
> space before {

Fixed.

> @@ +144,5 @@
> > +
> > +mail-emailaddress[selected="true"] > .emailDisplayButton{
> > +  /* when an email address context menu is selected,
> > +    make sure that the email bubble stays displayed, and
> > +    tweak the bottom to blend in more w/ the menu */
> 
> Ugly comment. OK, pre-existing, was moved.

I now added the leading * to be more consistent with the following comment and exchanged the "bubble" to "menupopup" to be more precise.
Attachment #8959110 - Attachment is obsolete: true
Attachment #8959443 - Flags: review+
Assignee

Comment 5

Last year
newest "ugly colours" test theme, if you want test it.
Assignee

Updated

Last year
Keywords: checkin-needed

Comment 6

Last year
Pushed by mozilla@jorgk.com:
https://hg.mozilla.org/comm-central/rev/0ab0740d6969
Use svg icons headerPane's email button and in edit contacts panel. r=jorgk
Status: ASSIGNED → RESOLVED
Closed: Last year
Keywords: checkin-needed
Resolution: --- → FIXED

Updated

Last year
Target Milestone: --- → Thunderbird 61.0
Assignee

Comment 7

Last year
Comment on attachment 8959443 [details] [diff] [review]
starIcon.patch

Can we have this uplifted for FX parity?
Attachment #8959443 - Flags: approval-comm-beta?

Updated

Last year
Attachment #8959443 - Flags: approval-comm-beta? → approval-comm-beta+
You need to log in before you can comment on or make changes to this bug.