1652015 - Make the folder pane dialogs themeable

Status: RESOLVED FIXED

(Thunderbird :: Theme, task)

Description

[Richard Marti (:Paenglab)]

Now we can make the folder pane dialogs like "New Folder", "Properties", "Subscribe" etc. themeable.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: 1652015-folderPane-dialogs-themeable.patch

More themeable dialogs.

Comment 2

[Alessandro Castellani [:aleca]]

Comment on attachment 9162815 [details] [diff] [review]
1652015-folderPane-dialogs-themeable.patch

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

Indeed, this changes make those dialogs themeable, but in the process the UI gets affected and it all feels a bit messy.
A lot of these issues are not related to this bug, so feel free to ignore them and defer them to another bug.

I'm uploading screenshots with the highlighted issues.

Comment 3

[Alessandro Castellani [:aleca]]

Attachment: Frame 1.png

Here's an overview of the UI aspects we should fix in these dialogs.
Let me know what you think and if some of these should be deferred to other bugs.

Comment 4

[Richard Marti (:Paenglab)]

Attachment: 1652015-folderPane-dialogs-themeable.patch

Better like this?

Comment 5

[Alessandro Castellani [:aleca]]

Comment on attachment 9163015 [details] [diff] [review]
1652015-folderPane-dialogs-themeable.patch

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

This looks way better, thank you so much.

You didn't align the label and input into a grid in the Create New Folder dialog.
It's not trivial if you think it shouldn't be done, but since we have that structure for the Virtual Folder dialog, I think we should keep it consistent.

::: mailnews/base/content/folderProps.xhtml
@@ +38,5 @@
>    <tabpanels id="folderPropTabPanels">
>  
>      <vbox id="GeneralPanel">
> +      <hbox id="nameBox" align="center" class="input-container"
> +            style="display: grid; grid-template-columns: auto 1fr;">

Better not having inline styles since they can't be reused anywhere else, like we should do in the New Folder dialog.

Also, since you're converting this into a grid, do we still need the `input-container` class?

Comment 6

[Richard Marti (:Paenglab)]

Attachment: 1652015-folderPane-dialogs-themeable.patch

(In reply to Alessandro Castellani (:aleca) from comment #5)

Comment on attachment 9163015 [details] [diff] [review]
1652015-folderPane-dialogs-themeable.patch

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

This looks way better, thank you so much.

You didn't align the label and input into a grid in the Create New Folder
dialog.
It's not trivial if you think it shouldn't be done, but since we have that
structure for the Virtual Folder dialog, I think we should keep it
consistent.

Better do this in a new bug.

Better not having inline styles since they can't be reused anywhere else,
like we should do in the New Folder dialog.

Moved to the CSS file.

Also, since you're converting this into a grid, do we still need the
input-container class?

It's still needed.

Comment 7

[Richard Marti (:Paenglab)]

Comment on attachment 9163040 [details] [diff] [review]
1652015-folderPane-dialogs-themeable.patch

[Approval Request Comment]
User impact if declined: inconsistent dark theming
Testing completed (on c-c, etc.): soon on c-c
Risk to taking this patch (and alternatives if risky): low

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/ed34be31a526
Make the folder pane dialogs themeable. r=aleca

Comment 9

[Wayne Mery (:wsmwk)]

Comment on attachment 9163040 [details] [diff] [review]
1652015-folderPane-dialogs-themeable.patch

Approved for beta (the size gives me pause, but it's been on nightly several days)

Walt and Eckard, can you check the results of this when you smoketest beta?

Comment 10

[Wayne Mery (:wsmwk)]

Comment on attachment 9163040 [details] [diff] [review]
1652015-folderPane-dialogs-themeable.patch

Approved for esr78 - optimistically.  We'll want to closely assess the smoketesting  of beta 2

Comment 11

[Rob Lemley [:rjl]]

Thunderbird 79.0b2:
https://hg.mozilla.org/releases/comm-beta/rev/22408104ed05

Comment 12

[Rob Lemley [:rjl]]

Thunderbird 78.0.1:
https://hg.mozilla.org/releases/comm-esr78/rev/edeffab4b02c

Comment 13

[WaltS48 [:walts48]]

(In reply to Wayne Mery (:wsmwk) from comment #9)

Comment on attachment 9163040 [details] [diff] [review]
1652015-folderPane-dialogs-themeable.patch

Approved for beta (the size gives me pause, but it's been on nightly several
days)

Walt and Eckard, can you check the results of this when you smoketest beta?

Looked great in 79.0b2 and 78.0.1 on Ubuntu 18.04.4 LTS in my testing.

Comment 14

[Eckard Berberich]

(In reply to Wayne Mery (:wsmwk) from comment #9)

Comment on attachment 9163040 [details] [diff] [review]
1652015-folderPane-dialogs-themeable.patch

Approved for beta (the size gives me pause, but it's been on nightly several
days)

Walt and Eckard, can you check the results of this when you smoketest beta?

Looks good here on the Retina 4k screen of my iMac running macOS 10.14.6.