1589005 - implement account hub for a centralized way to set up mail, calendar, filelink, online addressbook, and potentially other services

Status: VERIFIED FIXED

(Thunderbird :: Account Manager, enhancement, P1)

Description

[Magnus Melin [:mkmelin]]

Bug 1551133 had the start of the account hub, but wasn't included back then. Filing this bug to move on with that work.

Comment 1

[Matt Weatherford]

+1

This would be a great addition to Thunderbird
Even if it just handled setup for Gmail accounts & set up address book sync, it would be a huge improvement

Comment 2

[Alessandro Castellani [:aleca]]

Attachment: account-hub.png

Attaching a screenshot of the original account hub first screen as a reference.
Most likely some further iterations and prototyping will be necessary.

Comment 3

[Matt Weatherford]

Looks like a good start ! Will there be the possibility of templates for various email providers like gmail, O365, pobox, zwrob, etc ?
If so, I hope the templates can go beyond just the basic mail server settings and do things like set the Sent mail folder, sync policy, etc to work best with whatever that provider's "optimal" (I know, its complicated, right?) settings should be. Im thinking about gmail mostly here.

Another thing to consider, especially with gmail, is that not all gmail addresses end in @gmail.com - I have an apps for education account that looks like this: [e-mail] - but it is a gmail account - the regular autodetection settings dont work properly for this kind of email format. If I could first choose "I have a google account" then put in my email as "[e-mail]" then I think it would be able to be mostly set up properly, althought my university also wishes us to use the local smtp.wa.edu server rather than the google one (I dont expect that to be autodetected - but I think you can see how this gets complicated) The autoconfig stuff that was done with the DNS record a while back was a great start to sorting this out, but has its own limitations such as the case I just described.
Appreciate the work on this feature!

Comment 4

[Magnus Melin [:mkmelin]]

Which folder is Sent (and such) is found out through IMAP.
I don't think we want templates like that, those would be thousands. Giving the email address already finds you the right one. It should even for hosted domains if they are set up correctly (bug 505267), though we do have bug 1494219 to fix. Either way, the scope of this bug is more of bringing what we have together than implementing account type specific improvements.

Comment 5

[Matt Weatherford]

The email address cannot find the correct server in the case of the University of Washington because we use an internal mail delivery mechanism to route "[e-mail] " to either Apps for EDU gmail or Microsoft Outlook 365 - there is no way to know which server the mail is delivered to from the email address. That is an internal routing by way of user preference. Perhaps we are a rare case, but I suspect there are others.

Comment 6

[Ben Bucksch (:BenB)]

internal routing by way of user preference. Perhaps we are a rare case

Yes, this is very rare, to use both Gmail and Office365 for the same domain. But we already have a solution even for this very rare case: https://developer.mozilla.org/en-US/docs/Mozilla/Thunderbird/Autoconfiguration#Configuration_server_at_ISP . Given that the University is the only one that can tell the difference, this is the only solution. I do know that UW once wrote their own mail server, so they should be able to whip up a small Python script that gives the right config for a given user :-) .

In any case, such questions are offtopic here in this bug. You can direct them to me per email.

Comment 7

[Alessandro Castellani [:aleca]]

Attachment: 1589005-account-hub.patch

This is a first WIP patch to implement the Account Hub as I was mostly focused on style rather than functionality.
I temporarily added a "Open account hub" link in the Account Central view to trigger it.

Before continuing, it's important to define proper objectives and expectations for this section.
Here's a series of question to define this implementation.

