Closed
Bug 335973
Opened 19 years ago
Closed 18 years ago
Improve how we present lists of e-mail addresses in the message pane
Categories
(Thunderbird :: Mail Window Front End, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird2.0
People
(Reporter: mscott, Assigned: neil)
References
Details
(Keywords: fixed1.8.1, verified1.8.1.3)
Attachments
(9 files, 4 obsolete files)
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
mnyromyr
:
review+
mscott
:
superreview+
mscott
:
approval-thunderbird2+
|
Details | Diff | Splinter Review |
(deleted),
image/jpeg
|
Details | |
(deleted),
image/jpeg
|
Details | |
(deleted),
image/jpeg
|
Details |
Currently, when the to/cc lines in the message header pane contain multiple e-mail addresses, we obey the following rules:
1) We show the first 3 addresses regadless of how much horizontal space may be available.
2) You can click a toggle to expand the list to see all of the headers.
This behavior has several flaws including:
We hide information from the user for addresses which we have horizontal space for.
The ideal solution I would like to see is similar to what Mail.app does:
Show as many addresses as you can on one line without wrapping, then add a "n more..." text link at the end. When clicked, the row gets converted to our long address view with all of the addresses wrapping across as many lines as necessary. And if you resize the window, the list of addresses expands/contracts appropriately.
I don't know if we can track the resizing of the window enough to do this last part, but that would be pretty cool too.
Reporter | ||
Comment 1•19 years ago
|
||
A snapshot of a proof of concept I'm working on, just wanted to save it off. This patch lays out as many addresses as will fit without wrapping. If you try to resize the window, we fall back to wrapping the row, so it doesn't actually recaculate how many addresses now fit after re-sizing.
Comment 2•19 years ago
|
||
cool!
can you attach some screen shots?
this sounds like it will help with the scenario where you have a message with tons of To or Cc addresses and you can't see the message body (because the headers are taking up all the space).
I'm sure there is a bug on that (...but I can't find it. my bugzilla kung fu is weak!)
Reporter | ||
Comment 3•19 years ago
|
||
I don't think a screen shot would show anything interesting. Just imagine seeing more than the default 3 e-mail addresses on the line before you have to click on the twisty to see the rest of them.
The problem with this approach is we have to first add the address node to the DOM so we can caculate its resolved width, then we figure out if all the addresses so far including this one still fit (horizontally), and if we no longer fit, we remove it from the DOM. This means you see the element get added, then disappear again. So you get a flicker.
Also, getting the width property on a box object seems slow and we have to do it for every address in the line. Message display seems sluggish to me with the patch.
Reporter | ||
Comment 4•19 years ago
|
||
I wonder if we could do something like layout the first one, calculate the width, divide by the # of letters in the name then use that to approximate the width of future addresses without going through style resolution and without adding the nodes to the DOM. Hmmm.
Reporter | ||
Comment 5•19 years ago
|
||
updated proof of concept patch that fixes some issues with cached address nodes
Attachment #220275 -
Attachment is obsolete: true
Reporter | ||
Comment 6•19 years ago
|
||
This patch uses the width of the first e-mail address node we layout to estimate the width requirements for the remaining address nodes. This makes things a lot faster for me now as we avoid calls to boxObject.width for the remaining addresses and we can guess if an address will fit before adding it to the DOM, thus eliminating the jiggle effect I was seeing where the address gets added and then removed quickly.
If we ever guesstimate wrong, then the address just wraps to the next line which is no big deal if it happens.
In fact this patch is functional and peforms well for me. I've got a bug in my logic where sometimes a trailing comma gets left after the last e-mail address.
Comment 7•19 years ago
|
||
With a very large number of e-mail addresses in the To field, the Subject pane overflows and expands to cover up the entire message pane so you can't see the message. It also removes the status bar for some reason.
Ideally, the pane should never grow taller than 50% of the height of the message pane, and scroll the list of e-mail addresses.
I think this is a related issue, but should I post a separate bug report on it?
Reporter | ||
Comment 8•19 years ago
|
||
Neil, what do you think about this approach to solving this problem, particularly the changes to mailWidgets.xml. If this looks interesting to you, I'll clean up the patch, make the appropriate seamonkey changes and will re-request a review. I wanted some initial feedback on the solution before I went that far in case you have another way to solve it.
The goal:
To show as many e-mail addresses as will fit without wrapping at the current window width. Currently we only show the first 3 e-mail addresses, you have to click the expander toggle to see the rest of them.
The solution I've come up with so far looks like:
1) Layout the first e-mail address and calculate the resolved style width for it. Divide that width by the number of letters in the e-mail node to determine an average width per character.
2) Before adding additional e-mail address nodes, guess at the width of those nodes using the number we got from (1) and stop adding when we've run out of horizontal space.
3) If you resize the window and make it smaller, the flex on the widget causes the addresses to wrap to the next line. If you make it wider, we don't re-calculate the width, so you see the same # of addresses until you display another message.
Estimating the width an address will take up works ok because we don't have to be precise. If we guess incorrectly, the address just wraps to the next line.
Attachment #223510 -
Flags: review?(neil)
Comment 9•19 years ago
|
||
Couldn't we simply have a text box that is exactly one line high, with a hidden overflow? Then, when we expand the Subject Box, we can set the text box's overflow attribute to Scroll vertically. This will solve the problem I mentioned earlier (comment #7) and calculations about character width (IMO a hack) will not be neccessary - faster.
Assignee | ||
Comment 10•18 years ago
|
||
(In reply to comment #9)
>Couldn't we simply have a text box that is exactly one line high, with a hidden overflow?
We would still need a way to detect the overflow so that we can show the twisty.
Comment 11•18 years ago
|
||
(In reply to comment #10)
> We would still need a way to detect the overflow so that we can show the twisty.
By 'twisty', I assume you mean the plus/minus button to expand the list. I had two different ideas that could be implemented:
Is there some way of measuring the e-mail list's internal height? And if this height is greater than the container's height, then we know there is a text wrap?
Alternatively, we could put every e-mail address into a span-like element. Get each element's width, and keep adding these widths until this value is greater than the width of the message pane.
I hope that inspires...
Assignee | ||
Comment 12•18 years ago
|
||
The email addresses are all shown on one line. However if they overflow then a twisty appears at the end of the line that allows the addresses to wrap.
Assignee | ||
Comment 13•18 years ago
|
||
This is a SeaMonkey version but the styles are obviously portable.
Bonus points for spotting my laziness.
Attachment #227830 -
Attachment is obsolete: true
Attachment #227891 -
Flags: superreview?(mscott)
Attachment #227891 -
Flags: review?(mnyromyr)
Reporter | ||
Comment 14•18 years ago
|
||
Comment on attachment 227891 [details] [diff] [review]
Proposed patch
I liked where this patch is going. Some comments:
1) I don't like having the twisty on the right where it moves with the addresses, I liked having it either in the old location (to the left of the header), or maybe replacing the twisty with a clickable label that says "More..." similar to mail.app.
2)I'm not sure I like how addresses get truncated in mid letter. it would be really nice if we just don't show the address instead of only showing part of it.
Reporter | ||
Comment 15•18 years ago
|
||
Comment on attachment 223510 [details] [diff] [review]
simpler fix
clearing the review request since we may not try to solve the problem this way.
Attachment #223510 -
Flags: review?(neil)
Assignee | ||
Comment 16•18 years ago
|
||
I worked out what was wrong with my original "show only whole addresses" patch: the min-height was causing the display to be incorrect. However you may now think that the headers are now too close together but we could try adjusting the margins to compensate (currently .25em but maybe we could make it, say, 5px). I also moved the twisty back to its original position as requested by Scott; I had only moved it because I thought it made sense with the truncated addresses.
Assignee: mscott → neil
Attachment #227891 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #228934 -
Flags: superreview?(mscott)
Attachment #228934 -
Flags: review?(mnyromyr)
Attachment #227891 -
Flags: superreview?(mscott)
Attachment #227891 -
Flags: review?(mnyromyr)
Reporter | ||
Comment 17•18 years ago
|
||
This new patch is working very well for me. I think I like it better with the margin at 5px (.5em also worked at least for me).
I get some messages where we show the twisty even though all of the addresses fit. If I force any kind of action like resizing the window slightly, the underflow event fires and the toggle gets hidden. I wonder if we should initially hide the twisty before we fill the node with addresses, waiting for the first overflow event to make it visible?
Assignee | ||
Comment 18•18 years ago
|
||
As well as a slightly larger margin, this also has a tweak to try and resolve Scott's spurious twisties.
Assignee | ||
Comment 19•18 years ago
|
||
Attachment #228993 -
Attachment is obsolete: true
Reporter | ||
Comment 20•18 years ago
|
||
Comment on attachment 228934 [details] [diff] [review]
Improved patch
clearing a review request since this patch is obsolete.
Attachment #228934 -
Flags: superreview?(mscott)
Reporter | ||
Comment 21•18 years ago
|
||
Comment on attachment 229011 [details] [diff] [review]
With better margins
Thanks for working on this Neil. this is going to be a great UI win.
I'd like to approve and get this landed on the 1.8.1 branch as well if Karsten is ok with it.
I'll land the Thunderbird theme changes for this after Neil checks it in.
Attachment #229011 -
Flags: superreview+
Updated•18 years ago
|
Attachment #228934 -
Flags: review?(mnyromyr)
Updated•18 years ago
|
Attachment #229011 -
Flags: review+
Assignee | ||
Comment 22•18 years ago
|
||
Comment on attachment 229011 [details] [diff] [review]
With better margins
Fix checked in.
Reporter | ||
Updated•18 years ago
|
Attachment #229011 -
Flags: approval-thunderbird2+
Assignee | ||
Comment 23•18 years ago
|
||
Comment on attachment 229011 [details] [diff] [review]
With better margins
Fix checked in to the 1.8 branch.
Reporter | ||
Comment 24•18 years ago
|
||
I landed the tb theme changes on the branch and trunk too.
Comment 25•18 years ago
|
||
This didn't seem too bad.
Comment 26•18 years ago
|
||
This makes fixing bug 9942 even more imperative.
Comment 27•18 years ago
|
||
I like this much better, but would prefer bug 9942 be fixed.
Comment 28•18 years ago
|
||
*** Bug 270511 has been marked as a duplicate of this bug. ***
Comment 29•18 years ago
|
||
*** Bug 320550 has been marked as a duplicate of this bug. ***
Comment 30•18 years ago
|
||
verified fixed 1.8.1.3 on Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.3) Gecko/20070326 Thunderbird/2.0.0.0 Mnenhy/0.7.5.0 ID:2007032620
Keywords: verified1.8.1.3
You need to log in
before you can comment on or make changes to this bug.
Description
•