Closed Bug 499410 Opened 15 years ago Closed 15 years ago

message header view: get rid of extra horizontal whitespace

Categories

(Thunderbird :: Message Reader UI, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.0b4

People

(Reporter: dmosedale, Assigned: dmosedale)

References

Details

(Whiteboard: [no l10n impact])

Attachments

(8 files, 7 obsolete files)

20.50 KB, image/png
Details
3.19 KB, image/png
Details
17.84 KB, image/png
Details
257.59 KB, image/png
Details
172.50 KB, image/png
Details
153.70 KB, image/png
Details
128.72 KB, image/png
Details
35.09 KB, patch
dmosedale
: review+
dmosedale
: ui-review+
Details | Diff | Splinter Review
Spin off from bug 480623 comment 21
Flags: blocking-thunderbird3+
Shouldn't this be: Platform = All
(In reply to comment #1)
> Shouldn't this be: Platform = All

Probably - setting in advance.
OS: Mac OS X → All
Hardware: x86 → All
Version: unspecified → Trunk
Target Milestone: Thunderbird 3.0b3 → Thunderbird 3.0b4
Why isn't this covered by bug 456168 already?
Looks like a duplicate to me...
here's a screenshot to illustrate

right now I'm seeing 50px on the left that are unneeded though if I recall correctly that space might be difficult to remove due to the implementation.

there is at least 7px at the top of the header area that is unneeded white space, we're leaving a small 5px padding at the top.

I didn't go after the space at the bottom yet because I think that requires the moving of the Other Actions button (which is a good move).  However we want only 5px padding at the bottom of the header just like the top.
The following reduces the vertical space below the last header entry by about 10px (pick a higher value to reduce it by less) and also retains the relative location of the "Other Actions" so that it moves up as well:

#variousHeadersBox {
  padding-bottom: 0px;
}
Attached image left margin
"newsgroups" and "references" are a longer than what's depicted for other items thus far
Whiteboard: [no l10n impact]
One possible way to do this will be to replace the goofy CSS pseudo-boxery that's used to position the headers with a single <grid>.  Right now, headers other than the top and bottom-most header can extend to the right as far as the edge of the "other actions" dropdown.  A down-side of the <grid> plan is that all the headers would only be able to extend as far as the "reply" button.

So my current thinking is that I'm gonna try and play around with some CSS fu that uses the HTML box model in places and see where that gets me.  I'm also intrigued by the suggestion in bug 511625.
(In reply to comment #5)
> The following reduces the vertical space below the last header entry by about
> 10px (pick a higher value to reduce it by less) and also retains the relative
> location of the "Other Actions" so that it moves up as well:
> 
> #variousHeadersBox {
>   padding-bottom: 0px;
> }

Good catch; there's a version of this in patch form in bug 511491.  I'll be landing it shortly.
Attached patch patch (obsolete) — Splinter Review
Let's save some pixels on top...
Attached image mockup (obsolete) —
Attachment #395691 - Attachment is patch: true
Attachment #395691 - Attachment mime type: application/octet-stream → text/plain
(In reply to comment #9)
> Created an attachment (id=395691) [details]

OK, not worth reviewing, only 3px win in total; I will try with grid/c#7 approach.
Assignee: dmose → splewako
Few thoughts after experimenting with this a little more:

We can reduce padding for the whole widget, "5px from top" as described on attachment (id=393209) but not too much (buttons visual separation from borders), 2px max probably.

First two rows/lines (from+buttons and subject) should have the same height - this way we can avoid many problems with subject header.

Letters should be placed exactly on the same line (headers labels, values and the buttons ones) - this would result in a 2px move up for the from header.

Next rows (for to, date and other headers) could be smaller as they are less important.

Eventually, we can play with buttons/first two rows size to save even more space.

Comments/ideas? I will try to provide a patch + screenshot tomorrow.
Additionally I'm not sure yet if grid is really necessary/if we want it here.
(In reply to comment #12)
> Few thoughts after experimenting with this a little more:
> 
> We can reduce padding for the whole widget, "5px from top" as described on
> attachment (id=393209) [details] but not too much (buttons visual separation from
> borders), 2px max probably.

Seems reasonable.

> First two rows/lines (from+buttons and subject) should have the same height -
> this way we can avoid many problems with subject header.
>
> Letters should be placed exactly on the same line (headers labels, values and
> the buttons ones) - this would result in a 2px move up for the from header.
>
> Next rows (for to, date and other headers) could be smaller as they are less
> important.
> 
> Eventually, we can play with buttons/first two rows size to save even more
> space.
> 
> Comments/ideas? I will try to provide a patch + screenshot tomorrow.
> Additionally I'm not sure yet if grid is really necessary/if we want it here.

Sounds worth a try!  I look forward to seeing what you come up with...
Attached image mockup v2
Mockup based on attachment (id=393209), it goes probably far beyond the scope of this bug but I would like to try it anyway.

1. Subject in the first row (common complain).
2. 18px height drop in height when compared to 393209 without any text size manipulations (more savings?).
3. Subject only ~20% shorter from the current implementation (and I have some better workarounds then the "more" indicator in my mind).
4. It's far from perfect, especially less important headers and other actions button but also the first row vs. whole rest size ratio.

If we want to try this approach I can polish the mockup and start rewrite - the patch should be ready for review around Monday. (probably in a different bug)
Attachment #395691 - Attachment is obsolete: true
Attachment #395694 - Attachment is obsolete: true
(In reply to comment #14)
> Created an attachment (id=396720) [details]
> mockup v2

Big improvement! Some issues:

1. The subject looks inconsistently aligned without a "subject" title like the other rows. Yes, this greatly reduces the space saving for the subject.

2. The subject should wrap to 2 or 3 lines (also to compensate for #1). Wrapping would(?) dynamically push the subsequent rows down as needed.

3. The "other actions" (non-button button) should be the same distance from the bottom as the "to" line.

4. The order of the rows should probably be: subject, from, to, date (not date, to).

5. "Tags" should be shown on the same row as "date" - avoids wasting vertical space.

BTW: You might get some ideas/insight from the awesome "CompactHeader" add-on:
https://addons.mozilla.org/en-US/thunderbird/addon/13564
(In reply to comment #14)
> Created an attachment (id=396720) [details]
> mockup v2
> 
> Mockup based on attachment (id=393209) [details], it goes probably far beyond the scope
> of this bug but I would like to try it anyway.

We have two weeks left before code freeze in the last beta.  At this stage, the only things that can be on the blocking list are things that critical that we have addressed by ship time; we have to be very careful of scope creep.

Since this is a blocker, we absolutely need to keep it to the minimal set.  I think the idea of exploring a variety of options for the longer-term (3.1) is an excellent one.  I'd suggest doing that exploration in an extension, as you'll be able to iterate and get feedback on it much more rapidly than if you try and work through the slower and more conservative UI processes in the mainline code base.  

So for this bug, I'd like to request the following:

> 1. Subject in the first row (common complain).

This is a contentious issue, and for the current release, the call has already been made that it's going to stay where it is.  We can't let controversy and discussion about this get in the way of squishing down vertical space, so this can't happen in this bug.

> 2. 18px height drop in height when compared to 393209 without any text size
> manipulations (more savings?).

Very cool!  It's hard for me to figure out where that drop in height came from.  For your next mockup, can you post both a before and an after picture from the same screen (so that it has the same resolution, set of fonts, etc)?  That should make it easier to understand exactly what the changes are.

> 3. Subject only ~20% shorter from the current implementation (and I have some
> better workarounds then the "more" indicator in my mind).

Dropping the Subject header name feels out of scope here.

> 2. The subject should wrap to 2 or 3 lines (also to compensate for #1).
> Wrapping would(?) dynamically push the subsequent rows down as needed.

This would be a fine thing; so far it's been tricky to make XUL behave here.  There's another bug assigned to BenB about this, but he hasn't had time to poke at it in a while; seeing if it's possible to drive the patch in that bug further as well sounds worthwhile.

> 3. The "other actions" (non-button button) should be the same distance from the
> bottom as the "to" line.

I would tend to agree; it's possible that the right way to achieve, however, is to get rid of padding under the last header, which would save even more space.
One of the issues is that the buttonbox forces the 'from' line to have the height of the buttons... A better solution here would be to let the button-box span multiple rows of the header.
(In reply to comment #17)
> One of the issues is that the buttonbox forces the 'from' line to have the
> height of the buttons... 

Yes.

> A better solution here would be to let the button-box
> span multiple rows of the header.

No, not really, just think about long subject line.
> > A better solution here would be to let the button-box
> > span multiple rows of the header.

I believe this is exactly what the <grid> solution proposes to allow.
 
> No, not really, just think about long subject line.

I don't understand what you mean by this.  Can you elaborate?
(In reply to comment #3)
> Why isn't this covered by bug 456168 already?
> Looks like a duplicate to me...

rsx11: just saw this comment now; it probably could have been.  That said, this bug is a little better defined and more specific, and we've got mockups and patches here now, so I'd suggest running with this one.  Marking it as blocking that one.
Blocks: 456168
(In reply to comment #19)
> > No, not really, just think about long subject line.
> I don't understand what you mean by this.  Can you elaborate?

Have a look at attachment 395694 [details], the Subject line would be truncated
when putting all headers into the same grid.

P.S.: Sure, comment #20 is fine, I had just noticed the similarity.
Priority: -- → P1
rsx11m: indeed; tradeoffs abound.  This feels like one that's worth experimenting with.  Note that we've also talked about making the Subject header wrap, though the technical means to do that has yet been figured out; see bug 489609.
er "has yet to be figured out".
Stefan, what's the status on the stuff you're working on?
Attached patch hoist the headers vertically (obsolete) — Splinter Review
I haven't had any luck contacting Stefan, so here's a partial patch: this frees the From header from being inline with the button box and then hoists all the headers up vertically.  However, none of the headers can now extend under the button box at all.  

If we take a version of the patch in bug 465138 which allows icon-only buttons, that will at least offer folks who make that choice to see more headers.

After playing around for a while, I can't see any obvious way to make the button box act like it's floating using display: inline-block; I'll try and catch up with one of the layout guys in MV this week and see if they can offer advice, though.

In the meantime, I took one run at <grid>-ification and found problems due to the XBL is structured (the sort of inner-content binding currently in use doesn't really work with <grid>.  That said, restructuring the bindings looks pretty straightforward to me (famous last words!).  I'll be working on that today...
Whiteboard: [no l10n impact] → [no l10n impact][patch in progress]
How about only putting the first 2 or 3 headers into the same grid and allow any following headers to expand beyond that limit? Given that "Subject" is the 2nd heading, this would still imply that bug 489609 has to be solved.
Assignee: splewako → dmose
That's a good idea; doing two grids shouldn't be notably harder than doing one, assuming it's not difficult to make the first columns line up.
This has won back the vertical and horizontal space requested, in part by no longer allowing the Subject flow under the button box.  The two grids trick worked nicely, however, so To: and other headers still get to take advantage of that space.
Attachment #399134 - Flags: ui-review?(clarkbw)
Here's what "all headers" looks like.  The reason the "name" headers are moved over to the right is because the one with the longest name, "Content-transfer-encoding", is out of scroll.

You'll notice that the twisty on the message-id header has changed location from left of the name to left of the value.  This is an artifact of XBL changes that had to be made to accommodate <grid>.  This same thing will also be true for references and in-reply-to.  In my opinion, this looks and functions entirely reasonably.
Attached patch patch, v1 (obsolete) — Splinter Review
Here's the first cut at a complete patch.  It still needs some cleanup before it's ready for review, and so far I've only done the CSS changes for Linux.
Attachment #399081 - Attachment is obsolete: true
Whiteboard: [no l10n impact][patch in progress] → [no l10n impact][needs ui-review; patch iteration, review]
(In reply to comment #28)
> This has won back the vertical and horizontal space requested, in part by no
> longer allowing the Subject flow under the button box.

I like the elimination of extra vertical whitespace, but wondering what happened to Bryan's great wrapping subject.  Now the subject uses less than half of the horizontal width.  So for example an email from this bug with normal header view would look like:  "[Bug 499410] message hea", a mere 3.5 words.  In fact, the "mini" toolbar buttons and one of the addresses get more horizontal space than the most important Subject.  And there's wasted space after the More button and the date.  I'm just glad that this is still a work in progress, I hope.
Attachment #399140 - Attachment is obsolete: true
Attachment #399281 - Flags: ui-review?(clarkbw)
Attachment #399281 - Flags: review?(philringnalda)
Whiteboard: [no l10n impact][needs ui-review; patch iteration, review] → [no l10n impact][needs ui-review clarkbw; review philor]
Pete: to some degree it will be a work in progress until 3.0 ships, which is to say that there are hard tradeoffs to make here, and we're still experimenting with various options.
There is also bug 514452 suggesting to move the date header into the header-
button "column" and to move "other actions" directly underneath. This would change conditions here (probably extending the number of headers that need to
be included into the split grid respectively, depending on vertical size requirements, not necessarily making things easier here).

Re wrapping the subject line, both this bug and bug 514452 are blocking tb3+ whereas bug 489609 has been removed from the tb3.0 list and moved to 3.1.
This seems to ask for some coordination among these bugs...
After some experimentation and further discussion, the latest theory is that some of the other bugs (date & reply-to header-related in particular) will win back enough vertical space that we're ok just using this bug for the grid work that wins back horizontal space.
Attachment #399134 - Attachment is obsolete: true
Attachment #399281 - Attachment is obsolete: true
Attachment #399396 - Flags: ui-review?(clarkbw)
Attachment #399396 - Flags: review?(philringnalda)
Attachment #399281 - Flags: ui-review?(clarkbw)
Attachment #399281 - Flags: review?(philringnalda)
Attachment #399134 - Flags: ui-review?(clarkbw)
(In reply to comment #34)
> 
> Re wrapping the subject line, both this bug and bug 514452 are blocking tb3+
> whereas bug 489609 has been removed from the tb3.0 list and moved to 3.1.
> This seems to ask for some coordination among these bugs...

Good point; I thought that one was still a blocker, but evidently not.  I'm not sure how important it is to consider pulling back onto the blocker list at this point, but I'll keep it in mind.
Summary: message header view: get rid of extra vertical whitespace N → message header view: get rid of extra horizontal whitespace
Comment on attachment 399396 [details] [diff] [review]
patch, v3: moved subject back down into the lower grid

this certainly saves a lot of space compared to the old version.
Attachment #399396 - Flags: ui-review?(clarkbw) → ui-review+
Comment on attachment 399396 [details] [diff] [review]
patch, v3: moved subject back down into the lower grid

Sweetness.

You've got a bunch of trailing whitespace, please get yourself a clean (for that, since it's sort of bogus for CSS) bill of health from http://beaufour.dk/jst-review/?patch=399396

>-      <vbox id="expandedHeaders" flex="1">
>+     <hbox id="expandedHeadersTopBox" flex="1">

I think you're touching enough of the file already to make it worth reindenting, rather than use that cute cheat of a one-space indent, though if you don't think so, I can live with it (until the inevitable whitespace crusade patch comes along and reindents it anyway).
Attachment #399396 - Flags: review?(philringnalda) → review+
Whiteboard: [no l10n impact][needs ui-review clarkbw; review philor] → [no l10n impact]
(In reply to comment #39)
> Created an attachment (id=399401) [details]
> screenshot of windows header view

Having the subject run underneath those huge buttons diminishes its noticeability. A solution might be to make icon *images* the default (smaller and less dominant), and/or to put the buttons on the bottom row - closer to the message it acts upon.

Shouldn't the "Other Actions" be vertically aligned with the bottom-most item (the Date in your screen-shot)?

All labels should be in Initial-Caps (from --> From, reply --> Reply). Initial-Caps look much better and are consistent with all other UI in the universe (slight exaggeration). This seems like such a trivially easy fix that I wonder why this hasn't been done yet.

Another thing I've been pondering is: What do people who have poor vision (the elderly and/or extremely near-sighted) do when they want to have larger UI text? Do the buttons become even more huge, or does the button text not re-size (thus leaving them SOL)? How does the header respond to and look like with huge button text?
Attached patch patch, v4Splinter Review
Attachment #399396 - Attachment is obsolete: true
Attachment #399556 - Flags: ui-review+
Attachment #399556 - Flags: review+
(In reply to comment #41)
> (From update of attachment 399396 [details] [diff] [review])
> Sweetness.
> 
> You've got a bunch of trailing whitespace, please get yourself a clean (for
> that, since it's sort of bogus for CSS) bill of health from
> http://beaufour.dk/jst-review/?patch=399396

I fixed the only whitespace changes flagged by the jst-review bot, which were all in the XUL.  The complaints it had about the CSS appear to be a bug in the bot, which has convinced itself that a leading hyphen in CSS code is actually the second half of a binary minus operator in some language that supports such things.  :-)

> I think you're touching enough of the file already to make it worth
> reindenting, rather than use that cute cheat of a one-space indent, though if
> you don't think so, I can live with it (until the inevitable whitespace crusade
> patch comes along and reindents it anyway).

You're quite right, I had intended to do that before submitting for review, but spaced it.
Pushed, with one final whitespace change: <http://build.mozillamessaging.com/mercurial/comm-central/rev/2c57b60e2d42>.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Blocks: 469878
Attachment #399556 - Attachment is patch: true
Attachment #399556 - Attachment mime type: application/octet-stream → text/plain
Depends on: 515729
I know it's probably too late but for add-ons modifying the header pane it would make life easier if the to boxes expandedHeadersTopBox and expandedHeadersBottomBox would be within a vbox. 

Then the add-on could add stuff to the left or right of this vbox containing all standard header elements.
Herb, can you file a separate bug with that comment? I think it might make it, it's a safe change.  Please nominate it for blocking-tb3.
... Bug 516033 -  Add vbox enclosing expandedHeadersTopBox and expandedHeadersBottomBox
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: