Closed
Bug 514452
Opened 15 years ago
Closed 15 years ago
date header should not take up an entire line
Categories
(Thunderbird :: Message Reader UI, defect)
Thunderbird
Message Reader UI
Tracking
(Not tracked)
RESOLVED
FIXED
Thunderbird 3.0b4
People
(Reporter: clarkbw, Assigned: davida)
Details
(Whiteboard: [no l10n impact])
Attachments
(3 files, 6 obsolete files)
(deleted),
image/png
|
Details | |
(deleted),
image/png
|
Details | |
(deleted),
patch
|
dmosedale
:
review+
davida
:
ui-review+
|
Details | Diff | Splinter Review |
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•15 years ago
|
||
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).
Reporter | ||
Comment 2•15 years ago
|
||
(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•15 years ago
|
||
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.
Assignee: nobody → dmose
Flags: blocking-thunderbird3+
Target Milestone: Thunderbird 3.0b4 → Thunderbird 3.0rc1
Assignee | ||
Comment 4•15 years ago
|
||
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.
Assignee | ||
Comment 5•15 years ago
|
||
this screenshot shows the scrollbar that shows up for no good reason.
Assignee | ||
Comment 6•15 years ago
|
||
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.
Attachment #398543 -
Attachment is obsolete: true
Attachment #398544 -
Attachment is obsolete: true
Attachment #398585 -
Flags: ui-review?(clarkbw)
Attachment #398585 -
Flags: review?(dmose)
Assignee | ||
Comment 7•15 years ago
|
||
the good part
Assignee | ||
Comment 8•15 years ago
|
||
the less-good effect
Comment 9•15 years ago
|
||
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•15 years ago
|
||
Bug 498590 (and tangentially, bug 512036) are possibly related to the scrollbar issue.
Reporter | ||
Comment 11•15 years ago
|
||
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.
Attachment #398585 -
Flags: ui-review?(clarkbw) → ui-review+
Updated•15 years ago
|
Whiteboard: [no l10n impact][needs review dmose]
Assignee | ||
Comment 12•15 years ago
|
||
Actually, the patch isn't ready for review just yet (only pinstripe done).
Whiteboard: [no l10n impact][needs review dmose] → [no l10n impact][needs updated patch]
Assignee | ||
Updated•15 years ago
|
Assignee: dmose → david.ascher
Comment 13•15 years ago
|
||
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?
Reporter | ||
Comment 14•15 years ago
|
||
(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•15 years ago
|
||
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).
Reporter | ||
Comment 16•15 years ago
|
||
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?
Assignee | ||
Comment 17•15 years ago
|
||
Yes, I'll have an updated patch tomorrow most likely.
Assignee | ||
Comment 18•15 years ago
|
||
Attachment #398585 -
Attachment is obsolete: true
Attachment #398585 -
Flags: review?(dmose)
Reporter | ||
Comment 19•15 years ago
|
||
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 {
Reporter | ||
Comment 20•15 years ago
|
||
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.
Assignee | ||
Comment 21•15 years ago
|
||
incorporating those comments, and a minor css tweak on the mac dealing w/ scrollbar & padding.
Attachment #399748 -
Attachment is obsolete: true
Attachment #399848 -
Flags: ui-review?(clarkbw)
Reporter | ||
Updated•15 years ago
|
Attachment #399848 -
Flags: ui-review?(clarkbw) → ui-review+
Reporter | ||
Comment 22•15 years ago
|
||
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.
Assignee | ||
Updated•15 years ago
|
Attachment #399848 -
Flags: review?(mkmelin+mozilla)
Comment 23•15 years ago
|
||
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.
Assignee | ||
Comment 24•15 years ago
|
||
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.
Attachment #399848 -
Attachment is obsolete: true
Attachment #400061 -
Flags: ui-review+
Attachment #400061 -
Flags: review?(mkmelin+mozilla)
Attachment #399848 -
Flags: review?(mkmelin+mozilla)
Assignee | ||
Comment 25•15 years ago
|
||
Comment on attachment 400061 [details] [diff] [review]
patch addressing first set of review comments
dmose says he has bandwidth today
Attachment #400061 -
Flags: review?(mkmelin+mozilla) → review?(dmose)
Assignee | ||
Comment 26•15 years ago
|
||
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.
Attachment #400061 -
Attachment is obsolete: true
Attachment #400150 -
Flags: ui-review+
Attachment #400150 -
Flags: review?(dmose)
Attachment #400061 -
Flags: review?(dmose)
Assignee | ||
Updated•15 years ago
|
Whiteboard: [no l10n impact][needs updated patch] → [no l10n impact][needs review dmose]
Comment 27•15 years ago
|
||
Comment on attachment 400150 [details] [diff] [review]
revised patch
Looks good, r=dmose; waiting for thoughts from clarkbw on element ordering before landing.
Attachment #400150 -
Flags: review?(dmose) → review+
Comment 28•15 years ago
|
||
Checked in: <http://build.mozillamessaging.com/mercurial/comm-central/rev/bb06545ce2ea>. Thanks for the patch, David!
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Whiteboard: [no l10n impact][needs review dmose] → [no l10n impact]
Comment 29•15 years ago
|
||
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•15 years ago
|
||
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.
Updated•15 years ago
|
Target Milestone: Thunderbird 3.0rc1 → Thunderbird 3.0b4
You need to log in
before you can comment on or make changes to this bug.
Description
•