1504187 - Style all trees with the sidebar colors

Status: RESOLVED FIXED

(Thunderbird :: Theme, enhancement)

Description

[Richard Marti (:Paenglab)]

Actually we style only the folderTree. With this patch we style all trees in the main window, the AB and the composer without the contacts sidebar (I'll look later into it).

Comment 1

[Richard Marti (:Paenglab)]

Attachment: 1504187-themeTrees.patch

This patch applies the sidebar colours to all trees of the three windows main, AB and composer. As wrote above. The contacts sidebar isn't styled yet and also the splitters will be done in a other bug.

Comment 2

[Jorg K (GMT+2)]

I find a dark tread pane quite gloomy. That said, there appears to be a problem with tagged rows which are selected and hovered. Say you use "3": not selected you have green on dark grey, selected you have white on green, but if also hovered, you have white on dark grey, so you can't see the tag colour.

For the address book, I'd leave the address preview in white even for the dark theme to match the main window where the message preview also has a light background.

Comment 3

[Richard Marti (:Paenglab)]

Attachment: 1504187-themeTrees.patch v2

The AB's address preview is now no more styled.

It's not possible to not highlight the tagged tree row when hovering. For this we'd need to add on every rule in tagColors.css a !important. With bug 1497041 this should be much simpler to do because this would be only one rule which will be filled by variables. it's also not so severe it's not static and when the user hovers it, he knows where he goes with the mouse.

Comment 4

[Jorg K (GMT+2)]

(In reply to Richard Marti (:Paenglab) from comment #3)
> It's not possible to not highlight the tagged tree row when hovering.
But the default theme does that. Row is green, hover a very light blue, you hover a green row, it stays green. Sorry about the ignorance ;-(

Comment 5

[Richard Marti (:Paenglab)]

The dark- and the WE-themes need to override the default theme. And this overrides have automatically a higher specificity than the tagColors.css rules, except on Linux where !important is set. Trees are very limited and a :not() isn't working with them.

Comment 6

[Jorg K (GMT+2)]

Attachment: add-important-to-background-for-windows.patch

So why don't we set !important on Windows? That makes the hovering of selected tagged rows work. Don't worry, I didn't do the 209 changes manually since regular expressions are my friend. If you like it, I can do the Mac file as well.

Comment 7

[Richard Marti (:Paenglab)]

Comment on attachment 9022332 [details] [diff] [review]
add-important-to-background-for-windows.patch

This looks good, thanks. Yes, please do the same for the Mac file, also when it's not really needed because Mac and Linux have no hover styles for the trees since the unification in m-c. Maybe I re-implement it again.

Comment 8

[Jorg K (GMT+2)]

Attachment: add-important-to-background-for-windows.patch - Win + Mac.

Win + Mac. Please merge with your patch.

Comment 9

[[:MakeMyDay]]

I observed two glitches with these patches applied:

1. In email tree of the 3-pane window some emails are highlighted with the background color used in the default theme when you move the mousepointer away from the tree item (or selecet another item using the up/down arrows). Other items are hightlighted with the dark bar as expected when doing the same. There seems to be no rule when this happens.

2. The popup to control which tree columns are displayed has still the grey background color.

Comment 10

[[:MakeMyDay]]

Using Win10 that is.

Comment 11

[Richard Marti (:Paenglab)]

Attachment: 1504187-themeTrees.patch v3

(In reply to [:MakeMyDay] from comment #9)
> I observed two glitches with these patches applied:
> 
> 1. In email tree of the 3-pane window some emails are highlighted with the
> background color used in the default theme when you move the mousepointer
> away from the tree item (or selecet another item using the up/down arrows).
> Other items are hightlighted with the dark bar as expected when doing the
> same. There seems to be no rule when this happens.

I tried different ways and haven't seen this. Was this with the first patch or with v2? In v2 I simplified some selectors.

> 2. The popup to control which tree columns are displayed has still the grey
> background color.

The popups aren't styled. FX doesn't do this too (context menus, main menu). I'm planning to theme the AppMenu too.

Comment 12

[Jorg K (GMT+2)]

Attachment: bad-highlight.png

(In reply to [:MakeMyDay] from comment #9)
> 1. In email tree of the 3-pane window some emails are highlighted with the
> background color used in the default theme when you move the mousepointer
> away from the tree item (or selecet another item using the up/down arrows).
> Other items are hightlighted with the dark bar as expected when doing the
> same. There seems to be no rule when this happens.

STR:
1) Select dark theme.
2) Use list view.
3) Click a row to select it. It's highlighted in a lighter grey.
4) Move the mouse away from the row, that is, stop hovering it.

Result: New hovered row gets light grey, selected row goes bright :-(

Seems to happen when there is a tagged row near by, but not easily reproducible.

Comment 13

[Jorg K (GMT+2)]

Comment on attachment 9022359 [details] [diff] [review]
1504187-themeTrees.patch v3

OK, let's go with this. We'll see whether we can fix the graphic glitch in a follow-up. (I need a patch to land now.)

Comment 14

[Jorg K (GMT+2)]

Attachment: bad-highlight2.png

Also happens with a WE theme.

Comment 15

[Jorg K (GMT+2)]

Actually, I'll land something else.

Comment 16

[Richard Marti (:Paenglab)]

(In reply to Jorg K (GMT+2) from comment #12)
> Created attachment 9022362 [details]
> bad-highlight.png
> 
> (In reply to [:MakeMyDay] from comment #9)
> > 1. In email tree of the 3-pane window some emails are highlighted with the
> > background color used in the default theme when you move the mousepointer
> > away from the tree item (or selecet another item using the up/down arrows).
> > Other items are hightlighted with the dark bar as expected when doing the
> > same. There seems to be no rule when this happens.
> 
> STR:
> 1) Select dark theme.
> 2) Use list view.

What is list view? The normal thread list?

> 3) Click a row to select it. It's highlighted in a lighter grey.
> 4) Move the mouse away from the row, that is, stop hovering it.
> 
> Result: New hovered row gets light grey, selected row goes bright :-(
> 
> Seems to happen when there is a tagged row near by, but not easily
> reproducible.

I still can't see this. Can you try this with attachment 9022113 [details] [diff] [review] if this still happens?

Comment 17

[Jorg K (GMT+2)]

(In reply to Richard Marti (:Paenglab) from comment #16)
> What is list view? The normal thread list?
Yes, the normal list, as you can see in the picture.

> I still can't see this. 
I'm not surprised. I've just destroyed my reproducible case by adding a tag to the row and removing it again.

> Can you try this with attachment 9022113 [details] [diff] [review] if this still happens?
Yes, the first patch here also had the problem. But my reproducible case is gone now and for as much as I tried, I couldn't reproduce it any more. We just have to take the patch and keep an eye out for it.

Comment 18

[Richard Marti (:Paenglab)]

Attachment: 1504187-themeTrees.patch v4

Working on the calendar part I found a small issue not setting the correct text colour when hovering or selecting the treeitems with my Nuvola-WE theme. Normally the text is black but when hovering/selecting the text should be white.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/e2d7001a1a87
Make all trees themeable with the WX-theme sidebar colors. r=jorgk

Comment 22

[Martijn]

Fixed in TB 65.0?!

My Thunderbird is on 60.x and it says it's up-to-date. Do I really have to wait 5 years?

Comment 23

[Jorg K (GMT+2)]

No, until July 2019. Or join the beta channel.