Closed Bug 1628497 Opened 11 months ago Closed 9 months ago

Improve Account Settings UI

Categories

(Thunderbird :: Theme, enhancement, P2)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 78.0

People

(Reporter: aleca, Assigned: Paenglab)

References

Details

(Keywords: ux-consistency)

Attachments

(4 files, 3 obsolete files)

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.

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

Flags: needinfo?(richard.marti)

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.

Flags: needinfo?(richard.marti)

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.

Flags: needinfo?(richard.marti)
Priority: -- → P2

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

Flags: needinfo?(richard.marti)

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

Attached patch 1628497-AM-UI-improve.patch (obsolete) — Splinter Review

(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.

Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #9150770 - Flags: review?(alessandro)
Attached patch 1628497-AM-UI-improve.patch (obsolete) — Splinter Review

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.

Attachment #9150770 - Attachment is obsolete: true
Attachment #9150770 - Flags: review?(alessandro)
Attachment #9150772 - Flags: review?(alessandro)
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.
Attachment #9150772 - Flags: review?(alessandro) → feedback+
Attached patch 1628497-AM-UI-improve.patch (obsolete) — Splinter Review

This should be the final patch.

Attachment #9150772 - Attachment is obsolete: true
Attachment #9150853 - Flags: review?(alessandro)
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.
Attachment #9150853 - Flags: ui-review+
Attachment #9150853 - Flags: review?(alessandro)
Attachment #9150853 - Flags: feedback+

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

Attachment #9150853 - Attachment is obsolete: true
Attachment #9151031 - Flags: review?(alessandro)
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.
Attachment #9151031 - Flags: review?(alessandro) → review+
Depends on: 1618616

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)

Attachment #9151113 - Flags: review?(richard.marti)
Attached image menulist.png
Comment on attachment 9151113 [details] [diff] [review]
1628497-xmpp-settings.diff

Looks good.
Attachment #9151113 - Flags: review?(richard.marti) → review+

Please land this patches after bug 1618616.

Target Milestone: --- → Thunderbird 78.0

Pushed by mkmelin@iki.fi:
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

Status: ASSIGNED → RESOLVED
Closed: 9 months ago
Resolution: --- → FIXED

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

(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.

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

Status: RESOLVED → REOPENED
Resolution: FIXED → ---

This fixes the HTML edit checkbox for me.

Attachment #9152454 - Flags: review?(mkmelin+mozilla)
Comment on attachment 9152454 [details] [diff] [review]
1628497-identityHtmlSigFormat.patch

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

Looks good, thx!
Attachment #9152454 - Flags: review?(mkmelin+mozilla) → review+

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

Status: REOPENED → RESOLVED
Closed: 9 months ago9 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.