Add some color to the sidebar under Aero

RESOLVED FIXED in Thunderbird 3.3a3

Status

RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: andreasn, Assigned: andreasn)

Tracking

Trunk
Thunderbird 3.3a3
x86
Windows Vista

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(4 attachments, 3 obsolete attachments)

(Assignee)

Description

8 years ago
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.
(Assignee)

Comment 1

8 years ago
Created attachment 492627 [details] [diff] [review]
3 line fix

and here is the patch
(Assignee)

Comment 2

8 years ago
Created attachment 492632 [details] [diff] [review]
updated patch

Missed some stuff in the last patch.
added moz-appearance: none;
Attachment #492627 - Attachment is obsolete: true
(Assignee)

Comment 3

8 years ago
Created attachment 492647 [details] [diff] [review]
another patch

This adds some changes to the sidebar header as well.
I would favor this bug to the previous one.
Attachment #492647 - Flags: ui-review?(clarkbw)
Attachment #492647 - Flags: review?(bwinton)
(Assignee)

Comment 4

8 years ago
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.
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?
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.
Attachment #492647 - Flags: review?(bwinton) → review+
(Assignee)

Comment 7

8 years ago
(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?
With this patch wouldn't the folderTree become the same problem as the AB window in Bug 549306?
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.
(Assignee)

Comment 10

8 years ago
(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.
(Assignee)

Comment 11

8 years ago
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.
Attachment #492647 - Attachment is obsolete: true
Attachment #492647 - Flags: ui-review?(clarkbw)
(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.
(Assignee)

Comment 13

8 years ago
Created attachment 497759 [details] [diff] [review]
sidebar patch without !important

Come on bugzilla, work this time!
(Assignee)

Comment 14

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

Thanks for the tip Richard, it did the trick!
Attachment #497759 - Flags: ui-review?(clarkbw)
Attachment #497759 - Flags: review?(bwinton)
(Assignee)

Updated

8 years ago
Duplicate of this bug: 568187
(Assignee)

Updated

8 years ago
Blocks: 545557
Attachment #497759 - Flags: ui-review?(clarkbw) → ui-review+
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.  :)
Attachment #497759 - Flags: review?(bwinton) → review+
Assignee: nobody → nisses.mail
Status: NEW → ASSIGNED
Created attachment 516359 [details] [diff] [review]
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?
Attachment #497759 - Attachment is obsolete: true
Attachment #516359 - Flags: ui-review+
Attachment #516359 - Flags: review+
(Assignee)

Comment 18

8 years ago
It is!
Setting checkin-needed keyword.
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/5f4c7d86f266
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a3
You need to log in before you can comment on or make changes to this bug.