Closed Bug 614215 Opened 10 years ago Closed 10 years ago

Add some color to the sidebar under Aero

Categories

(Thunderbird :: Folder and Message Lists, defect)

x86
Windows Vista
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.3a3

People

(Reporter: andreasn, Assigned: andreasn)

References

Details

Attachments

(4 files, 3 obsolete files)

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.
Attached patch 3 line fix (obsolete) — Splinter Review
and here is the patch
Attached patch updated patchSplinter Review
Missed some stuff in the last patch.
added moz-appearance: none;
Attachment #492627 - Attachment is obsolete: true
Attached patch another patch (obsolete) — Splinter Review
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)
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.
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+
(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.
(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.
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.
Attached patch sidebar patch without !important (obsolete) — Splinter Review
Come on bugzilla, work this time!
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)
Duplicate of this bug: 568187
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
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+
It is!
Setting checkin-needed keyword.
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/5f4c7d86f266
Status: ASSIGNED → RESOLVED
Closed: 10 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.