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)

x86
Windows XP
defect
Not set
normal

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)

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.
Attached patch proof of concept patch (obsolete) (deleted) — Splinter Review
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.
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!)
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.
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.
Attached patch updated proof of concept patch (deleted) — Splinter Review
updated proof of concept patch that fixes some issues with cached address nodes
Attachment #220275 - Attachment is obsolete: true
Attached patch a better fix (deleted) — Splinter Review
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.
Attached image Subject Box Overflow (deleted) —
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?
Attached patch simpler fix (deleted) — Splinter Review
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)
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.
(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.
(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...
Attached patch Work in progress (obsolete) (deleted) — Splinter Review
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.
Attached patch Proposed patch (obsolete) (deleted) — Splinter Review
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)
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.
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)
Attached patch Improved patch (deleted) — Splinter Review
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)
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?
Attached patch Tweaked patch (obsolete) (deleted) — Splinter Review
As well as a slightly larger margin, this also has a tweak to try and resolve Scott's spurious twisties.
Attached patch With better margins (deleted) — Splinter Review
Attachment #228993 - Attachment is obsolete: true
Comment on attachment 228934 [details] [diff] [review] Improved patch clearing a review request since this patch is obsolete.
Attachment #228934 - Flags: superreview?(mscott)
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+
Attachment #228934 - Flags: review?(mnyromyr)
Attachment #229011 - Flags: review+
Comment on attachment 229011 [details] [diff] [review] With better margins Fix checked in.
Attachment #229011 - Flags: approval-thunderbird2+
Comment on attachment 229011 [details] [diff] [review] With better margins Fix checked in to the 1.8 branch.
I landed the tb theme changes on the branch and trunk too.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Keywords: fixed1.8.1
Resolution: --- → FIXED
Attached image screenshot - SM before this fix (deleted) —
This didn't seem too bad.
Attached image screenshot - SM after this fix (deleted) —
This makes fixing bug 9942 even more imperative.
I like this much better, but would prefer bug 9942 be fixed.
Blocks: 345818
*** Bug 270511 has been marked as a duplicate of this bug. ***
*** Bug 320550 has been marked as a duplicate of this bug. ***
Depends on: 348717
Depends on: 346966
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.

Attachment

General

Created:
Updated:
Size: