1613758 - Update the Account Manager UI to match the Preferences Tab UI

Status: RESOLVED FIXED

(Thunderbird :: Theme, enhancement, P1)

Description

[Alessandro Castellani [:aleca]]

Now that bug 1610445, we finally have the Account Manager living in a tab.
With that said, it looks pretty ugly.

Let's update the UI to match what we did with the Preference Tab and keep everything consistent.

Some prototyping needs to happen for the sidebar in order to properly handle those submenu elements and the button at the bottom, but for the rest, we should stick to what we have in the preferences tab.

Comment 1

[Alessandro Castellani [:aleca]]

Attachment: account-settings-udpate.png

I think the easiest and immediate step we can take is to update the overall UI to match exactly what we did in the Preferences Tab.
Let's ignore for now the UX of the left sidebar, as we'll deal with it in a follow up bug.

Richard, if I'm not wrong you took care of the Preferences Tab UI, right?
Would you be able to work on this?

Comment 2

[Richard Marti (:Paenglab)]

Attachment: 1613758-accountManage-in-content-styles.patch

I was working hard on Friday for this. ;-)

Magnus, we are touching now many files in mailnews which would break SM. And future changes, for example making the dialogs subdialogs like in prefs, would break more. Wouldn't it be better when we fork the files, like we did with the editor files, to not use many #ifdef? If yes, where do you think should we move them, components/accountmanager or components/preferences?

Comment 3

[Richard Marti (:Paenglab)]

Attachment: new-account-manager.png

How it would look with the patch. No fine tuning done on reorganising the UI.

Comment 4

[Richard Marti (:Paenglab)]

Attachment: 1613758-accountManage-in-content-styles.patch

Sorry forgot to qrefresh before uploading.

Again to Magnus:
Magnus, we are touching now many files in mailnews which would break SM. And future changes, for example making the dialogs subdialogs like in prefs, would break more. Wouldn't it be better when we fork the files, like we did with the editor files, to not use many #ifdef? If yes, where do you think should we move them, components/accountmanager or components/preferences?

Comment 5

[Magnus Melin [:mkmelin]]

Comment on attachment 9125266 [details] [diff] [review]
1613758-accountManage-in-content-styles.patch

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

Local directory is too narrow, but other than that, looks much better than what we have.

Re SeaMonkey, true, some of this will kind of break them even further. But it's all theoretical of course, since their UI didn't start up at all for the last 2 years. What are the chances it will ever work again? For editor the case was slightly different in that it actually had a bunch of files we didn't use at all.

Comment 6

[Richard Marti (:Paenglab)]

Attachment: 1613758-accountManage-in-content-styles.patch

Fixed the Local directory field width.

Comment 7

[Alessandro Castellani [:aleca]]

Comment on attachment 9125270 [details] [diff] [review]
1613758-accountManage-in-content-styles.patch

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

Thank you so much for taking care of this, this is a bit of beast.
There are a lot of visual inconsistencies in terms of spacing, font-size, font-weight, alignments, etc, and many things don't work, like some dialogs, and some input fields are not rendered properly.

As we fine tuned the Preferences panel pretty well, we should stick to that exact same style without variation.

So, my question is, how should we approach this?
I don't think we should duplicate the CSS styling, copying those attributes from the pref panel and add them to the classes of the account settings.
I think we should update these class names to match what we're using in the preferences panel, in order to prevent code duplication.

What do you think?
Also, do you want to split the patch to deal with a general style, and then tackle each panel individually in order to land this progressively and leave this bug open?

