Closed Bug 679696 Opened 13 years ago Closed 12 years ago

Identity picker on compose window is hard to use when you have a lot of identities

Categories

(Thunderbird :: Message Compose Window, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 18.0

People

(Reporter: justdave, Assigned: aceman)

References

(Depends on 1 open bug)

Details

(Keywords: ux-efficiency, ux-error-prevention)

Attachments

(1 file, 4 obsolete files)

I participate in a lot of "role" addresses in my duties as a sysadmin, many of which are set up in Thunderbird as available addresses to send mail from.  Thus my identity picker on the compose window has about 50 addresses in it, with some on each of my mail accounts.

90% of the time when I'm manually setting an identity, I actually want the primary identity for an account, just a different one than it defaulted to.  The primary identities are hard to find because they look just like the rest of them.

Two possible solutions for this:
1) Make the primary identities all show as boldface in the menu

or

2) Draw a divider line between each account's identities within the menu, which will still make primary identities easy to spot, since they're the first one within each group.
What made me realize I should file this was because someone was asking me about using mailto: links and I wound up having to explain to they why I do right-click "Copy Email Address" in Firefox in order to paste the address into Thunderbird instead of just clicking the link.  And that explanation was that it's too hard to find the primary identities in the menu, and it was easier to choose the inbox from the account I wanted and then hit the "Write" button.
I too have a large number of identities but the reason is that I use a specific address for each organisation I deal with (similar to disposable addresses) but these are attached to a single account. The proposed solutions would therefore not help me. The identity picker is a nightmare because (a) the list is not sorted into any logical order, (b) the list order cannot be changed and (c) the list cannot be searched. Providing any or all of these capabilities would be a great help.
Depends on: identity_ordering
Comment 0 is an interesting proposal. I can look at it.

Barrie, for (b) see bug 384303 and bug 314806.
Assignee: nobody → acelists
Bwinton, Mkmelin, which option would you like from comment 0?
I think either would be fine…
Attached patch patch (obsolete) — Splinter Review
So make the default identities bold. I wanted to do a border too, but it wasn't ignored. Am I doing something wrong? Maybe there can't be border set on menulist items?
Attachment #663159 - Flags: ui-review?(bwinton)
I mean the border WAS ignored (not shown).
Status: NEW → ASSIGNED
OS: Mac OS X → All
Hardware: x86 → All
Maybe Seamonkey guys would want this too.
Blocks: 793135
Comment on attachment 663159 [details] [diff] [review]
patch

[I know, you haven't asked for code review yet...]

>+  let result = [];
>+  for each (let member in fixIterator(supportsArray, iid))
>+    result.push(member);
>+
>+  return result;
Or you could use toArray(fixIterator(...))

>+    let firstIdentity = true;
>+    for each (let identity in identites) {
>+      let item = menulist.appendItem(identity.identityName, identity.key,
>                                      account.incomingServer.prettyName);
>       item.setAttribute("accountkey", account.key);
>+      if (firstIdentity) {
Now this is where a simple for/next loop wins, since you can just use if (!index)

>+        item.setAttribute("class", "defaultIdentity");
item.setAttribute("default", "true"); also works.
(In reply to :aceman from comment #6)
> So make the default identities bold. I wanted to do a border too, but it
> wasn't ignored. Am I doing something wrong? Maybe there can't be border set
> on menulist items?

Do you just want a <menuseparator>?
Squib, that could be a solution too. I can try that.
(In reply to neil@parkwaycc.co.uk from comment #9)
> >+        item.setAttribute("class", "defaultIdentity");
> item.setAttribute("default", "true"); also works.
How does this work?
Attached patch patch v2 (obsolete) — Splinter Review
New version, incorporating the goodies from squib and Neil.
Attachment #663159 - Attachment is obsolete: true
Attachment #663159 - Flags: ui-review?(bwinton)
Attachment #663757 - Flags: ui-review?(bwinton)
Comment on attachment 663757 [details] [diff] [review]
patch v2

If we only have one identity for each account (which I believe is the usual case), then the lines look a little weird, and make the list take up twice as much space.  Could we only have the lines above and below if there is more than one identity for the account?  Also, what happened to bolding the default account (which we seem to do in the Manage Identities pane, so I think we should be consistent here if at all possible).

ui-r- so that I can check out the other two options.

Thanks,
Blake.
Attachment #663757 - Flags: ui-review?(bwinton) → ui-review-
Depends on: 793819
Attached patch patch v3 (obsolete) — Splinter Review
So this should fix the separators and I have filed bug 793819 to sort out the bold styling of default=true globally.
Attachment #663757 - Attachment is obsolete: true
Attachment #664172 - Flags: ui-review?(bwinton)
Comment on attachment 664172 [details] [diff] [review]
patch v3

Well, this is close, but if I have an account with two items, and then an account with one item, the separator doesn't seem to show up, as you can see in https://dl.dropbox.com/u/2301433/Screenshots/AddressLines.png

ui-r=me with that fixed.

(And, as a code note, I think you might want to move the separator check out of the "for (let i = 0; i < identities.length; i++)" loop, since we only need to insert a separator once per account.)

Thanks,
Blake.
Attachment #664172 - Flags: ui-review?(bwinton) → ui-review+
Attached patch patch v4 (obsolete) — Splinter Review
Attachment #664172 - Attachment is obsolete: true
Attachment #666628 - Flags: ui-review?(bwinton)
Attachment #666628 - Flags: review?(mkmelin+mozilla)
Comment on attachment 666628 [details] [diff] [review]
patch v4

(Since you fixed the only thing I complained about, you could have carried forward the ui-r=me, but just to make it official, ui-r=me.  ;)

Thanks,
Blake.
Attachment #666628 - Flags: ui-review?(bwinton) → ui-review+
Comment on attachment 666628 [details] [diff] [review]
patch v4

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

Loos good, aceman! r=mkmelin

::: mail/components/compose/content/MsgComposeCommands.js
@@ +3230,5 @@
>    }
>  }
>  
>  function queryISupportsArray(supportsArray, iid) {
> +  return toArray(fixIterator(supportsArray, iid));

Just drop this method and inline the two callers.
Attachment #666628 - Flags: review?(mkmelin+mozilla) → review+
Attached patch patch v5Splinter Review
Ok, thanks. Planned to do that in other bug but no problem with that.
Attachment #666628 - Attachment is obsolete: true
Attachment #666667 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/a3aa5a852740
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 18.0
You need to log in before you can comment on or make changes to this bug.