1711360 - Improve the theming of the new address book

Status: RESOLVED FIXED

(Thunderbird :: Theme, task)

Description

[Richard Marti (:Paenglab)]

I am so cheeky and fix some UI glitches. What I'm doing in this bug is:

• Update the variables to be inline with the latest colour changes
• Make the button, text field and select sizing inline with our in-content styles
• Make the icon in card column follow the text colour
• Make the toolbar buttons use the correct colours with dark themes
• On Windows 10 the sort popup has a weird left padding
• Make the focus rings follow the Proton style

Comment 1

[Richard Marti (:Paenglab)]

Attachment: 1711360-new-ab-theming.patch

(In reply to Richard Marti (:Paenglab) from comment #0)

I am so cheeky and fix some UI glitches. What I'm doing in this bug is:

• Update the variables to be inline with the latest colour changes

done

• Make the button, text field and select sizing inline with our in-content styles

done, I also reordered the loading of the CSS files to make the buttons be more inline with the in-content buttons. We still have a border when :hover:active. This comes from messenger.css we can't remove because it is needed for other parts of this page.

• Make the icon in card column follow the text colour

done

• Make the toolbar buttons use the correct colours with dark themes

done

• On Windows 10 the sort popup has a weird left padding

done by adding needsgutter="" to the menupopup

• Make the focus rings follow the Proton style

done

I also gave the list elements a inline margin of 6px and a border radius. What do you think about this?

A big issue is, that with LW-themes with a background image the image isn't shown on the toolbar like on other tabs (like the Find more add-ons). This is because the #document inside the tab doesn't have a transparent background. I don't know how we could fix this. If the prefs would have a toolbar it would be the same problem. This would apply on all HTML pages with a toolbar.
With a dark LW-theme like Dark Foxthe toolbar is completely unreadable actually.

Geoff, I hope you aren't angry that I did this.

Comment 2

[Richard Marti (:Paenglab)]

(In reply to Richard Marti (:Paenglab) from comment #1)

I also gave the list elements a inline margin of 6px and a border radius. What do you think about this?

With 8px it would align with the search box.

Comment 3

[Geoff Lankow (:darktrojan)]

I'm not mad, in fact one of the reasons I got it all landed was so that I'd stop messing around with the theming myself. I spent a lot of time trying to keep up with the changes (thanks Proton!).

• Make the icon in card column follow the text colour

This was working, and then it stopped. I'm having issues with -moz-context-properties that never used to be there.

• Make the toolbar buttons use the correct colours with dark themes

Ugh, I tried. Never found the right class name obviously.

• Make the focus rings follow the Proton style

🙄

The background colour of the search box should probably be fixed in common.css, but I hadn't got around to filing a patch for that.

I don't know what to do about the toolbar background either. Certain people would get rid of the toolbar completely.

Comment 4

[Alessandro Castellani [:aleca]]

Comment on attachment 9222054 [details] [diff] [review]
1711360-new-ab-theming.patch

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

This is great, thanks for the improvements.

A bunch of things that we should implement but maybe in a follow up patch.
- The menupopup should be a doorhanger (popupanel).
- The current sorting order should be represented by a checkmark to the left side of the item.
- The button should have a title tag with "Change list sorting order" or something like that.
- The items of the list should have an hover effect.
- The subdialogs opened from the toolbarbuttons should be "themeable" as they conflict with the in-content style.

Apologies if some of these things are already known.
Great work everybody.

Comment 5

[Richard Marti (:Paenglab)]

Attachment: 1711360-new-ab-theming.patch

(In reply to Alessandro Castellani [:aleca] from comment #4)

Comment on attachment 9222054 [details] [diff] [review]
1711360-new-ab-theming.patch

Review of attachment 9222054 [details] [diff] [review]:

A bunch of things that we should implement but maybe in a follow up patch.

• The menupopup should be a doorhanger (popupanel).

Something for a follow up bug.

• The current sorting order should be represented by a checkmark to the left
  side of the item.

On Mac and Windows are checkmarks shown. Maybe a toolkit bug? You should switch your default OS. ;-) With a doorhanger this could be fixed.

• The button should have a title tag with "Change list sorting order" or
  something like that.

Done

• The items of the list should have an hover effect.

Done

• The subdialogs opened from the toolbarbuttons should be "themeable" as
  they conflict with the in-content style.

The same issue like we had for the edit Card dialog in AM. The dialogs are designed as stand alone dialogs. We should wait until the new AB is the default and change them then to work as subdialogs.

Comment 6

[Richard Marti (:Paenglab)]

Attachment: 1711360-new-ab-theming.patch

Aligned the listitems with the search box by using 6px for the search box.

Comment 7

[Alessandro Castellani [:aleca]]

Comment on attachment 9222237 [details] [diff] [review]
1711360-new-ab-theming.patch

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

Good job, thanks.

Comment 8

[Geoff Lankow (:darktrojan)]

(In reply to Alessandro Castellani [:aleca] from comment #4)

• The menupopup should be a doorhanger (popupanel).

Until recently it was a menu button but I'm not allowed to have that because it's XUL.

• The current sorting order should be represented by a checkmark to the left
  side of the item.

I experience the same problem on Linux but not Windows or Mac. I haven't got to figuring out what's going on yet, but if we make it a doorhanger then it doesn't have to be figured out.

• The subdialogs opened from the toolbarbuttons should be "themeable" as
  they conflict with the in-content style.

I'm not sure how many of them will be hanging around long-term. I figure it's good enough for now given they're still in use the old way.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/29952c81a1a4
Improve the theming of the new address book. r=aleca