::: mail/themes/shared/mail/accountManage.css
@@ +10,5 @@
> +@import url("chrome://global/skin/in-content/common.css");
> +@import url("chrome://messenger/skin/preferences/preferences.css");
> +
> +window {
> +  font-size: 1.05em !important;

Adding this attribute to the window blows up the text and makes it look almost doubled.
I think it's a mix of HiDPI and relative values of internal sizing.

@@ +15,5 @@
> +}
> +
> +window > vbox {
> +  padding-block: 40px;
> +  padding-inline: 25px 28px;

Adding the font-size attribute here makes it look normal and properly scaled.
Also, the value used in the preferences panel is 1.11em.

@@ +82,5 @@
>  
> +.header {
> +  font-size: 1.05em;
> +  margin-block: 16px 4px;
> +  padding-bottom: 0;

This should have a font-weight: 600;

@@ +181,3 @@
>  .dialogheader-title {
> +  margin-block: 0 8px;
> +  margin-inline-start: 6px;

This causes the titles to not be properly aligned to the left compared to the rest of the content.

Comment 8

[Richard Marti (:Paenglab)]

Attachment: 1613758-accountManage-in-content-styles.patch

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

Comment on attachment 9125270 [details] [diff] [review]
1613758-accountManage-in-content-styles.patch

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

Thank you so much for taking care of this, this is a bit of beast.
There are a lot of visual inconsistencies in terms of spacing, font-size,
font-weight, alignments, etc, and many things don't work, like some dialogs,
and some input fields are not rendered properly.

Correct, the font sizes aren't correct. The Font sizes/weights should now be correct. The spacing of the elements too, but it looks still different because the elements aren't the same positioned, like between the title is a input field and then the groupbox. Where is one dialog that looks a bit weird, the advanced server settings. The edit identities dialog completely uses the in-content styling. They should be fixed when they are sub dialogs.

As we fine tuned the Preferences panel pretty well, we should stick to that
exact same style without variation.

So, my question is, how should we approach this?
I don't think we should duplicate the CSS styling, copying those attributes
from the pref panel and add them to the classes of the account settings.
I think we should update these class names to match what we're using in the
preferences panel, in order to prevent code duplication.

What do you think?

I think the actual patch is good for landing to get the same styling as the prefs. With follow-up bug we can implement the sub dialogs (I have already a patch with the simplest dialogs) and further improvements of the remaining thing, like rearranging elements.

Also, do you want to split the patch to deal with a general style, and then
tackle each panel individually in order to land this progressively and leave
this bug open?

We could use this bug for the initial landing and then use follow-up bugs. This could be better for the history.

::: mail/themes/shared/mail/accountManage.css
@@ +10,5 @@

+@import url("chrome://global/skin/in-content/common.css");
+@import url("chrome://messenger/skin/preferences/preferences.css");
+
+window {

• font-size: 1.05em !important;

Yes, on Windows it looked good. But I copied now the prefs approach which use now the exact same font metrics.

Adding this attribute to the window blows up the text and makes it look
almost doubled.
I think it's a mix of HiDPI and relative values of internal sizing.

Should be fixed.

@@ +82,5 @@

+.header {

• font-size: 1.05em;
• margin-block: 16px 4px;
• padding-bottom: 0;

This should have a font-weight: 600;

Fixed

@@ +181,3 @@

.dialogheader-title {

• margin-block: 0 8px;
• margin-inline-start: 6px;

This causes the titles to not be properly aligned to the left compared to
the rest of the content.

Fixed

Comment 9

[Richard Marti (:Paenglab)]

Attachment: 1613758-accountManage-in-content-styles.patch

Now the correct patch. Sorry.

Comment 10

[Alessandro Castellani [:aleca]]

Comment on attachment 9125907 [details] [diff] [review]
1613758-accountManage-in-content-styles.patch

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

Awesome, this is already a million times better. r+
The commit message is not clear, can you update it to:
`Bug 1613758 - Update the Account Manager in-tab UI to match the Preferences Tab UI. r=aleca`

Comment 11

[Richard Marti (:Paenglab)]

Attachment: 1613758-accountManage-in-content-styles.patch

Fixed the commit message. Didn't copied the spell error in your proposal. ;-)

Comment 12

[Alessandro Castellani [:aleca]]

Sweet, thanks.
Now we need to decide on some follow ups. The most obvious I can think of are:

• Fix dialogs UI.
• Improve elements (labels, inputs, checkboxes, etc) alignment.
• Improve sidebar UI.
• Improve Account Actions button UI.

Anything I'm missing?

Later on, after I finally manage to finish bug 1573678, we could implement a search field also in here.

Comment 13

[Richard Marti (:Paenglab)]

I think, that's it. A main problem is, that it isn't one page, it uses a iframe and loads different pages in it.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/992826d12a7e
Update the Account Manager in Tab UI to match the Preferences Tab UI. r=aleca

Comment 15

[Alex Ihrig [:Thunderbird_Mail_DE]]

Attachment: Zoomed text should increase the whole overlay size, but doesn't

When testing the daily build and following the cross-link from account settings to MDN settings, I noticed that, firstly, not all subordinate dialogs have been updated yet and secondly (see Global Settings → MDN) do not scale with the zoom (the text yes, but not the pseudo window overlay size)

Comment 16

[Alessandro Castellani [:aleca]]

Thanks for the heads up, but as written in Comment 12, this is only the first step of many others to properly polish this section.

Comment 17

[Richard Marti (:Paenglab)]

This isn't a issue of this patch. This happens with the prefs (your screenshot) too. When you set browser.zoom.full it works as expected.

Comment 18

[Alex Ihrig [:Thunderbird_Mail_DE]]

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

Thanks for the heads up, but as written in Comment 12, this is only the first step of many others to properly polish this section.

No problem, if you're aware that the overlays doesn't scale properly yet.

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

Later on, after I finally manage to finish bug 1573678, we could implement a search field also in here.

This would be great.

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

This isn't a issue of this patch. This happens with the prefs (your screenshot) too. When you set browser.zoom.full it works as expected.

Will this be enabled for standard users automatically?

Comment 19

[Richard Marti (:Paenglab)]

Can't say now. This needs a new bug where this can be decided.

Comment 20

[Alex Ihrig [:Thunderbird_Mail_DE]]

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

This isn't a issue of this patch. This happens with the prefs (your screenshot) too. When you set browser.zoom.full it works as expected.

I don't know the other effects of the option, but based on the topic here, the option should probably be set to true by default.

Maybe someone could create the spin off bug.

Comment 21

[Wayne Mery (:wsmwk)]

Comment on attachment 9125926 [details] [diff] [review]
1613758-accountManage-in-content-styles.patch

[Triage Comment] Yes, let's keep the UI feedback train chugging

Comment 22

[Rob Lemley [:rjl]]

Thunderbird 74.0b2:
https://hg.mozilla.org/releases/comm-beta/rev/af9061aaf712