514452 - date header should not take up an entire line

Status: RESOLVED FIXED

(Thunderbird :: Message Reader UI, defect)

Description

[Bryan Clark (DevTools PM) [:clarkbw]]

In the standard message header the message date takes up an entire line for what amounts to a small amount of needed horizontal space.

I'm suggesting the date field could be placed directly under the button box on the right hand side.  In the current layout a relatively similar amount of horizontal space is taken up by the other actions button so it won't be consuming more space than is already taken.

I would also assert that people can recognize dates by their format alone so we don't need to include the date header to label the entry.

Any other ideas for date field locations?

Comment 1

[Richard Marti (:Paenglab)]

Would that mean the date will be between the button box and the other actions? If yes, would it not look better, if the other actions is directly under the button box and the date under the other actions? So the interactive elements are together and not cluttering the header pane.

In my theme I masked the spacer over the other actions out and the button box and the other actions stay always together (also when showing all headers).

Comment 2

[Bryan Clark (DevTools PM) [:clarkbw]]

(In reply to comment #1)
> Would that mean the date will be between the button box and the other actions?
> If yes, would it not look better, if the other actions is directly under the
> button box and the date under the other actions?

That's a good option to try, maybe we can start with that arrangement.

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

Marking as blocking-thunderbird3+, Bryan, feel free to change if you disagree.
Moving to rc1 because I'm going to argue that we wouldn't actually stop b4 for this if it were the last bug standing, but I do like the idea of trying to pull it in to b4 if there's time.

This doesn't actually depend on bug 514452 (the vertical space bug), but it'll be easier to implement after that one lands, because we will have hopefully thrown out a bunch of icky CSS hackery.

Comment 4

[David Ascher (:davida)]

Attachment: first cut

I was feeling restless, so I took a stab at this one.  Mostly it works nicely, except that I get scrollbars for no obvious reason.  Putting up the patch in case someone has an idea.

Comment 5

[David Ascher (:davida)]

Attachment: screenshot (mac)

this screenshot shows the scrollbar that shows up for no good reason.

Comment 6

[David Ascher (:davida)]

Attachment: second cut

Totally, totally mysterious to me.  The _vertical_ scrollbars were caused by the _horizontal_ padding around _email addressed_, which I had added decades ago it seems, to support the on-hover effect having a nice consistent padding around the name.  That effect, IMO, is not worth the impact of the scrollbar, which seems to be triggered by the reduction in the number of lines (or ???).

This patch is only tested on mac -- I expect that we'll need to make similar removals of the padding on the gnomestripe and qute themes (see the changes needed to make alignment of non-email headerValues work).

Dan, I'd love your eyes on this patch, as you fought with this code before, so you may have some test cases that show problems w/ the approach.

I'm marking r? to get it on your radar, but it's not ready for checkin even if it were perfect due to the theme issues mentioned above.

I'll attach a screenshot or two.

Comment 7

[David Ascher (:davida)]

Attachment: screenshot showing the space savings and absence of scrollbar

the good part

Comment 8

[David Ascher (:davida)]

Attachment: screenshot showing the bubble hugging the left edge of the label

the less-good effect

Comment 9

[Dan Mosedale (:dmosedale, :dmose)]

I think what's going on there is that the current functionality of hiding the addresses is actually achieved with two boxes.  The inner box, which adds addresses horizontally, doesn't overflow, but instead wraps to the next line.  When wrap in the inner box happens, that causes the outer box to overflow vertically.

Comment 10

[Jim Porter (:squib)]

Bug 498590 (and tangentially, bug 512036) are possibly related to the scrollbar issue.

Comment 11

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 398585 [details] [diff] [review]
second cut

Position of the date looks great.

I think the loss of padding vs. the somewhat random scroll bar is a trade-off I'd take.

Comment 12

[David Ascher (:davida)]

Actually, the patch isn't ready for review just yet (only pinstripe done).

Comment 13

[rsx11m]

Looking at attachment 398586 [details], how does this truncate long lines
for the subject compared with attachment 399139 [details] in bug 499410?
Would it extend up to the date or cut off at the "Reply" button already?

Comment 14

[Bryan Clark (DevTools PM) [:clarkbw]]

(In reply to comment #13)
> Looking at attachment 398586 [details], how does this truncate long lines
> for the subject compared with attachment 399139 [details] in bug 499410?
> Would it extend up to the date or cut off at the "Reply" button already?

you can see in attachment 399139 [details] the margin on right hand side under the reply buttons (where the more label aligns against).  The date will sit in that vbox area that is already being used.  Roughly starting under the middle of the archive button.

Comment 15

[rsx11m]

Thanks, it looks like the revised patch for bug 499410 has resolved that issue
by putting everything back on individual rows. That hopefully avoids having unused space, by aligning all items against the left or right pane borders and allowing the heading on the left to fill up the row until an item on the right is met (buttons, header, or drop-down menu for the first three rows).

Comment 16

[Bryan Clark (DevTools PM) [:clarkbw]]

it would be tres good if we could get this fix in for b4 instead of sliding out to rc1.

david, are you going to continue on this patch?

Comment 17

[David Ascher (:davida)]

Yes, I'll have an updated patch tomorrow most likely.

Comment 18

[David Ascher (:davida)]

Attachment: updated patch, but not fully tested on windows.

Comment 19

[Bryan Clark (DevTools PM) [:clarkbw]]

This works for me however the old date used to have text selection and we probably don't want to regress that.

The following CSS change (windows only) gives you text selection and focus but doesn't give a context menu like the subject has.

diff --git a/mail/themes/qute/mail/messageHeader.css b/mail/themes/qute/mail/messageHeader.css
--- a/mail/themes/qute/mail/messageHeader.css
+++ b/mail/themes/qute/mail/messageHeader.css
@@ -177,16 +177,19 @@ description[selectable="true"]:focus > d
 
 #expandedButtonBox {
   -moz-padding-end: 6px;
   padding-top: 3px;
 }
 
 .dateLabel {
   -moz-padding-end: 3px;
+  -moz-user-select: text;
+  -moz-user-focus: normal;
+  cursor: text;
 }
 
 .hdrReplyButton {
   -moz-appearance: dualbutton;
   -moz-margin-end: 0 !important;
 }
 
 .hdrReplyButton > .button-menubutton-button {

Comment 20

[Bryan Clark (DevTools PM) [:clarkbw]]

also, from an extension developer perspective we might want to wrap the label in a <box> with an id like dateValueBox so people can sneak things in before or after it.

Comment 21

[David Ascher (:davida)]

Attachment: updated patch

incorporating those comments, and a minor css tweak on the mac dealing w/ scrollbar & padding.

Comment 22

[Bryan Clark (DevTools PM) [:clarkbw]]

Comment on attachment 399848 [details] [diff] [review]
updated patch

this looks good to me.  

it's surprising that the full date takes up more space (width) than the other actions buttons.

Comment 23

[Magnus Melin [:mkmelin]]

Comment on attachment 399848 [details] [diff] [review]
updated patch

Won't get to this until sunday at the earliest. 

A couple of comments:
>+ padding: 0!important;*

Add space

Are the margin-right rules supposed to be that? Not margin-end? If they should, add a comment in the file so it people won't wonder in the future.

Comment 24

[David Ascher (:davida)]

Attachment: patch addressing first set of review comments

Thanks, I'm not used to the RTL considerations yet.  

I may hand the review to someone else if we decide we want to squeeze it into b4.

Comment 25

[David Ascher (:davida)]

Comment on attachment 400061 [details] [diff] [review]
patch addressing first set of review comments

dmose says he has bandwidth today

Comment 26

[David Ascher (:davida)]

Attachment: revised patch

ok, so i cleaned up this patch to not deal with the scrollbar issues.  we therefore have spurious scrollbars with this patch (as we do w/o), but those will be dealt with by bug 513318.

Other changes which I guess we could take out but they seem like good hygiene:

1) make all themes set padding/margin on the outermost dom element, to try and bring them more in sync

2) move the border-radius from the :hover to the plain state for email addresses

I've tested this on mac, windows and linux.

Comment 27

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 400150 [details] [diff] [review]
revised patch

Looks good, r=dmose; waiting for thoughts from clarkbw on element ordering before landing.

Comment 28

[Dan Mosedale (:dmosedale, :dmose)]

Checked in: <http://build.mozillamessaging.com/mercurial/comm-central/rev/bb06545ce2ea>.  Thanks for the patch, David!

Comment 29

[IU]

Side-effect: Long subject lines are truncated, as they stop where the date begins.  Is there a follow-on to deal with this?

Comment 30

[rsx11m]

If you mean wrapping of the subject line, that's bug 489609 (and went off the blocker list for 3.0 recently, this may be good to revisit). In contrast to my assumption of comment #15, it appears though that further headers are also truncated at this alignment, even if they have free space to the right.