1628497 - Improve Account Settings UI

Status: RESOLVED FIXED

(Thunderbird :: Theme, enhancement, P2)

Description

[Alessandro Castellani [:aleca]]

Now that the Account Settings lives in a tab, we need to give it a little bit more love and solve a couple of quirks to make it look and behave a little bit more closely to how we handle other pref tabs.

Here's an overview of the issue we should fix:

All Platforms

• Limit the right panel size like we do for the Preferences.
• Move Copies & Folder > Archive options... button underneath the menulists.
• Align the various global preferences buttons to the right:
  • Composition & Address > Global Addressing Preferences...
  • Junk Settings > Global Junk Preferences...
  • Feeds > Manage Subscriptions...
  • Newsgroup > Composition & Addressing
• XMPP account Connection security menulists should be inline to its label.
• XMPP account > End-to-end Encryption > Your Fingerprint: label is not centre aligned.
• Newsgroup Outgoing Server menulist should be inline to its label.
• Newsgroup > Server settings > Message Storage should have inline field and capitalized labels.
• Outgoing Server > Details of selected server should be better styled.

macOS

• All the buttons not inline other elements are too thin as it seems to padding block is present.

More generic improvements

• Vertical space between sections is inconsistent and some options are visually too attached vertically.

Sidebar Improvements

• We should style the tree to not show the focus when clicking on an empty space.
• Disable tree context menu when clicked on empty space.
• Add the darker grey background color to the entire columns.

Comment 1

[Alessandro Castellani [:aleca]]

Richard, what do you think about this overview of proposed changes?

Comment 2

[Richard Marti (:Paenglab)]

This sounds good and is necessary.
.
(In reply to Alessandro Castellani (:aleca) from comment #0)

macOS

• All the buttons not inline other elements are too thin as it seems to padding block is present.

This rule needs to be removed.

Comment 3

[Alessandro Castellani [:aleca]]

As we're getting close to 78, I think we should tackle these issue to improve visual consistency and polish this section.

Richard, is this something you might have some capacity to tackle?
Otherwise I can take it early next week.

Comment 4

[Richard Marti (:Paenglab)]

I'll take first the low hanging fruits, okay?

Comment 5

[Alessandro Castellani [:aleca]]

Sounds good.
We can divide and conquer this bug :D

Comment 6

[Richard Marti (:Paenglab)]

Attachment: 1628497-AM-UI-improve.patch

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

Here's an overview of the issue we should fix:

All Platforms

• Limit the right panel size like we do for the Preferences.

done. I had to add a second <vbox> to let the scrollbar at edge of the window. I added it to the same line to show it's only there for this purpose.

• Move Copies & Folder > Archive options... button underneath the menulists.

done

• Align the various global preferences buttons to the right:
  • Composition & Address > Global Addressing Preferences...
  • Junk Settings > Global Junk Preferences...
  • Feeds > Manage Subscriptions...
  • Newsgroup > Composition & Addressing

done

• XMPP account Connection security menulists should be inline to its label.

I have no XMPP and can't find it on a other protocol I have.

• XMPP account > End-to-end Encryption > Your Fingerprint: label is not centre aligned.

done, this exists also for IRC. I moved also the "Manage Fingerprints Of Contacts" button to the right.

• Newsgroup Outgoing Server menulist should be inline to its label.

done. This is for the IMAP/POP server page too.

• Newsgroup > Server settings > Message Storage should have inline field and capitalized labels.

done. I use now News.rc to show what type of file this should be. Okay?

• Outgoing Server > Details of selected server should be better styled.

I changed the background colour and added a border, better? Or what have you in mind?

macOS

• All the buttons not inline other elements are too thin as it seems to padding block is present.

done in a other bug.

More generic improvements

• Vertical space between sections is inconsistent and some options are visually too attached vertically.

I tried to fix them, okay?

Sidebar Improvements

• We should style the tree to not show the focus when clicking on an empty space.
• Disable tree context menu when clicked on empty space.

This are more things for you to to.

• Add the darker grey background color to the entire columns.

done, but it looks weird with the tree in it. I think, we should convert the tree to a richlist or something there we show only the main entries (Server). Only the active server shows its sub-entries expanded.

Comment 7

[Richard Marti (:Paenglab)]

Attachment: 1628497-AM-UI-improve.patch

Sorry, one change wasn't in the previous patch.

And when you see the "Manage Identities" button overlap the "Edit SMTP server" button, this is bug 1618616.

Comment 8

[Alessandro Castellani [:aleca]]

Comment on attachment 9150772 [details] [diff] [review]
1628497-AM-UI-improve.patch

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

Great start.
We're doing a full round of feedback and improvements on Matrix.

Comment 9

[Richard Marti (:Paenglab)]

Attachment: 1628497-AM-UI-improve.patch

This should be the final patch.

Comment 10

[Alessandro Castellani [:aleca]]

Comment on attachment 9150853 [details] [diff] [review]
1628497-AM-UI-improve.patch

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

Amazing work.
The UI updates are perfect.
Just a few code nits we should deal before an r+.

::: mail/components/im/content/am-im.xhtml
@@ +29,5 @@
>      <html:link rel="localization" href="messenger/preferences/am-im.ftl"/>
>      <html:link rel="localization" href="messenger/otr/am-im-otr.ftl"/>
>    </linkset>
>  
> +  <vbox flex="1" style="overflow: auto;"><vbox id="containerBox" flex="1">

Too bad we're dealing with these sections in an iframe and we can't use a single overflow for every section, even with the scrolling="auto" attribute it doesn't work, and we're forced to add this extra container for each file, which is not really scalable and a bit redundant unfortunately.

I guess this is good for now and we can explore some optimization later.

::: mail/themes/shared/mail/accountManage.css
@@ +231,5 @@
> +}
> +
> +#autosync\.notDownload {
> +  margin-inline-end: 12px;
> +}