• Are we going to trigger this dialog on startup instead of the emailWizard dialog?
• What is the list of accounts we want to highlight? (Email, Calendar, FileLink, etc..)
• What is the list of accounts we want to put inside the "Others" tab? (Newsgroups, movemail, etc..)
• Do we want to define some unique IDs in order to offer add-ons developers easy access to these sections to add their own creation buttons?
• How do we want to update the Account Central view in the "Setup and Account:" section? Should we leave all those separate links to the various accounts or should we force users to always go through the Account Hub? (This might happen as a follow up on bug 1559867, here's an old mock-up: https://bug1559867.bmoattachments.org/attachment.cgi?id=9074640)
• Are we missing something important?

Comment 8

[Richard Marti (:Paenglab)]

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

• Are we going to trigger this dialog on startup instead of the emailWizard dialog?

I think, this would make sense.

• What is the list of accounts we want to highlight? (Email, Calendar, FileLink, etc..)

I think, the normal accounts (Email, Calendar, Chat, Feed, News, movemail) Not sure about FileLink as it isn't a normal account, it's only a helper for the other accounts.

• What is the list of accounts we want to put inside the "Others" tab? (Newsgroups, movemail, etc..)

News, movemail and eventually FileLink

• Do we want to define some unique IDs in order to offer add-ons developers easy access to these sections to add their own creation buttons?

Are there other account types or only sub types of the main accounts. The sub types should have their place in the associated account dialogs.

• How do we want to update the Account Central view in the "Setup and Account:" section? Should we leave all those separate links to the various accounts or should we force users to always go through the Account Hub? (This might happen as a follow up on bug 1559867, here's an old mock-up: https://bug1559867.bmoattachments.org/attachment.cgi?id=9074640)

What is about Mail Summaries (bug 492158 and bug 489999)? This would also affect this view. This needs a complete rethinking.

Comment 9

[Magnus Melin [:mkmelin]]

(mid-aired, but submitting anway)

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

• Are we going to trigger this dialog on startup instead of the emailWizard dialog?

I think so.

• What is the list of accounts we want to highlight? (Email, Calendar, FileLink, etc..)

Email, Calendar, Address Book, Chat, Filelink, maybe Feeds

• What is the list of accounts we want to put inside the "Others" tab? (Newsgroups, movemail, etc..)

Newsgroups, Movemail. I'm not sure Feeds should be here or above.

• Do we want to define some unique IDs in order to offer add-ons developers easy access to these sections to add their own creation buttons?

Ids are good, but won't help add-ons much anymore. Longer term we might want to have some api to add things.

• How do we want to update the Account Central view in the "Setup and Account:" section? Should we leave all those separate links to the various accounts or should we force users to always go through the Account Hub?

As we discussed earlier today, the current account central is pretty crude and pointless. There's been a post on tb-planning re integrating the mail summaries add-on, which would take over at least part of this space.
Anyway, could be useful to have the account central either as an integrated part of that page, or as a dedicated tab, so that there are not that many dialogs popping around the screen.

Comment 10

[Magnus Melin [:mkmelin]]

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

I think, the normal accounts (Email, Calendar, Chat, Feed, News, movemail) Not sure about FileLink as it isn't a normal account, it's only a helper for the other accounts.

I'd like to have filelink there so you have a flow where you set thunderbird up all at once. It's also a request by potential partners, and it's good for organizations where you need to use their specific solution for your files.

One thing to have separate might also be Tasks. Some calendars support tasks, but many don't - e.g. google do not, fastmail stopped and have now separated them into a separate "calendar" for tasks. We might want to defer this one to a later time though....

Comment 11

[Alessandro Castellani [:aleca]]

Based on our previous discussion, I'll take some time to mock-up how this implementation might look into a better organized Account Central, in order to move away from implementing another popup.
Stay tuned.

Comment 12

[Wayne Mery (:wsmwk)]

My 2cents - I'd rather us not continue displaying an "Other" secondary category, i.e. two tiers, regardless of whether some are heavily used vs others not heavily used. Doing this we end up displaying more items, but IMO it reduces complexity and support (where do I find X?) questions.

Comment 13

[Alessandro Castellani [:aleca]]

Attachment: Account Central - Initial Setup.png

Here's a couple of mock-ups to prototype how it could look having an Account Hub inside the first tab.

This first screen shows how it might look when a user opens Thunderbird for the first time and doesn't have any account yet.
I was thinking maybe the "accounts area" can be a flexbox or css-grid, which will allow use to add as many account types as we want, and automatically stack those containers based on the client's width.
I added a little bit of UI polishing to put an emphasis on our brand identity, and some important links at the bottom.

Comment 14

[Alessandro Castellani [:aleca]]

Attachment: Account Central - Existing Account.png

This second mock-up shows how it might look once the use access the Account Central with an already set up account.
I added some blank placeholders to represent where those graphs and info from the account summary patch might end up.

The Account Hub is always present to offer a quick access to a new configuration, but of course is pushed to the bottom.
We could also try to change those "account containers" to simple links or making them smaller to save space.

Comment 15

[Richard Marti (:Paenglab)]

Comment on attachment 9124413 [details]
Account Central - Initial Setup.png

Looks very good. An improvement to the actual state.

Comment 16

[Richard Marti (:Paenglab)]

Comment on attachment 9124415 [details]
Account Central - Existing Account.png

And this too. :-)

Comment 17

[Magnus Melin [:mkmelin]]

Comment on attachment 9124413 [details]
Account Central - Initial Setup.png

Looks pretty good, though the content in the footer needs some adjustments. 
This could be a good place to tell the Thunderbird story, and explain a bit what it is. 

But this is not the "first run" since you have an account set up already. Could it take up all the screen until anything is set up?

Comment 18

[Magnus Melin [:mkmelin]]

Comment on attachment 9124415 [details]
Account Central - Existing Account.png

Since this is the account summary for a given account, maybe it should be more clearly separated that the content is now describing two/three things: this account, further setups, and general things.

Comment 19

[Alessandro Castellani [:aleca]]

Thanks both for the positive feedback.

This could be a good place to tell the Thunderbird story, and explain a bit what it is.

For sure.

But this is not the "first run" since you have an account set up already.

Sorry for the confusing mock-up, but I took the screenshot from my installation. The folder pane should be empty on the "Initial Setup" image.

Could it take up all the screen until anything is set up?

I think it can. The Today Pane won't be visible anyway, and I think we can hide the folder pane until at least an account is present.
I'll update the mock-up to see how it looks.

Since this is the account summary for a given account, maybe it should be more clearly separated that the content is now describing two/three things: this account, further setups, and general things.

I see what you mean. I'll try to update it a little bit to create a better visual separation, but I'd like to keep this section light and easy on the eyes, without using too many dividers as this area will be pretty full of info and I'd like to avoid being too busy.

I'll work on these things and upload a new mock-up in a bit.

Comment 20

[Alessandro Castellani [:aleca]]

Pinging Ryan to get him into the loop.

Comment 21

[Ryan Lee Sipes]

I love this mock-up.

Everything is perfect and I very, very much like the idea of Email patterns. I proposed "Insights" last year - I think, to show users information about how they use their email that may help them gain an understanding of how they can manage their email.

I would remove Space Usage and rename Email patterns to "Insights" and then gather up different, helpful, information there that users might find interesting. It could be like this: https://bug489999.bmoattachments.org/attachment.cgi?id=386242

Not sure what else I can add here, I would change some of the content under about (I think "Read the Blog" and "Donate" make sense), not sure about the other two.

But the most important thing is the concept, and I think Alessandro is right on with this page. It is exactly the style and layout that I think will best serve our users and it is very beautiful.

Comment 22

[Alessandro Castellani [:aleca]]

Attachment: Account Central - Initial Setup.png

Here's an updated mock-up to implement the account hub as a first tab when users open TB for the first time.
With this implementation, no popups will be opened and the user can decide what to do (and we can guide them better).

I guess I could start working on this implementation while we decide which links and content we want to add at the bottom.

Comment 23

[Alessandro Castellani [:aleca]]

Attachment: Account Central - Existing Account.png

This is a quick update for the second screen, increasing a bit the visual separation between sections, and reducing the visual importance of the account setup buttons.

What type of content would we want to list i the account summary?
What type of insights?
Other important data the user might find useful?

The more specific we get in this early stage, the quick we will be able to implement it.

Comment 24

[Richard Marti (:Paenglab)]

Comment on attachment 9124943 [details]
Account Central - Initial Setup.png

Looks good, thanks.

I'm actually not in front of a TB and can't say if the mock-up shows the real toolbarbutton activation. Maybe we need to look at the toolbar and menu to disable all action which makes no sense before a account is configured. Now the dialog is modal and this prevents the user to do things on the main window.

Comment 25

[Richard Marti (:Paenglab)]

Comment on attachment 9124947 [details]
Account Central - Existing Account.png

Looks good too.

Comment 26

[Magnus Melin [:mkmelin]]

I think the footer should still be cut down significantly. If a link isn't likely to be relevant for many people it shouldn't be there - like the blog.
Maybe two columns would be enough? The things under Explore Features seem rather unnecessary.

== About Thunderbird ==
[The text from the current start page related to that]

== Resources ==

• Explore features - https://www.thunderbird.net/en-US/features/
• Support - https://support.mozilla.org/products/thunderbird
• Get involved - https://www.thunderbird.net/en-US/get-involved/
• Developer documentation - https://developer.thunderbird.net/

Comment 27

[Magnus Melin [:mkmelin]]

Comment on attachment 9124943 [details]
Account Central - Initial Setup.png

Not sure what to have in the footer for the first run - it might be different. Potentially also partner offerings?

This page should also have more text to explain to people what Tunderbird is. People who are not technical and come to set it up often don't understand. 

Importing should be given some importance here (though we need to fix the feature a bit). Chances are you already had Thunderbird set up elsewhere and would like to import your stuff. This seems like the place to offer that.

Beneath the buttons, when one is selected, there could also be some explaining/clarifying/whatever details appearing.

Also a good place to lower down on the page tell the Thunderbird story. Maybe Ryan wants to put together some text?

Looks good, but I think I liked the wider buttons better.

Comment 28

[Magnus Melin [:mkmelin]]

Comment on attachment 9124947 [details]
Account Central - Existing Account.png

Looks good, but the big question is what the two upper large areas would contain....

I think in this age, people don't care too much about space usage since so many providers give essentially unlimited storage.
Where space usage is, there could be something like a Today, where you'd have an agenda view (similar to Android calendar agenda view), maybe mixed with some other things - but exactly what those things would be we need to figure out.

Comment 29

[Wayne Mery (:wsmwk)]

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

Comment on attachment 9124947 [details]
Account Central - Existing Account.png

Looks good, but the big question is what the two upper large areas would
contain....

Account Summaries add-on like information?

I think in this age, people don't care too much about space usage since so
many providers give essentially unlimited storage.

For the big providers, yes. But not everyone likes the big providers, and high percentage of our users are corporate who may not have unlimited storage. Plus, there are reasons other than purely disk capacity to want to manage how much space is used, performance for example.

Comment 30

[Wayne Mery (:wsmwk)]

I feel like we should get some feedback from our support people on the front lines in further iterations.

I don't have too much concern about presenting too much information - different classes of users have different and varying interests, they can choose for themselves what is important. So I'd rather see us err on the side of presenting too much than not enough.

But I having trouble deciding how the initial setup should be different from someone who has an existing account. I have a couple major concerns:

1. a user who successfully does the initial setup quickly now sees the "has an existing account" variant, even though he/she is still a novice user
2. to what extent should or does this replace/augment our existing start page?

Comment 31

[Alessandro Castellani [:aleca]]

Attachment: Account Central - Initial Setup 2.png

Exploring some ideas based on feedback.
Let's focus on one screen at a time in order to nail it and start coding it.
Which intro text we want to put here? Same as the start page we show in the message pane?
Which links we should put underneath?
Anything else we could use this space for?

Comment 32

[Richard Marti (:Paenglab)]

Comment on attachment 9125991 [details]
Account Central - Initial Setup 2.png

I think, this good as it is. We shouldn't put too much into this screen to not overcharge the user when he is starting with TB.

Comment 33

[Wayne Mery (:wsmwk)]

I like this iteration - including the removal of reporting a bug.
(Generally speaking, reporting a bug should happen only after a user first goes through support - especially a new user)

Comment 34

[Magnus Melin [:mkmelin]]

Comment on attachment 9125991 [details]
Account Central - Initial Setup 2.png

Yes looks good. Perhaps the lower part(s) should have more of a separation?

I think below the line of account types, there could be an area that describes, e.g. for Email, that explains. Like
"Thunderbird lets you connect to your existing mail account, to read your emails conveniently and efficiently from with the application."

The Movemail box we can remove. (And actually seems I'm not able to set up such accounts on trunk, wonder what happened to that?)

Comment 35

[Ben Bucksch (:BenB)]

Aleca, this looks nice.

But as a user, I wouldn't know what to do when I see this screen. The "Email" button looks like a tab. Given that it's blue, it seems like it's already selected. So, what should I do, given that email is already selected? I would expect the email setup below. But it says "About Thunderbird", nothing to do with email. That's confusing me. I wouldn't know where to click. Obviously, I cannot click on a button that's already selected/pressed. If you intended these a push buttons, remove the blue background, and make the border look more like push buttons, so that I know to click on it.

Also, add a line right above that says "Set up account:". (The sub-title of the dialog already says that, but I didn't see that, I skimmed only "Welcome" and "Email".) The words should not be capitalized.

After first launch, we should show this page, but we should nonetheless start the email setup wizard, showing above this page. The user should set up an email account first. Then afterwards, he can add calendar etc.. The user installs Thunderbird to read his email, so we should streamline the email setup. Once that's done, this account hub will help the user discover that Thunderbird can do more than email.

Movemail box we can remove

Strongly agreed!

(We should remove the feature altogether. Nobody uses this anymore.)

Comment 36

[Matt Weatherford]

A suggestion - if there is are one or more "autoconfigurations" available (via DNS or local XML file or ?) This screen could offer it up: "One or more autoconfigurations are available - would you like to preview those settings?"

https://developer.mozilla.org/en-US/docs/Mozilla/Thunderbird/Autoconfiguration

Comment 37

[Ben Bucksch (:BenB)]

Autoconfig requires the user to enter his email address (we cannot know it) and his password (we need it to set up the account and sometimes even just to find a working configuration). In the account hub, we don't have this info.

Comment 38

[Alessandro Castellani [:aleca]]

(In reply to Ben Bucksch (:BenB) from comment #35)

Aleca, this looks nice.

Thanks :D

But as a user, I wouldn't know what to do when I see this screen. The "Email" button looks like a tab. Given that it's blue, it seems like it's already selected.

Apologies for the confusion, that's a representation of a hovered/clicked state, just to show the colour variation. The normal state would be like all the other buttons.

After first launch, we should show this page, but we should nonetheless start the email setup wizard

Sure, for now we will keep the emailWizard dialog auto-triggering on first launch.

Movemail, we should remove the feature altogether. Nobody uses this anymore.

I won't include it from this screen for sure. maybe we should have a dedicated bug to strip away all the related functions?

Comment 39

[Alessandro Castellani [:aleca]]

Attachment: Account Central - Initial Setup.png

This is how it should look on first startup, without hover effect or any other interaction happening.

Comment 40

[Alessandro Castellani [:aleca]]

Attachment: Account Central - Initial Setup - HOVER.png

This would be the state when the user hovers over one of those buttons.
In this state we could show a custom tooltip with an introduction/explanation related to the hovered option.

Comment 41

[Charles]

(In reply to Ben Bucksch (:BenB) from comment #37)

Autoconfig requires the user to enter his email address (we cannot know it) and his password (we need it to set up the account and sometimes even just to find a working configuration). In the account hub, we don't have this info.

I'm virtually certain that simply determining the settings (server name, security settings (STARTTLS vs SSL)etc absolutely does NOT require the password.

This has always really bugged me. It should be able to determine the settings, and never FORCE me to enter my password in order to set up the account.

I would very much like to see this password requirement removed.

Comment 42

[Ryan Lee Sipes]

Comment on attachment 9124947 [details]
Account Central - Existing Account.png

Love it. Still not keen on the "Space Usage" bit - but the rest of it is perfect.

Comment 43

[Ryan Lee Sipes]

Space usage could be a part of the "Insights", but doesn't need its own standalone column IMO.

Comment 44

[Ben Bucksch (:BenB)]

Comment on attachment 9127710 [details]
Account Central - Initial Setup.png

I like it!

Comment 45

[Ben Bucksch (:BenB)]

Comment on attachment 9124947 [details]
Account Central - Existing Account.png

I think the following existing buttons have some value when viewing an existing account:
[View settings for this account...]
[Filters...]
I think we should keep those two.

Comment 46

[Ben Bucksch (:BenB)]

not ... the "Space Usage"

Agreed. There's a lot of things you can do with "Insights", so keep the space free for it.

Comment 47

[Alessandro Castellani [:aleca]]

I almost complete the first part of this patch, but I'm having some issues with the dark theme on Linux.
Richard, do you know why all these attributes are completely ignored?

:root[lwt-default-theme-in-dark-mode]
:root[lwt-tree-brighttext]
:root[lwthemetextcolor="bright"]
:root[lwt-tree]

I'm trying to update the CSS variables I defined in the :root element.
Is this possible? Am I doing something wrong?

Comment 48

[Richard Marti (:Paenglab)]

[lwt-default-theme-in-dark-mode] is a Mac/Windows only thing, except you create the ui.systemUsesDarkTheme pref. Linux has since long time dark theme support as it uses the system colours natively. Mac/Windows don^t get the dark colours natively (except Windows High Contrast). That's also why on Mac with dark system theme the buttons/menulists/textfields are still bright.

The other attributes should be honoured when they are set by the TB theme.

Maybe you need the @media (prefers-color-scheme: dark) {} like we use in the account central. But this has the issue to not follow when the user has set the light TB theme with a dark system theme.

Comment 49

[Alessandro Castellani [:aleca]]

Another question regarding the written copy.
How do we want to handle the "About Thunderbird" section?
Should that be part of a fluent file, or should we load it from a static file like we're doing for the welcome pane?

If we add that in fluent, we're gonna be tied to the translation cycle and we won't be able to quickly update the text or fix typos without releasing a new version of TB.
But if we dynamically fetch it from somewhere else, we will need to handle translations on our own and load the proper file.
Thoughts?

Comment 50

[Alessandro Castellani [:aleca]]

Attachment: 1589005-account-hub.diff

Here's a first WIP patch to sync up on the progress and define some directions.

I implemented a separated vbox container for this new style, which will be hidden after the first setup happens. This gives us the flexibility to land small portions of this implementation without breaking what we have.
The Folder Pane is also hidden by default.
I'm currently working on implementing a refresh of the UI once an email account is set, so be aware that it's a bit broken for now.

Tooltip Text
Right now I implemented the tooltip for the email button.
Do we want to continue with this approach or would be good to explore something more custom like I did in the mock-up?

Account Buttons
For now, all those buttons trigger the dedicated dialogs, except for FileLink which opens the Pref tab at the right section.
The Address Book is the worst one, since that dialog is pretty useless as it is, but I'd like to tackle it in a follow up bug, giving the user more meaningful options.

About Section
As per comment 49, how do we want to handle this big chunk of text?

Test it also with the dark theme enabled if you can.

Comment 51

[Ben Bucksch (:BenB)]

I would strongly suggest to put all strings into the client. Reasons:

• It avoids unnecessary server pings, including tracking. Our users are very privacy conscious and prefer a completely local client.
• The text will change rarely and there's no point to have 20 million people constantly pull the information from the server every day. It's extremely wasteful.
• It's more reliable to have it on the client. If the client has no Internet (we do have an offline mode), things still work normally.
• It's easier to have all translations for the client in the same place, rather than split over several systems.

Comment 52

[Magnus Melin [:mkmelin]]

Comment on attachment 9128989 [details] [diff] [review]
1589005-account-hub.diff

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

Yeah put the text into Fluent. We might not want to change it much from what's on the web now: the wording has a balanced urgency. 
I like the text under the button row.

Let's also just do this page as xhtml to start with so we don't have to adjust later.

::: mailnews/base/content/msgAccountCentral.js
@@ +66,5 @@
> +
> +      document.getElementById("version").textContent = Services.appinfo.version;
> +
> +      if (!AppConstants.NIGHTLY_BUILD) {
> +        // Show a release notes link if we have a URL.

Better not to hide for nightly. It would actually be nice to have a "real" page for nightlies, maybe to begin with just a page listing what landed in the last day(s).

@@ +305,5 @@
> + *
> + * @param {string} id - The ID of the element to find.
> + * @param {boolena} collapse - If the element should be collapsed.
> + */
> +function toggleItemCollapse(id, collapse) {

Let's not create more unhelpful helper functions, I'd like to remove more of what we have too.
You can just use element.hidden = true or false.

Comment 53

[Richard Marti (:Paenglab)]

Comment on attachment 9128989 [details] [diff] [review]
1589005-account-hub.diff

Yes, the text should be local.

About the tooltip text, I think it's good when it describes for what the button is. It's mostly an explanation for new users.

Could you try to make the Account buttons the same width. When the window narrow and the buttons have to wrap it looks not well aligned. Also it could be that with some locales where are big differences of the width. Maybe let the button text wrap inside the button.

Could you also try a "glow" of the button outline when focused like we have on in-content buttons?

Comment 54

[Alessandro Castellani [:aleca]]

Attachment: 1589005-account-hub.diff

Another day, another round of feedback.
All the content, icons, and images are finally in the page.
I still need to finish the update of this page once the user created an account, but it's almost there.

Text
First of all, please give a thorough review and all the strings and descriptions I wrote in order to polish them and fix possible typos.

Button label overflow
Richard, I can't seem to be able to force a word wrap on those labels, would you be able to give it a look?

HTML
I used semantic HTML5 pretty much everywhere in the new section, but left the XUL in the old section, which I will update later anyway.

Panel
I'm using the XUL panel to reveal descriptions on mouseover. I was going to create a custom element, but this approach feels easier, and we inherit the auto-flip if the popup is too long and ends up outside the available screen size.

make a donation link
Adding this was a bit weird as fluent doesn't allow to write the <a> HTML tag inside a string, so I had to create that elements in JS and append it to the paragraph.
I can't seem to be able to allow keyboard focus on this newly created element tho, anyone has any insight?

Comment 55

[Richard Marti (:Paenglab)]

Comment on attachment 9129670 [details] [diff] [review]
1589005-account-hub.diff

I like it.

Comment 56

[Richard Marti (:Paenglab)]

Attachment: 1589005-account-hub-toolbarbutton.diff

This is your patch with a rough change from <button> to <toolbarbutton> because they have multiline support. I only added to the Address book button the attribute wrap="true" to show how the multiline looks. I don't know if we need to add it in the xhtml or this should be added in JS.

Maybe the buttons need to be a bit wider to look better with the "Newsgroups" label which can't wrap.

Also the watermark is very faint with the dark theme.

Comment 57

[Alessandro Castellani [:aleca]]

Attachment: 1589005-account-hub.diff

All right, another update on this.
I implemented the refresh of the UI after an account is configured in order to show the folder pane and update the account manager to the old UI.

I'm asking an early review, which I'm sure it will mostly be negative or cancelled, as I need to be sure the code approach and UI look good before start taking care of the little details.
Also, please ignore the accountHub.xhtml file, that's a temporary placeholder I was using fro the old patch.

Richard, I haven't implemented the toolbarbutton solution yet, I will do that in a follow up.

Comment 58

[Richard Marti (:Paenglab)]

Comment on attachment 9129924 [details] [diff] [review]
1589005-account-hub.diff

Looks really good. Thank you for the hard work.

With the actual implementation the text of some hub buttons with longer text are looking weird because they aren't centred. Maybe they need to be a bit wider, also for other locales with longer words that can't be wrapped (when you implement the wrapping).

The arrow-popus instead of the tooltips are looking good too. But now I get both, the tooltip and the popup. Depending of the mouse position, the tooltip is covering the popup and makes it unreadable. BTW how is the accessibility of this popups?

And if you go further the arrow-popup way, could you change the other tooltips to arrow-popups too? Then the page would be more consistent.

Again, the watermark is very faint with the dark theme.

Comment 59

[Alessandro Castellani [:aleca]]

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

Looks really good. Thank you for the hard work.

Thanks :D

With the actual implementation the text of some hub buttons with longer text
are looking weird because they aren't centred. Maybe they need to be a bit
wider, also for other locales with longer words that can't be wrapped (when
you implement the wrapping).

I was trying to work on this and find a good balance between wrapping and min-width, but then I decided to adopt a more natural approach since it was turning into a somewhat hacky solution to keep everything properly vertically centred aligned even when wrapping text, and how to handle those long strings during l10n.

I ended up letting the UI adapt to whatever string we end up with. The buttons have a min-width of 110px, which gives plenty of space to handle long strings, but just in case we have some very long word, that button will grow to adapting to its content.
It's better to have 1 button not perfectly aligned than over engineering things.

The arrow-popus instead of the tooltips are looking good too. But now I get
both, the tooltip and the popup. Depending of the mouse position, the
tooltip is covering the popup and makes it unreadable. BTW how is the
accessibility of this popups?

Yeah, that was a leftover. I'm using aria-labels for accessibility instead of tooltips, as we don't need that extra visual hint with the popups.

And if you go further the arrow-popup way, could you change the other
tooltips to arrow-popups too? Then the page would be more consistent.

I'm not sure about this.
Tooltips should be used for short strings and quick hints, while those popups are necessary due tot he longer descriptions.

Again, the watermark is very faint with the dark theme.

I increased the opacity a little bit, but I don't want to make it too visible as it gets distracting with the text and the other elements.
That watermark should be faint and out of the way, and almost unnoticeable once the user gets used to it.

Comment 60

[Alessandro Castellani [:aleca]]

Attachment: 1589005-account-hub.diff

I'm pinging Paul for a review as he's been dealing with startup things for the calendar integration and maybe can identity some shenanigans I might have missed in this scenario.

Ryan, can you review and strings in the accountCentral.ftl file?

Comment 61

[Richard Marti (:Paenglab)]

Comment on attachment 9130303 [details] [diff] [review]
1589005-account-hub.diff

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

ui-r+ with my comment reconsidered.

::: mail/themes/shared/mail/messenger.css
@@ -346,5 @@
>  
>  /* ::::: Status bar ::::: */
>  
> -#status-bar:-moz-lwtheme {
> -  border-top: 1px solid var(--tabs-border-color);

Why have you removed this border? With light LW-themes like Suave there is no division between the status bar and the tree content.

I'm okay with the changes in linux/mail/mailWindow1.css but here we should still have a border.

Comment 62

[Alessandro Castellani [:aleca]]

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

Why have you removed this border? With light LW-themes like Suave there is
no division between the status bar and the tree content.

Ah, thanks for the heads up.
It doesn't look good when using the built-in dark theme and it's not necessary having that border separation due to the contrast difference between the body and the sidebar background color.

What if I add this condition for light text, and put back the previous statement?

#status-bar:-moz-lwtheme-brighttext {
  border-top: none;
}

Comment 63

[Richard Marti (:Paenglab)]

With no border for dark themes the display jumps 1px when you change to a light theme or vice versa.

I propose to use

#status-bar:-moz-lwtheme-brighttext {
  border-top-color: var(--lwt-accent-color);
}

Like this the border has the same colour as the background of the status bar.

Comment 64

[Alessandro Castellani [:aleca]]

Boom, great!

Comment 65

[Alessandro Castellani [:aleca]]

Attachment: 1589005-account-hub.diff

Patch updated and try-run launched: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=fa6fb04f88474e766c97dc8da1cc2f2ffb058dc4

Comment 66

[Paul Morris [:pmorris]]

Comment on attachment 9130303 [details] [diff] [review]
1589005-account-hub.diff

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

This is looking really nice!  I have some minor suggestions and some typos.  Not sure I have much to add on the startup sequence side of things, but maybe I just don't know what the questions/issues are?  Here are some things I noticed, probably intentional or known issues, but maybe helpful, so FWIW:

- The "set up an email account" dialog opens on startup which seems unnecessary with this new UI.
- I see what you mean about the today pane being in the way.  Quickest fix (while we work on a more robust reworking of the default calendar situation), would be to have it hidden by default in the mail tab.  To do that, add `collapsedinmodes="mail"` to the <calendar-modevbox id="today-pane-panel"` here https://searchfox.org/comm-central/source/calendar/lightning/content/calendar-today-pane.inc.xhtml#12  I think that should do it.
- The external links, like for "Support" etc., were not opening for me.
- After setting up an email account the old account central came back.

::: mail/base/content/folderPane.js
@@ +151,5 @@
>      this._treeElement = aTree;
>      this.messengerBundle = document.getElementById("bundle_messenger");
>  
> +    // The folder pane can be used for other trees which may not have these
> +    // elements. Collpase them if no account is currently available.

typo: "Collapse"

@@ +166,2 @@
>        document.getElementById("folderPaneBox").collapsed = false;
>      }

Maybe wrap these two conditionals in one `if (MailServices.accounts.accounts.length > 0) {}` to avoid the duplication?

::: mail/base/content/msgMail3PaneWindow.js
@@ +462,5 @@
>      okCallback();
>      return;
>    }
>  
> +  // Collpase the Folder Pane since no account is currently present.

Collapse

::: mail/locales/en-US/messenger/accountCentral.ftl
@@ +13,5 @@
> +
> +email-label =
> +    .label = Email
> +    .aria-label = Connect to your existing Email account
> +email-description = { -brand-short-name } let's you connect to your existing email account, to read your emails conveniently and efficently from withing the application.

let's -> lets (and also in similar places below)
withing -> within
efficently -> efficiently

::: mailnews/base/content/msgAccountCentral.js
@@ +76,5 @@
> +      if (relNotesPrefType != Services.prefs.PREF_INVALID) {
> +        let relNotesURL = Services.urlFormatter.formatURLPref(
> +          "app.releaseNotesURL"
> +        );
> +        if (relNotesURL != "about:blank") {

You could move the `let relNotesLink = ...` variable declaration here just above where it is used, and avoid the `getElementById` call if it isn't needed.

@@ +389,5 @@
> +            this.gCurrentAccount.incomingServer.rootMsgFolder
> +          );
> +        }
> +      }, type);
> +      break;

While we're at it, I'd be in favor of dropping this CreateNewAccountTypeTB function since it doesn't do much.  Like, why not just call `AddAddressBook() and friends directly?  Looks like we only need the default case for newsgroups, so we could keep it for that.  Am I missing something?

Comment 67

[Richard Marti (:Paenglab)]

Comment on attachment 9130303 [details] [diff] [review]
1589005-account-hub.diff

There is a newer patch available.

Comment 68

[Richard Marti (:Paenglab)]

(In reply to Paul Morris [:pmorris] from comment #66)

• The "set up an email account" dialog opens on startup which seems
  unnecessary with this new UI.

It's okay when this dialog appears. Mail is the main usage of TB and it should help the user with showing it. Like this the user hasn't to search how he can apply a mail account.

Comment 69

[Alessandro Castellani [:aleca]]

(In reply to Paul Morris [:pmorris] from comment #66)

The "set up an email account" dialog opens on startup which seems unnecessary with this new UI.

As Richard said, this dialog needs to happen as we want to direct the user towards the most common/important thing they have to do.
Later on, there's a plan to remove the dialog and convert the form to be inline inside the page, but nothing has been set in stone yet.

I see what you mean about the today pane being in the way. Quickest fix (while we work on a more robust reworking of the default calendar situation), would be to have it hidden by default in the mail tab. To do that, add collapsedinmodes="mail" to the <calendar-modevbox id="today-pane-panel"` here https://searchfox.org/comm-central/source/calendar/lightning/content/calendar-today-pane.inc.xhtml#12 I think that should do it.

Can I then remove that attribute once the account setup is completed in order to show the Today Pane?

The external links, like for "Support" etc., were not opening for me.

Yeah, they don't work and I don't know why, I need to investigate this.

After setting up an email account the old account central came back.

Yes, that's on purpose as we're implementing this progressively.
The next follow up patch will take care of updating the regular account central for users with an account already configured.

Thanks for the review, I'm fixing everything you highlighted.

Comment 70

[Paul Morris [:pmorris]]

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

As Richard said, this dialog needs to happen as we want to direct the user towards the most common/important thing they have to do.
Later on, there's a plan to remove the dialog and convert the form to be inline inside the page, but nothing has been set in stone yet.

Makes sense.

Can I then remove that attribute once the account setup is completed in order to show the Today Pane?

Good question. I checked and to show the today pane again you'll need to do a call like this:

TodayPane.toggleVisibility({
// this is a fake event object
target: document.getElementById("calendar_toggle_todaypane_command")
});

Another option that feels better to me is to have the default calendar disabled at first. That will hide the today pane (and some of the other calendar UI, particularly menus). Then if the user sets up a new calendar or enables the default calendar the today pane will appear. To make that path smooth we'd need some UI to help the user see how to enable the disabled calendar. (That's something we probably want in any case for when all calendars are disabled.)

That seems closer to where we want to be. I can look into what it would take for the default calendar to be disabled at first, if you want to go this route.

Yes, that's on purpose as we're implementing this progressively.
The next follow up patch will take care of updating the regular account central for users with an account already configured.

Makes sense.

Thanks for the review, I'm fixing everything you highlighted.

Sure thing, glad it's helpful.

Comment 71

[Alessandro Castellani [:aleca]]

That seems closer to where we want to be. I can look into what it would take for the default calendar to be disabled at first, if you want to go this route.

Yes, that'd be ideal I think.
I was trying to hide the today pane with what you suggested, and then showing it only after a successful setup, but unfortunately I found out that we trigger the same callback function for successful or unsuccessful setup wizard, and all UI operations are triggered from the same method.
It was getting pretty hacky to handle the various scenarios.

Comment 72

[Alessandro Castellani [:aleca]]

Attachment: 1589005-account-hub.diff

Comment 73

[Paul Morris [:pmorris]]

Attachment: default-disabled-home-calendar-0.patch

Okay, on first run, this sets up the default "Home" calendar as disabled so that parts of the calendar UI (such as the today pane) are not visible until a calendar is enabled.

I assume that in a follow-up we'll want to add some UI to the calendar and task tabs to guide the user on how to enable a calendar when they are all disabled. (And/or something more dramatic on setting up an initial calendar.)

Flagging Geoff for review since it's a calendar change.

Comment 75

[Ryan Lee Sipes]

Comment on attachment 9130874 [details] [diff] [review]
1589005-account-hub.diff

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

Reviewed the strings, I don't see any issues in this iteration.

Comment 76

[Magnus Melin [:mkmelin]]

Comment on attachment 9130874 [details] [diff] [review]
1589005-account-hub.diff

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

I get a
 "[l10nregistry] Attempting to synchronously load file\n            resource://gre/localization/en-US/messenger/accountCentral.ftl while it's being loaded asynchronously."'

JavaScript error: chrome://lightning/content/messenger-overlay-sidebar.js, line 499: TypeError: Node.replaceChild: Argument 2 is not an object.
(not sure where)

JavaScript error: chrome://messenger/content/msgAccountCentral.js, line 406: TypeError: can't access property "launchExternalURL", m is undefined
 (after clicking the info icon on top?)

The buttons move around some when hovering. I think they should not do that, the hover effect seems a bit too much.

::: mail/locales/en-US/messenger/accountCentral.ftl
@@ +12,5 @@
> +    .tooltiptext = Release Notes
> +
> +email-label =
> +    .label = Email
> +    .aria-label = Connect to your existing Email account

email

@@ +17,5 @@
> +email-description = { -brand-short-name } lets you connect to your existing email account, to read your emails conveniently and efficiently from within the application.
> +
> +calendar-label =
> +    .label = Calendar
> +    .aria-label = Create a new Calendar

calendar

@@ +22,5 @@
> +calendar-description = { -brand-short-name } lets you create a local calendar or connect to a remote calendar to always keep all your events in sync.
> +
> +chat-label =
> +    .label = Chat
> +    .aria-label = Connect to your IM Account

to your chat account?

@@ +28,5 @@
> +
> +filelink-label =
> +    .label = Filelink
> +    .aria-label = Set Up Filelink
> +filelink-description = { -brand-short-name } lets you set up a convenient filelink cloud upload to easily manage large attachments.

...cloud account to easily send large attachments?

@@ +32,5 @@
> +filelink-description = { -brand-short-name } lets you set up a convenient filelink cloud upload to easily manage large attachments.
> +
> +addressbook-label =
> +    .label = Address Book
> +    .aria-label = Create a new Address Book

address book

@@ +37,5 @@
> +addressbook-description = { -brand-short-name } lets you create local address books, import existing ones, or connect to remote address books to keep all your contacts in sync.
> +
> +feeds-label =
> +    .label = Feeds
> +    .aria-label = Connect to a Feed

Feeds

@@ +39,5 @@
> +feeds-label =
> +    .label = Feeds
> +    .aria-label = Connect to a Feed
> +feeds-description = { -brand-short-name } lets you connect to all the RSS feeds you want and organize them in dedicated folders.
> +

{ -brand-short-name } lets you connect to RSS/Atom feeds and read get news and updates from all around.

@@ +42,5 @@
> +feeds-description = { -brand-short-name } lets you connect to all the RSS feeds you want and organize them in dedicated folders.
> +
> +newsgroups-label =
> +    .label = Newsgroups
> +    .aria-label = Connect to a Newsgroup

to newsgroups

@@ +44,5 @@
> +newsgroups-label =
> +    .label = Newsgroups
> +    .aria-label = Connect to a Newsgroup
> +newsgroups-description = { -brand-short-name } lets you connect to all the newsgroups you want and organize them in dedicated folders.
> +

This is incorrect. Newsgroups are groups, and you can't move them or organize in any way.

@@ +50,5 @@
> +import-paragraph = { -brand-short-name } lets you import mail messages, address book entries, feed subscriptions, preferences, and/or filters from other mail programs and common address book formats.
> +
> +import-label =
> +    .label = Import
> +    .aria-label = Import Data from other Programs

no capitalizations please

@@ +52,5 @@
> +import-label =
> +    .label = Import
> +    .aria-label = Import Data from other Programs
> +
> +about-paragraph = { -brand-full-name } is the leading open source, cross-platform email and calendaring client, free for business and personal use. We want it to stay secure and become even better. A donation will allow us to hire developers, pay for infrastructure, and continue to improve.

I think for these we should write out Thunderbird instead of { -brand-full-name }

@@ +54,5 @@
> +    .aria-label = Import Data from other Programs
> +
> +about-paragraph = { -brand-full-name } is the leading open source, cross-platform email and calendaring client, free for business and personal use. We want it to stay secure and become even better. A donation will allow us to hire developers, pay for infrastructure, and continue to improve.
> +
> +# {" "} is used to add a blank space character at the end of the text.

why?

@@ +55,5 @@
> +
> +about-paragraph = { -brand-full-name } is the leading open source, cross-platform email and calendaring client, free for business and personal use. We want it to stay secure and become even better. A donation will allow us to hire developers, pay for infrastructure, and continue to improve.
> +
> +# {" "} is used to add a blank space character at the end of the text.
> +about-paragraph-2 = <b>{ -brand-full-name } is funded by users like you! If you like { -brand-full-name }, please consider making a donation.</b> The best way for you to ensure { -brand-full-name } remains available is to{" "}

Same thing here, write out Thunderbird

::: mailnews/base/content/msgAccountCentral.xhtml
@@ +43,5 @@
>    <script src="chrome://messenger/content/msgMail3PaneWindow.js"/>
>    <script src="chrome://messenger/content/msgAccountCentral.js"/>
>    <script src="chrome://messenger/content/mailCore.js"/>
>  
> +  <panel id="accountDescriptionPanel" type="arrow" orient="vertical"

Can we do it directly as xhtml

@@ +63,5 @@
> +  </panel>
> +
> +  <main id="accountCentral" hidden="true">
> +
> +    <header class="account-central-header">

You need to have all these be html:main, html:header etc, or the other way around but set xhtml as the default namespace. Now it's really xul:header which is nothing, but may resemble a box

@@ +165,5 @@
> +    <section class="account-central-section">
> +      <h2 class="section-title" data-l10n-id="about-title"/>
> +
> +      <aside class="row-container">
> +        <p data-l10n-id="about-paragraph"/>

should not do self-closing tags, even in xhtml (except for the elements that are defined as such like img)

@@ +171,5 @@
> +      </aside>
> +    </section>
> +
> +    <section class="account-central-section">
> +      <h2 class="section-title" data-l10n-id="resources-title"/>

same, no self closing

Comment 77

[Alessandro Castellani [:aleca]]

I get a "[l10nregistry] Attempting to synchronously load file resource://gre/localization/en-US/messenger/accountCentral.ftl while it's being loaded asynchronously."'
JavaScript error: chrome://lightning/content/messenger-overlay-sidebar.js, line 499: TypeError: Node.replaceChild: Argument 2 is not an object.

Uh? I'm not getting these errors at all, that's strange.

JavaScript error: chrome://messenger/content/msgAccountCentral.js, line 406: TypeError: can't access property "launchExternalURL", m is undefined
(after clicking the info icon on top?)

I've been struggling with these and I saw from old bugs that already happened. Any suggestion?
"messenger" in window is true, but window.messenger is null.
And even if I directly create the instance let m = Cc["@mozilla.org/messenger;1"].createInstance(Ci.nsIMessenger);, I get the same error.

The buttons move around some when hovering. I think they should not do that, the hover effect seems a bit too much.

The buttons translate up by 2px on hover, is distracting? I find it pleasant to give a sense of depth.
Those buttons are the focal point of this area, that's why the hover effect is very vivid and pretty impossible to miss.

+# {" "} is used to add a blank space character at the end of the text.

why?

Because I'm adding the make a donation link dynamically in order to populate the URL as fluent doesn't allow using <a> html tags in a string, and automatically strips away blank spaces.
I can do that, which is the fluent recommended approach, or I can do donationParagraph.append(" ");
What do you think?

Can we do it directly as xhtml

Are we using a <panel> element in xhtml anywhere else? Or is there a direct counterpart element?

Comment 78

[Magnus Melin [:mkmelin]]

For the links, do you actually have to use any js to open them, or wouldn't they handle it them selves?

Re panel, I don't know, but it's not very webby. I would prefer having something show up using the whole line (or more) instead of the panel tooltippy thing showing on hover. You'd only really need to implement showing the right content in the area below on hover.

Comment 79

[Alessandro Castellani [:aleca]]

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

For the links, do you actually have to use any js to open them, or wouldn't they handle it them selves?

A simple href doesn't seem to work, and I noticed the same problem for external links in the about dialog.
Sometimes I get the error that window.messenger is null, but other times I get that dialog warning that Firefox is already running.

Comment 80

[Alessandro Castellani [:aleca]]

Attachment: 1589005-account-hub.diff

Here's an updated version of the patch using only HTML elements for the new section.
I still haven't figure out what's the problem with the links, I wonder if it's an underlying issue of daily, as the same methods work on 68.

Comment 81

[Magnus Melin [:mkmelin]]

Comment on attachment 9132756 [details] [diff] [review]
1589005-account-hub.diff

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

Looks much better!

::: mail/locales/en-US/messenger/accountCentral.ftl
@@ +3,5 @@
> +# file, You can obtain one at http://mozilla.org/MPL/2.0/.
> +
> +account-central-title = Welcome to { -brand-full-name }
> +
> +setup-title = Choose an Account to Set Up

Maybe this should be
Choose What to Set Up

@@ +17,5 @@
> +
> +calendar-label = Calendar
> +    .aria-label = Create a new calendar
> +calendar-description = { -brand-short-name } lets you create a local calendar or connect to a remote calendar to always keep all your events in sync.
> +

would suggest (not to advertize local calendars):
{ -brand-short-name } handles event data keeps you organized using an electronic calendar. With a remote calendar you can keep all your events in sync across all your devices.

@@ +28,5 @@
> +filelink-description = { -brand-short-name } lets you set up a convenient filelink cloud account to easily send large attachments.
> +
> +addressbook-label = Address Book
> +    .aria-label = Create a new address book
> +addressbook-description = { -brand-short-name } lets you create local address books, import existing ones, or connect to remote address books to keep all your contacts in sync.

would suggest:
{ -brand-short-name } stores your contacts in an address book. You can create a local address book but also connect to a remote address book  to keep all your contacts in sync.

@@ +38,5 @@
> +newsgroups-label = Newsgroups
> +    .aria-label = Connect to a newsgroup
> +newsgroups-description = { -brand-short-name } lets you connect to all the newsgroups you want.
> +
> +import-title = Import from Another Program

maybe we should change this to cover the case for importing from another thunderbird instance too, even if it's not automatic just yet

::: mailnews/base/content/msgAccountCentral.xhtml
@@ +16,5 @@
>    <!ENTITY % lightningDTD SYSTEM "chrome://lightning/locale/lightning.dtd">
>     %lightningDTD;
>  ]>
>  
> +<window xmlns="http://www.w3.org/1999/xhtml"

this would now be an html:window which is nothing. The top level should be <html>, and please add a <head> under it

@@ +31,5 @@
> +
> +  <link rel="localization" href="branding/brand.ftl"/>
> +  <link rel="localization" href="messenger/accountCentral.ftl"/>
> +
> +  <script src="chrome://communicator/content/utilityOverlay.js"/>

since it will be html, use a closing tag for <script>

@@ +46,3 @@
>  
> +      <header class="account-central-header">
> +        <img id="brandLogo" src="chrome://branding/content/about-logo.svg"/>

The images want an alt too. Probably alt="" since it's all decorative only

@@ +48,5 @@
> +        <img id="brandLogo" src="chrome://branding/content/about-logo.svg"/>
> +        <aside>
> +          <h1 class="account-central-title" data-l10n-id="account-central-title"></h1>
> +          <div class="account-central-version">
> +            <label id="version"></label>

label? I guess you want a span or div?

@@ +303,5 @@
>  
> +        <spacer id="createAccountSpacer" flex="1" collapsed="true"/>
> +        <hbox id="createAccount" class="acctCentralRow" collapsed="true">
> +          <hbox>
> +  #ifdef MOZ_THUNDERBIRD

I think ifdefs need to be at col 0.

Comment 82

[Alessandro Castellani [:aleca]]

{ -brand-short-name } handles event data keeps you organized using an electronic calendar. With a remote calendar you can keep all your events in sync across all your devices.

I think this is missing an and after "handles event data".
Also, it doesn't sound great as the way it's written seems that electronic and remote calendars are two different things, and also changes the subject, first is Thunderbird that keeps things organized, and then it's the user that can keep the events in sync.

I'd suggest:
"{ -brand-short-name } lets you handle events and keep you organized. Connecting to a remote calendar will keep all your events in sync across all your devices."

would suggest:
{ -brand-short-name } stores your contacts in an address book. You can create a local address book but also connect to a remote address book to keep all your contacts in sync.

I would shorten it down a bit to avoid repeating "address book" too many times.
"{ -brand-short-name } lets you organize all your contacts in an address book. You can also connect to a remote address book to keep all your contacts in sync."

import-title = Import from Another Program
maybe we should change this to cover the case for importing from another thunderbird instance too, even if it's not automatic just yet

I don't think we should do it if it's not part of that dialog yet as it can be misleading. We could tackle it in a dedicated bug once that option is implemented.

Comment 83

[Magnus Melin [:mkmelin]]

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

I'd suggest:
"{ -brand-short-name } lets you handle events and keep you organized. Connecting to a remote calendar will keep all your events in sync across all your devices."

Sounds good.

I would shorten it down a bit to avoid repeating "address book" too many times.
"{ -brand-short-name } lets you organize all your contacts in an address book. You can also connect to a remote address book to keep all your contacts in sync."

Sounds good as well.

Comment 84

[Alessandro Castellani [:aleca]]

Attachment: 1589005-account-hub.diff

Patch updated and try-run launched:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=4174841df25c40e25a610aa965d070dacb41be34

There a couple of test failures I'm dealing with right now.

Comment 85

[Alessandro Castellani [:aleca]]

Attachment: 1589005-account-hub.diff

Tests fixed: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=6e600ec71c2b0083738cb0f273772ea44b9408eb

Comment 86

[Alessandro Castellani [:aleca]]

Attachment: 1589005-account-hub.diff

Another test fixed and took care of missing icons on macos.
Sorry for the spam.

Comment 87

[Magnus Melin [:mkmelin]]

Comment on attachment 9133307 [details] [diff] [review]
1589005-account-hub.diff

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

One think I notice: the status bar "show today pane" is still shown. Not a biggie.

::: mail/locales/en-US/messenger/accountCentral.ftl
@@ +16,5 @@
> +email-description = { -brand-short-name } lets you connect to your existing email account, to read your emails conveniently and efficiently from within the application.
> +
> +calendar-label = Calendar
> +    .aria-label = Create a new calendar
> +calendar-description = { -brand-short-name } lets you handle events and keep you organized. Connecting to a remote calendar will keep all your events in sync across all your devices.

keepS you organized

@@ +31,5 @@
> +    .aria-label = Create a new address book
> +addressbook-description = { -brand-short-name } lets you organize all your contacts in an address book. You can also connect to a remote address book to keep all your contacts in sync.
> +
> +feeds-label = Feeds
> +    .aria-label = Connect to a Feeds

to feeds

@@ +32,5 @@
> +addressbook-description = { -brand-short-name } lets you organize all your contacts in an address book. You can also connect to a remote address book to keep all your contacts in sync.
> +
> +feeds-label = Feeds
> +    .aria-label = Connect to a Feeds
> +feeds-description = { -brand-short-name } lets you connect to RSS/Atom feeds and get news and updates from all around.

I guess we shoudl replace and with to

::: mailnews/base/content/msgAccountCentral.js
@@ +346,5 @@
>  function CreateNewAccount() {
>    window.parent.msgOpenAccountWizard();
>  }
>  
> +function CreateNewsgroups(type) {

This wouldn't take a type

Comment 88

[Alessandro Castellani [:aleca]]

Attachment: 1589005-account-hub.diff

Requested changes implemented, and I also tested every single account creation to be sure the UI gets properly updated at the end of every wizard.
Another try-run before marking it for check-in: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=db1d1c475654a86cfed68a7e8ce0fe50ca51bbb3

Comment 89

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/7b3549479ead
Let default 'Home' calendar be disabled by default. r=darktrojan
https://hg.mozilla.org/comm-central/rev/4c328c3d6c01
Implement new Account Hub for centralized set ups. r=mkmelin, ui-r=paenglab

Comment 90

[Geoff Lankow (:darktrojan)]

I backed out Paul's patch because it broke a bunch of the calendar tests. I really should've seen that coming.

https://hg.mozilla.org/comm-central/rev/6b7f4b4acf12c581adaa6588006cfd1d2b33c6bc

Comment 91

[WaltS48 [:walts48]]

I only see this with a new profile using BuildID: 20200317104217 on Ubuntu 18.04.4 Linux behind the Account Wizard dialog.

I created an email account and saw the new Account Hub with the email account showing in the Folder pane.
Selected the Inbox, then went back to the account level and the old Account Central appeared.

Expected behavior?

Comment 92

[Alessandro Castellani [:aleca]]

(In reply to WaltS48 [:walts48] from comment #91)

Expected behavior?

Yes.
That's why this bug has the leave-open keyword, a follow up patch is coming.

Comment 93

[Paul Morris [:pmorris]]

(In reply to Geoff Lankow (:darktrojan) from comment #90)

I backed out Paul's patch because it broke a bunch of the calendar tests. I really should've seen that coming.

Ahhh... sorry, I should have caught that too. I'm working on a fix and have opened bug 1623152 for it.

Comment 94

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/fb7372b0b4f7
Let default 'Home' calendar be disabled on first run. r=darktrojan

Comment 96

[Alessandro Castellani [:aleca]]

Attachment: 1589005-account-hub-PART2.diff

And here's the second part of this bug that takes care of implementing the new account hub for existing accounts.

A couple of questions and issues I'm having related to this.

• In the xhtml file, I can't seem to be able to remove the #ifdef MOZ_THUNDERBIRD conditions as my build breaks.
• The msgAccountCentral.dtd can be delete. Should I create a Fluent migration for those few strings I'm reusing? Will the Fluent migration prevent this from landing?
• There a lot of try catch in the JS file, are those necessary? That method gets called only if a server is available, so I think those extra conditions could be removed.

Comment 97

[Richard Marti (:Paenglab)]

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

Created attachment 9137354 [details] [diff] [review]
1589005-account-hub-PART2.diff

And here's the second part of this bug that takes care of implementing the new account hub for existing accounts.

A couple of questions and issues I'm having related to this.

• In the xhtml file, I can't seem to be able to remove the #ifdef MOZ_THUNDERBIRD conditions as my build breaks.

You need to remove the star in front of this line https://searchfox.org/comm-central/source/mailnews/jar.mn#74. The star means preprocess this file and when you remove the last preprocessing the preprocessor fails.

Comment 98

[Magnus Melin [:mkmelin]]

For Fluent migration recipe, it's very few strings so maybe not worth it.
Probably lot of try-catches can be removed.

Comment 99

[Magnus Melin [:mkmelin]]

Comment on attachment 9137354 [details] [diff] [review]
1589005-account-hub-PART2.diff

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

::: mail/locales/en-US/messenger/accountCentral.ftl
@@ +62,5 @@
>  developer-link = Developer Documentation
> +
> +email-features = Email Features
> +rss-features = Feeds Features
> +nntp-features = Newsgroup Features

Not sure about the wording for these two - it doesn't read very well. Should it be just

RSS/Atom Feeds
Newsgroups

@@ +63,5 @@
> +
> +email-features = Email Features
> +rss-features = Feeds Features
> +nntp-features = Newsgroup Features
> +local-features = Local Folder Features

what's this?

@@ +71,5 @@
> +search = Search messages
> +filter = Manage message filters
> +subscription = Manage folder subscriptions
> +nntp-subscription = Manage newsgroup subscriptions
> +rss-subscription = Manage feeds subscriptions

feed subscriptions

::: mail/themes/shared/mail/accountCentral.css
@@ +66,5 @@
>  
> +button[hidden],
> +section[hidden] {
> +  display: none;
> +}

I wouldn't think this is needed.

@@ +125,5 @@
> +.account-central-header.summary-header {
> +  padding-block: 16px;
> +}
> +
> +.account-central-header[hidden] {

shouldn't be needed
hidden="hidden" should hide, and removing will show

::: mailnews/base/content/msgAccountCentral.xhtml
@@ +31,2 @@
>  
> +      <header id="headerFirstRun" class="account-central-header" hidden="true">

hidden="hidden"

Comment 100

[Richard Marti (:Paenglab)]

Comment on attachment 9137354 [details] [diff] [review]
1589005-account-hub-PART2.diff

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

Now there's a lot of content on this page. But I think it's the just the right amount. More would be too much.

::: mail/themes/shared/mail/accountCentral.css
@@ +204,5 @@
>    padding-block-end: 30px;
>  }
>  
> +.account-central-section.zebra {
> +  background-color: rgba(0, 0, 0, 0.03);

This doesn't work with the dark theme. Maybe you could use var(--header-bg-color) as it's almost the same value.

Comment 101

[Alessandro Castellani [:aleca]]

+button[hidden],
+section[hidden] {
+display: none;
+}
I wouldn't think this is needed.

I thought that too but I had to add it otherwise those elements wouldn't hide.
I don't know why.

Comment 102

[Alessandro Castellani [:aleca]]

Attachment: 1589005-account-hub-PART2.diff

Comment 104

[Alessandro Castellani [:aleca]]

And here's a try-run: https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=29dcad826acf87178b08068bf5fe096c5ce77281

Comment 105

[Magnus Melin [:mkmelin]]

Comment on attachment 9137519 [details] [diff] [review]
1589005-account-hub-PART2.diff

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

The "tooltip" is shown in above the next header when the "Set up another Account" sections is on two rows (narrow screen).

"Explore features" and the other links do not work, all I get is:
JavaScript error: chrome://messenger/content/msgAccountCentral.js, line 328: TypeError: can't access property "launchExternalURL", m is undefined

::: mail/locales/en-US/messenger/accountCentral.ftl
@@ +8,5 @@
>  
> +#   $accounts (Number) - the number of configured accounts
> +setup-title = { $accounts ->
> +    [0]      Choose What to Set Up
> +    *[other] Set up another Account

Set Up Another Account

@@ +62,5 @@
>  developer-link = Developer Documentation
> +
> +email-features = Email Features
> +rss-features = RSS/Atom Feeds
> +nntp-features = Newsgroup

This should be Newsgroups

Though, I'm not convinced we need these headers

::: mail/themes/shared/mail/accountCentral.css
@@ +390,5 @@
> +  border: none;
> +  cursor: pointer;
> +  padding: 0;
> +  line-height: 1em;
> +  display: flex;

this is the problem with the buttons and [hidden], they get display: flex;

Not sure which sections had problems, but check the computed display proprety fom developer tools and where it comes from while hidden is set.

::: mailnews/base/content/msgAccountCentral.js
@@ +54,3 @@
>    } catch (ex) {
>      Cu.reportError("Error getting selected account: " + ex + "\n");
>    }

you could just get rid of this try/catch

@@ +241,5 @@
> +    .getElementById("advancedFeatures")
> +    .toggleAttribute(
> +      "hidden",
> +      !canHaveFilters && !canSubscribeImapFolders && !supportsOffline
> +    );

It's a bit of a shame we don't show filters management for POP accounts. Contrary to to the other things we list here that's actually a bit more advanced. I think the other parts should really be scrapped. I don't really see why we'd list folder subscriptions or offline settings there... Maybe we actually want to take the opportunity and remove them now. One thing we COULD add is End-To-End Encryption, which really IS advanced. 

Maybe we just want to scrap the Advanced section all together? And potentially just list read write seach and encryption horizontally?

Comment 106

[Alessandro Castellani [:aleca]]

The "tooltip" is shown in above the next header when the "Set up another Account" sections is on two rows (narrow screen).

Those tooltips are absolute floating elements that appear only on rollover. I made them like this on purpose so it doesn't matter how tall the text is, or how small the window gets resized, the text floats above everything to be readable, and it doesn't push the content down, creating an empty gap underneath the setup buttons when not visible.

Though, I'm not convinced we need these headers

I'd recommend leaving them for now for a couple of reasons.
First we give the context of those actions, if those are related to email, feed, newsgroups, as in the title bar is only visualized the name of the account and not the type.
I agree it's a bit redundant, but better keep it for the second reason, which is the fact that this patch aims at implementing a new UI but without adding too many changes to the features we already have.
This section will also be modified with a follow up patch to introduce stats and other things.

this is the problem with the buttons and [hidden], they get display: flex;
Not sure which sections had problems, but check the computed display property from developer tools and where it comes from while hidden is set.

Ah, thanks for finding this out.
That's very strange that a hidden HTML attribute doesn't supersede a CSS attribute, but anyway that's an easy fix.

Maybe we just want to scrap the Advanced section all together? And potentially just list read write search and encryption horizontally?

Are we sure those quick links are not useful for the user?
I don't know what was the rationale behind putting those links there, but I kinda guessed that those are specific features for a specific account type that we wanted to highlight for the user.

Richard and Magnus, what do you guys think?

Comment 107

[Alessandro Castellani [:aleca]]

Attachment: 1589005-account-hub-PART2.diff

This patch fixes the issue with the links not opening and the CSS hidden attribute.

Comment 108

[Richard Marti (:Paenglab)]

Actually with the patch I never see the filter management, not in a POP account nor in a IMAP one. Without patch I see them in every account type.

I'm for leaving the advanced features. Without them we have mostly only a account hub without special account functions.

Comment 109

[Alessandro Castellani [:aleca]]

Actually with the patch I never see the filter management, not in a POP account nor in a IMAP one. Without patch I see them in every account type.

Ah! That's a mistake, sorry.

Comment 110

[Magnus Melin [:mkmelin]]

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

The "tooltip" is shown in above the next header when the "Set up another Account" sections is on two rows (narrow screen).

Those tooltips are absolute floating elements that appear only on rollover. I made them like this on purpose so it doesn't matter how tall the text is, or how small the window gets resized, the text floats above everything to be readable, and it doesn't push the content down, creating an empty gap underneath the setup buttons when not visible.

I think I suggested in the past, it could be an area there that opens up, with a css transition. I don't know, maybe we can leave it like this too for now, but I don't think it's ideal.

I agree it's a bit redundant, but better keep it for the second reason, which is the fact that this patch aims at implementing a new UI but without adding too many changes to the features we already have.

But we don't have those headers now. It's pretty obvious for instance, that the account is an email account (if that's what you have).

Maybe we just want to scrap the Advanced section all together? And potentially just list read write search and encryption horizontally?

Are we sure those quick links are not useful for the user?

I'm pretty sure they were put there because there wasn't anything better at that time:/ However, Offline should not really be touched. Back when it was added, we did not automatically set up Offline usage but now there's not really much you can gain by fiddling with it.. In the end, sending the user to the account settings is just more useful. Folder subscriptions: oh well, it's just too technical for the average use.

Comment 111

[Alessandro Castellani [:aleca]]

I think I suggested in the past, it could be an area there that opens up, with a css transition. I don't know, maybe we can leave it like this too for now, but I don't think it's ideal.

The problem with revealing items in pure CSS is that you can't animate the display attribute, therefore those elements will need to occupy their space with a position relative if you want to avoid overlaps, and since we have multiple descriptions, we will end up with a big empty gap underneath those buttons.

This problem can be solved with Javascript, but that will cause all the elements underneath those tooltips to be pushed up and down at every rollover (pretty jarring), and I think we should avoid animations via JS as they are taxing on the CPU.

Comment 112

[Alessandro Castellani [:aleca]]

Attachment: 1589005-account-hub-PART2.diff

Fixed the message filters issue, remove those unnecessary advanced features, added the e2e encryption link, and did a quick round of CSS improvements.

Comment 113

[Magnus Melin [:mkmelin]]

Comment on attachment 9137878 [details] [diff] [review]
1589005-account-hub-PART2.diff

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

::: mail/locales/en-US/messenger/accountCentral.ftl
@@ +3,5 @@
>  # file, You can obtain one at http://mozilla.org/MPL/2.0/.
>  
>  account-central-title = Welcome to { -brand-full-name }
> +account-settings = Account Settings
> +advanced-title = Advanced Features

not used now

::: mail/themes/shared/mail/accountCentral.css
@@ +128,5 @@
> +.btn-link:not([hidden]),
> +.account-central-header:not([hidden]),
> +.account-central-section:not([hidden]) {
> +  display: flex;
> +}

Alternatively we might want to just have a rule

[hidden] {
  display: none !important; /* Esure flex items obey hidden="hidden". */
}

::: mailnews/base/content/msgAccountCentral.js
@@ +174,5 @@
> +  document
> +    .getElementById("e2eButton")
> +    .toggleAttribute(
> +      "hidden",
> +      !MailConstants.MOZ_OPENPGP || !canGetMessages || isRssAccount

There is e2e with S/MIME too, so let's remove the !MailConstants.MOZ_OPENPGP check

@@ +204,5 @@
>  
> +/**
> + * Open the AccountManager to view the settings for a given account.
> + *
> + * @param selectPage  the xhtml file name for the viewing page,

nit: 
@param selectPage - The ...

Comment 114

[Alessandro Castellani [:aleca]]

Attachment: 1589005-account-hub-PART2.diff

Comment 115

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/comm-central/rev/eb455a1431bf
Implement new Account Hub for users with existing account. r=mkmelin, ui-r=paenglab

Comment 116

[Thomas D. (:thomas8)]

Awesome work here, Alex!!!

We've come a long way since bug 529499 which I filed 11 years ago, also thanks to the great work of Aceman who fixed much of the worst at the time in bug 713277 and friends.