1619146 - Let the emailWizard use the dark theme

Status: RESOLVED FIXED

(Thunderbird :: Theme, task)

Description

[Richard Marti (:Paenglab)]

When we let the emailWizard use the dark theme when the system theme is dark it looks more integrated.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: 1619146-emailWizard-dark-theme.patch

Alessandro, what do you think about this? I created a new stylesheet, themeableDialog.css which can be used also for other dialogs.

Comment 2

[Richard Marti (:Paenglab)]

Attachment: 1619146-emailWizard-dark-theme.patch

Same patch without special casing btn-link.

Comment 3

[Richard Marti (:Paenglab)]

Attachment: 1619146-emailWizard-dark-theme.patch

Made the input fields using always the variables for background- and text colour. Sorry for the spam.

Comment 4

[Alessandro Castellani [:aleca]]

Comment on attachment 9130185 [details] [diff] [review]
1619146-emailWizard-dark-theme.patch

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

Hey, this is great, I love it so much!
I did a review with screenshots as it was easier to track everything, I hope you don't mine.
Overall this is pretty good and I like the overall feeling and outcome, I think this is the way to go.

Before spending too much time on this, let's ping Magnus and see what he thinks as I remember he was a big proponent of keeping the native elements and let the OS style them.
I'm personally inclined on handling custom element with this solution.

Comment 5

[Alessandro Castellani [:aleca]]

Attachment: dialog-review.jpg

Comment 6

[Richard Marti (:Paenglab)]

The accountWizard, with and without the patch, uses already non-native colours. When the Linux theme is dark, the dialog is still light with some dark parts.

I could make the default theme variables use system colours and when a light TB theme is set use the actual colours and with a dark TB theme use the dark colours.

Comment 7

[Alessandro Castellani [:aleca]]

Oh yeah, sorry if what I wrote was confusing, when I said

I'm personally inclined on handling custom element with this solution.

I was referring to THIS patch, to your solution, and not native styling.

Comment 8

[Richard Marti (:Paenglab)]

No, it wasn't confusing. It was only a note that we should look at this issue too because Linux has no switch to use the lwt-default-theme-in-dark-mode and the dialog doesn't switch to the dark mode except with a dark TB theme.

Comment 9

[Richard Marti (:Paenglab)]

Attachment: 1619146-emailWizard-dark-theme.patch

This patch should address all review comments.

Additionally I removed all unused CSS code and made the dialog use the system default colours when no theme is enabled.

Comment 10

[Alessandro Castellani [:aleca]]

Comment on attachment 9130587 [details] [diff] [review]
1619146-emailWizard-dark-theme.patch

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

This looks super slick, great work.
Just a couple of minor fixes and then it's ready for a full review from Magnus.

::: mail/themes/linux/mail/themeableDialog.css
@@ +21,5 @@
> +
> +menulist[is="menulist-editable"][editable="true"] {
> +  -moz-appearance: none;
> +  margin-inline-end: 4px;
> +  padding: 0;

The default menulist has 1px border transparent which is causing a weird blurred effect around this element (see screenshot).
Maybe we should remove the border entirely from an editable menulist since we're handling it in the child input.

@@ +25,5 @@
> +  padding: 0;
> +}
> +
> +menulist[is="menulist-editable"][editable="true"] .menulist-input {
> +  padding: 3px 4px;

If we remove the menulist default border, this padding can be 4px like the other inputs.

Comment 11

[Alessandro Castellani [:aleca]]

Attachment: Screenshot from 2020-03-03 14-54-18.png

As you can see the menulist items have an extra blurred border compared to the sharp borders of the input fields.

Comment 12

[Richard Marti (:Paenglab)]

Attachment: 1619146-emailWizard-dark-theme.patch

Fixed the border thing. Magnus, what do you think about this? It's an improvement as it makes the dialog use more system colours to better fit but still uses custom widget appearance.

Comment 13

[Alessandro Castellani [:aleca]]

Comment on attachment 9130815 [details] [diff] [review]
1619146-emailWizard-dark-theme.patch

I'm giving this a ui-r+

Comment 14

[Magnus Melin [:mkmelin]]

Comment on attachment 9130815 [details] [diff] [review]
1619146-emailWizard-dark-theme.patch

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

LGTM! r=mkmelin

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/05c780359ee8
Let the emailWizard use the dark theme. ui-r=aleca, r=mkmelin DONTBUILD