If we're not interfering with any other section, all these IDs should be updated to remove the period from the selector.

::: mail/themes/shared/mail/preferences/dialog.css
@@ +118,5 @@
>  }
> +
> +/* Edit SMTP Server dialog */
> +#smtp\.username {
> +  margin-inline: 8px 0;

Same for this ID.

::: mailnews/base/prefs/content/am-server.xhtml
@@ +46,5 @@
>        <div>
>          <xul:label value="&serverType.label;"/>
>        </div>
>        <div>
>          <xul:label id="servertype.verbose"/>

If this ID is only used here, we should remove the period as it's really odd and forces us to escape in CSS.

Comment 11

[Richard Marti (:Paenglab)]

Attachment: 1628497-AM-UI-improve.patch

Updated the IDs. But only the ones I'm touching in CSS. There are too much and I don't want to introduce regressions.

Comment 12

[Alessandro Castellani [:aleca]]

Comment on attachment 9151031 [details] [diff] [review]
1628497-AM-UI-improve.patch

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

Perfect, thank you.
Hold on a few minutes on the checkin as I'm gonna quickly do a follow up patch on top of this to take care of the XMPP section.

Comment 13

[Alessandro Castellani [:aleca]]

Attachment: 1628497-xmpp-settings.diff

This patch sits on top of the one created by Richard and takes care of the menulist created in the am-im section. (screenshot coming)

Comment 14

[Alessandro Castellani [:aleca]]

Attachment: menulist.png

Comment 15

[Richard Marti (:Paenglab)]

Comment on attachment 9151113 [details] [diff] [review]
1628497-xmpp-settings.diff

Looks good.

Comment 16

[Richard Marti (:Paenglab)]

Please land this patches after bug 1618616.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/1ba42b85d43f
Improve Account Manager UI. r=aleca
https://hg.mozilla.org/comm-central/rev/1f54197e9163
Improve the UI of the XMPP account settings tab. r=paenglab

Comment 18

[Magnus Melin [:mkmelin]]

Ugh, while we don't like dots in ids, they must stay in the account manager until we fix it properly - removing the dot breaks the wsm_persist automatic save/restore which looks for "identity." https://searchfox.org/comm-central/rev/89c30dfce0f903bcab7e25e9a9f24f39988ef40c/mailnews/base/prefs/content/AccountManager.js#114

Comment 19

[Richard Marti (:Paenglab)]

(In reply to Magnus Melin [:mkmelin] from comment #18)

Ugh, while we don't like dots in ids, they must stay in the account manager until we fix it properly - removing the dot breaks the wsm_persist automatic save/restore which looks for "identity." https://searchfox.org/comm-central/rev/89c30dfce0f903bcab7e25e9a9f24f39988ef40c/mailnews/base/prefs/content/AccountManager.js#114

But our changes should be okay? I changed servertype.verbose, identity.htmlSigFormat, autosync.notDownload and smtp.username.

Comment 20

[Magnus Melin [:mkmelin]]

Unfortunately not. htmlSigFormat is broken (the checbox "Use HTML") on the main settings.

Comment 21

[Richard Marti (:Paenglab)]

Attachment: 1628497-identityHtmlSigFormat.patch

This fixes the HTML edit checkbox for me.

Comment 22

[Magnus Melin [:mkmelin]]

Comment on attachment 9152454 [details] [diff] [review]
1628497-identityHtmlSigFormat.patch

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

Looks good, thx!

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/53bd3a77386e
Revert to dot-ID for identityHtmlSigFormat to fix the "Use HTML" checkbox. r=mkmelin