908397 - OS X - Make tree-item selected backgrounds consistent with the system style (Finder)

Status: RESOLVED FIXED

(Thunderbird :: Theme, defect)

Description

[Josiah Bruner [:jsbruner]]

On OS X the selected item in the folder pane uses images (sidebar-item) to fill it. We could use CSS instead here. This would allow better scaling and allow better ways to customize its look.

Comment 1

[Josiah Bruner [:jsbruner]]

Attachment: selectedGradients.html

Webpage demoing the idea

Comment 2

[Josiah Bruner [:jsbruner]]

Attachment: Patch.

This switches to CSS for all the selection colors and makes the colors consistent with Finder. It also removes sidebar-item.png from themes/osx/mail/icons/ and the jar.

Comment 3

[Richard Marti (:Paenglab)]

Josiah, after a quick view over the patch I saw two things you could change before my review:
- Please change the -moz-linear-gradient to linear-gradient.
- Please make the lines, where possible, not longer than 80 chars.

Not tested on OS X, does the Finder with graphite theme not differenciate between selected and focus?

Comment 4

[Josiah Bruner [:jsbruner]]

Attachment: Patch.

Fixed.

Finder has only three colors,

* Normal Selected
* Graphite Selected
* Window inactive

I duplicated this as best as possible in this patch. However, the address book pane doesn't use window inactive and instead uses not selected, but that is because of a separate bug.

Comment 5

[Richard Marti (:Paenglab)]

Comment on attachment 798663 [details] [diff] [review]
Patch.

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

linear-gradient(top,...) isn't valid, shows no gradient and is throwing errors in console. It needs either a 'to bottom' or nothing (then it automatically uses 'to bottom').

ui-r- because this patch shows no gradients.

::: mail/themes/osx/mail/addrbook/addressbook.css
@@ +347,5 @@
>    background-color: transparent;
>  }
>  
> +#dirTree treechildren::-moz-tree-row(selected),
> +#dirTree treechildren:-moz-window-inactive::-moz-tree-row(selected) {

Please remove the line with :-moz-window-inactive as it doesn't work in AB. It can be readded with the bug you mentioned you want to file.

@@ +348,5 @@
>  }
>  
> +#dirTree treechildren::-moz-tree-row(selected),
> +#dirTree treechildren:-moz-window-inactive::-moz-tree-row(selected) {
> +  background: linear-gradient(top, #bbc5d7 6%, #c3cbe0 6%, #bdc6dd 7%, 

trailing space

@@ +354,4 @@
>  }
>  
>  #dirTree treechildren::-moz-tree-row(selected, focus) {
> +  background: linear-gradient(top, #5ea3df 6%, #73b8ea 6%, #6cafe5 7%, 

trailing space

I think we don't need to differentiate between selected and focus and provide only one gradient like in mac-graphite-theme, chat and folderTree (and also Finder).

@@ +359,5 @@
>  }
>  
> +#dirTree:-moz-system-metric(mac-graphite-theme) treechildren::-moz-tree-row(selected),
> +#dirTree:-moz-system-metric(mac-graphite-theme) treechildren::-moz-tree-row(selected, focus) {
> +  background: linear-gradient(top, #7b8b9e 6%, #93a3b4 6%, #8fa0b1 7%, 

trailing space

::: mail/themes/osx/mail/chat.css
@@ +231,5 @@
>    }
>  }
>  
>  :-moz-any(imconv, imcontact, imgroup)[selected] {
> +  background: linear-gradient(top, #5ea3df 6%, #73b8ea 6%, #6cafe5 7%, 

trailing space

@@ +237,4 @@
>    color: HighlightText;
>  }
>  
>  #contactlistbox:focus > :-moz-any(imconv, imcontact, imgroup)[selected] {

This selector isn't needed. The previous selector is enough and we differentiate no more like Finder.

@@ +237,5 @@
>    color: HighlightText;
>  }
>  
>  #contactlistbox:focus > :-moz-any(imconv, imcontact, imgroup)[selected] {
> +  background: linear-gradient(top, #5ea3df 6%, #73b8ea 6%, #6cafe5 7%, 

trailing space

@@ +246,1 @@
>  #contactlistbox:focus > :-moz-any(imconv, imcontact, imgroup)[selected]:-moz-system-metric(mac-graphite-theme) {

Again the #contactlistbox:focus > ... isn't needed.

@@ +246,2 @@
>  #contactlistbox:focus > :-moz-any(imconv, imcontact, imgroup)[selected]:-moz-system-metric(mac-graphite-theme) {
> +  background: linear-gradient(top, #7b8b9e 6%, #93a3b4 6%, #8fa0b1 7%, 

trailing space

@@ +249,4 @@
>  }
>  
>  :-moz-any(imconv, imcontact, imgroup)[selected]:-moz-window-inactive {
> +  background: linear-gradient(top, #bbc5d7 6%, #c3cbe0 6%, #bdc6dd 7%, 

trailing space

::: mail/themes/osx/mail/mailWindow1.css
@@ +79,5 @@
>  }
>  
> +#folderTree treechildren::-moz-tree-row(selected),
> +#folderTree treechildren::-moz-tree-row(selected, focus) {
> +  background: linear-gradient(top, #5ea3df 6%, #73b8ea 6%, #6cafe5 7%, 

trailing space

@@ +91,4 @@
>  }
>  
>  #folderTree treechildren:-moz-window-inactive::-moz-tree-row(selected) {
> +  background: linear-gradient(top, #bbc5d7 6%, #c3cbe0 6%, #bdc6dd 7%, 

trailing space

@@ +96,3 @@
>  }
>  
> +#folderTree treechildren::-moz-tree-cell-text(selected, focus), 

trailing space

Comment 6

[Josiah Bruner [:jsbruner]]

Attachment: Patch.

So it looks like the "top" thing is only valid with the -moz- prefix. Fixed now.

Resetting review flags.

Comment 7

[Richard Marti (:Paenglab)]

Comment on attachment 800169 [details] [diff] [review]
Patch.

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

Yeah, now it looks good.

Comment 8

[Josiah Bruner [:jsbruner]]

Attachment: Final Patch.

Adding commit message....

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/comm-central/rev/740217e93916