614215 - Add some color to the sidebar under Aero

Status: RESOLVED FIXED

(Thunderbird :: Folder and Message Lists, defect)

Description

[Andreas Nilsson (:andreasn)]

It would be good to add some color to the sidebar under aero, similar to what Firefox does for it's bookmark sidebar. Especially since we're landing the changes in bug 545557 and we need to make the difference between the areas more clear.

Comment 1

[Andreas Nilsson (:andreasn)]

Attachment: 3 line fix

and here is the patch

Comment 2

[Andreas Nilsson (:andreasn)]

Attachment: updated patch

Missed some stuff in the last patch.
added moz-appearance: none;

Comment 3

[Andreas Nilsson (:andreasn)]

Attachment: another patch

This adds some changes to the sidebar header as well.
I would favor this bug to the previous one.

Comment 4

[Andreas Nilsson (:andreasn)]

Blake: I picked you as the reviewer since you were doing the code review in bug 545557 that I feel is closely related to this. Let me know if someone else as reviewer would be better.

Comment 5

[Richard Marti (:Paenglab)]

Attachment: proposal of an alternate sidebar header styling

I had also a new sidebar header styling in my head, but would wait until bug
545557 was checked in. My proposal is a copy of the toolbar gradient from bug
545557.

What are you thinking about this?

Comment 6

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 492647 [details] [diff] [review]
another patch

I like it, although I think that the message list and message body should perhaps also be that nice #eef2fa…

Still, that's no reason not to accept this patch.  :)

Later,
Blake.

Comment 7

[Andreas Nilsson (:andreasn)]

(In reply to comment #5)
> Created attachment 492703 [details]
> proposal of an alternate sidebar header styling
> 
> I had also a new sidebar header styling in my head, but would wait until bug
> 545557 was checked in. My proposal is a copy of the toolbar gradient from bug
> 545557.
> 
> What are you thinking about this?

I can't really decide what of the two is better. I always like flat things, but it might clash a bit with the tree header next to it. What do you think Blake?

Comment 8

[Richard Marti (:Paenglab)]

With this patch wouldn't the folderTree become the same problem as the AB window in Bug 549306?

Comment 9

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 492647 [details] [diff] [review]
another patch

Does this need to check for the default theme?  I'm wondering because there are background colors and that often doesn't work well with the a11y colors.

Comment 10

[Andreas Nilsson (:andreasn)]

(In reply to comment #9)
> Comment on attachment 492647 [details] [diff] [review]
> another patch
> 
> Does this need to check for the default theme?  I'm wondering because there are
> background colors and that often doesn't work well with the a11y colors.

No, it probably need some work as Richard said in Comment 8. Will do a new patch to fix that.

Comment 11

[Andreas Nilsson (:andreasn)]

Attachment: updated sidebar patch

This takes highcontrast and classic into consideration.
I'm a bit unhappy about the border: none !imprortant; bits though, and I can't figure out why I need them.

Comment 12

[Richard Marti (:Paenglab)]

(In reply to comment #11)
> Created attachment 497265 [details] [diff] [review]
> updated sidebar patch
> 
> This takes highcontrast and classic into consideration.
> I'm a bit unhappy about the border: none !imprortant; bits though, and I can't
> figure out why I need them.

This is because the folderPane.css is treated before mailWindow1.css which has also definitions for #folderTree. When you apply the patch at the end (or after the definition at line 55) of mailWindow1.css, then the !important aren't needed.

Comment 13

[Andreas Nilsson (:andreasn)]

Attachment: sidebar patch without !important

Come on bugzilla, work this time!

Comment 14

[Andreas Nilsson (:andreasn)]

Comment on attachment 497759 [details] [diff] [review]
sidebar patch without !important

Thanks for the tip Richard, it did the trick!

Comment 16

[Blake Winton (:bwinton) (:☕️)]

Comment on attachment 497759 [details] [diff] [review]
sidebar patch without !important

>+++ b/mail/themes/qute/mail/mailWindow1-aero.css
>@@ -98,3 +98,21 @@ padding: 4px 0px;
>+#folderPaneHeader {
>+  -moz-appearance: none;
>+  background-color: #f8f8f8;
>+  border-top: 0px;
>+  border-bottom: 1px solid #a9b1b8;

I think these lines should be indented another 2 spaces.

>+  }

But this line is fine.

>+#folderTree {
>+  -moz-appearance: none;
>+  background-color: #eef3fa;
>+  border-top: 1px solid #ffffff;
>+  border-bottom: none;
>+  border-left: none;

And these should be indented another two spaces as well.

>+  }
>+}

But these are fine.

Other than that, I don't think I have a problem with it.  r=me.

Oh, and it looks like philor would be your other choice for review, but as I recall, he's got an awfully long review queue as it is, so I'm happy to pick this one up.  :)

Comment 17

[Richard Marti (:Paenglab)]

Attachment: sidebar patch for check-in

Patch addressing the comments and unbitrotted.
Taking over r+ and ui-r+ from previous patch.

Andreas, if this patch is okay for you, can you set the checkin-needed keyword?

Comment 18

[Andreas Nilsson (:andreasn)]

It is!
Setting checkin-needed keyword.

Comment 19

[Mark Banner (:standard8)]

Checked in: http://hg.mozilla.org/comm-central/rev/5f4c7d86f266