Closed Bug 1264001 Opened 4 years ago Closed 4 years ago

Softly restyle the multiMessage view

Categories

(Thunderbird :: Theme, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 50.0

People

(Reporter: Paenglab, Assigned: Paenglab)

Details

Attachments

(2 files, 1 obsolete file)

The multimessage view is with it's hard coded colors doesn't respect High Contrast themes for example.

Also the toolbar buttons don't look good because they have some transparency and are not so clear.
Attached patch mulitiMessage.patch (obsolete) — Splinter Review
This patch uses system colors except for the toolbar buttons to have a defined background color.

I also added between the messages a thin separator to better divide the messages.

One question, why do we have two common files (mail/base/content/multimessageview.css and mail/base/content/sharedsummary.css) for the styles? can't we combine them in one file?
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8740512 - Flags: review?(squibblyflabbetydoo)
Isn't this the same as bug 690241?
Ah yes. But the xul part and some css rules from old patch are already in tree. The difference in this patch to the one from bug 690241 is the missing

#heading-wrapper {
  background-image: -moz-linear-gradient(rgba(242,242,240,0),
                                         rgba(242,242,240,.25));

my addition of the bottom border to separate the messages and the toolbar button styles.

And my patch applies on all platform.

What would we do? Go further with this bug and close the old, or vice versa?
Let's work on it in this bug. I don't think we necessarily need that gradient. I think I added it to match Mail Summaries, but it's ok without it. Still, I kind of like the look of the gradient.

I think we should keep the CSS files separate as well, since that will help simplify things when I finally merge Mail Summaries into Thunderbird.
Comment on attachment 8740512 [details] [diff] [review]
mulitiMessage.patch

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

Overall, this looks good. There are a few things I'm not sure about; see below.

::: mail/base/content/multimessageview.css
@@ +34,5 @@
>    list-style-type: none;
>  }
>  
>  #footer {
> +  margin-top: 1em;

Is the additional margin-top necessary? (I don't have an opinion either way, really.)

@@ +50,5 @@
>    margin-bottom: 1ex;
>    border-width: 2px;
> +  border-radius: 2px;
> +  border: 1px solid transparent;
> +  border-bottom-color: lightgrey;

What are these changes for?

::: mail/themes/windows/mail/multimessageview.css
@@ +50,5 @@
> +    }
> +
> +    .toolbarbutton-1.msgHeaderView-button:not([type="menu-button"]):not([disabled="true"]):-moz-any([open="true"],[checked="true"],:hover:active) {
> +      background-image: linear-gradient(rgba(0, 0, 0, .15), rgba(0, 0, 0, .15)), linear-gradient(-moz-dialog, -moz-dialog);
> +    }

Why are these necessary? I thought we got all the relevant styles for the toolbarbuttons from elsewhere.
Attachment #8740512 - Flags: review?(squibblyflabbetydoo)
Attached image Patch in action
(In reply to Jim Porter (:squib) from comment #5)
> Comment on attachment 8740512 [details] [diff] [review]
> mulitiMessage.patch
> 
> Review of attachment 8740512 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Overall, this looks good. There are a few things I'm not sure about; see
> below.
> 
> ::: mail/base/content/multimessageview.css
> @@ +34,5 @@
> >    list-style-type: none;
> >  }
> >  
> >  #footer {
> > +  margin-top: 1em;
> 
> Is the additional margin-top necessary? (I don't have an opinion either way,
> really.)

This is to give a gap between the messages and the footer which have a summary info about the messages. This to show this footer is not part of the messages.

> @@ +50,5 @@
> >    margin-bottom: 1ex;
> >    border-width: 2px;
> > +  border-radius: 2px;
> > +  border: 1px solid transparent;
> > +  border-bottom-color: lightgrey;
> 
> What are these changes for?

This is to make the border radius smaller for threaded message groups. Also to show always a line between the messages (see the screenshot). I could also remove this line. But for me this separates them better.
 
> ::: mail/themes/windows/mail/multimessageview.css
> @@ +50,5 @@
> > +    }
> > +
> > +    .toolbarbutton-1.msgHeaderView-button:not([type="menu-button"]):not([disabled="true"]):-moz-any([open="true"],[checked="true"],:hover:active) {
> > +      background-image: linear-gradient(rgba(0, 0, 0, .15), rgba(0, 0, 0, .15)), linear-gradient(-moz-dialog, -moz-dialog);
> > +    }
> 
> Why are these necessary? I thought we got all the relevant styles for the
> toolbarbuttons from elsewhere.

Correct, but this buttons use semi transparent gradients which make the buttons not clearly separated from the background. I added a additional gradient with the toolbar background color to let them look like the normal toolbar buttons.
Comment on attachment 8740512 [details] [diff] [review]
mulitiMessage.patch

I'm adding r? with the questions replied in previous comment.
Attachment #8740512 - Flags: review?(squibblyflabbetydoo)
Updated to tip after unprefix bug 1272606.
Attachment #8740512 - Attachment is obsolete: true
Attachment #8740512 - Flags: review?(squibblyflabbetydoo)
Attachment #8752578 - Flags: review?(squibblyflabbetydoo)
Comment on attachment 8752578 [details] [diff] [review]
mulitiMessage.patch v2

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

This looks good to me. I don't have any strong opinion about the changes I was asking about earlier, so let's land this!
Attachment #8752578 - Flags: review?(squibblyflabbetydoo) → review+
Thank you Jim.

https://hg.mozilla.org/comm-central/rev/886e5e5c86c8
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 50.0
You need to log in before you can comment on or make changes to this bug.