Nit: Improve header pane metrics (e.g. "other actions" button too big, fix right alignment, reduce vertical space)

RESOLVED FIXED in Thunderbird 3.0rc1

Status

Thunderbird
Message Reader UI
P3
minor
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: Thomas D. (currently busy elsewhere), Assigned: andreasn)

Tracking

({polish})

Trunk
Thunderbird 3.0rc1
All
Windows XP
polish
Dependency tree / graph
Bug Flags:
blocking-thunderbird3 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [no l10n impact])

Attachments

(6 attachments, 7 obsolete attachments)

15.40 KB, image/gif
Details
4.67 KB, image/gif
Details
5.70 KB, image/gif
Details
10.84 KB, image/png
Details
3.67 KB, patch
andreasn
: review+
clarkbw
: ui-review+
Details | Diff | Splinter Review
67.67 KB, image/png
Details
Created attachment 405354 [details]
Screenshot of headerpane as of 2009-10-08, with metrics

Nit: Header pane metrics can still be fine-tuned to look neater and save space.

1) More-actions button, my personal enemy, is two pixels higher on hover than the rest of the buttons (please shrink the vspace-eater, and reduce whole header pane vertically from bottom by same measure).
More-actions button also looks too long on the right, besides the dropdown arrow (shrink from right by some px).
2) Right-align of buttons, date, and more-actions-button on hover(!) should be fixed (start with pushing button row 6px to the right).
3) Consider pushing buttons up 2-3px, and release that vspace.

We're still wasting considerable vspace (currently, it would make no difference in terms of used vspace to have button toolbar (including other actions) /above/ all other headers). It's tricky, and it'll never look right. But it can be improved...

Ultimately, resizable and dockable would be great.
I kinda suspect this wouldn't block Tb3, but that's Bryan's call to make...
Flags: blocking-thunderbird3?
Keywords: polish
(Reporter)

Comment 2

9 years ago
Created attachment 405431 [details]
Screenshot of header buttons, junk button width too small

Another nit that should be fixed:
- in icon-only mode, all buttons are 26px wide and look square (good)
- but junk button is only 22px wide, and looks squeezed (bad)
I don't even know how this can happen, because they're all 16x16 icons, or not?
-> please ensure iconic junk button looks square like all the other iconic buttons (26px wide)
(Reporter)

Comment 3

9 years ago
Created attachment 405433 [details]
Screenshot 3: with reply-to header: extra gap before to-header

More wasted vspace:
- mail with reply-to header
-> before to-header, unused extra gap of +3px (please remove this)
(Reporter)

Comment 4

9 years ago
Related bug 460647 - Focussing contacts causes respective header and label to move down some px

Miraculously, the proposed fix for that bug appears to be vspace-neutral on the whole although it adds a transparent 1px border to contacts so that they stop jumping up and down when focused/unfocused.
Depends on: 460647
(Reporter)

Updated

9 years ago
Summary: Nit: Improve header pane metrics (e.g. more actions button too big, right alignment, vertical space) → Nit: Improve header pane metrics (e.g. "other actions" button too big, fix right alignment, reduce vertical space)
(Assignee)

Updated

9 years ago
Assignee: nobody → nisses.mail
(Assignee)

Comment 5

9 years ago
this sounds like my area, so I'll see if I'm able to fix it.
Would block for an hour on this, but not for a day.
Flags: blocking-thunderbird3? → blocking-thunderbird3+
Priority: -- → P3

Updated

9 years ago
Whiteboard: [no l10n impact]
(Assignee)

Comment 7

9 years ago
Created attachment 406992 [details] [diff] [review]
patch to fix the button size problem

This patch fixes the button size, the alignment of the other button and the line height (it seems, but try it to be sure).
(Assignee)

Updated

9 years ago
Attachment #406992 - Flags: ui-review?(clarkbw)
Attachment #406992 - Flags: review?(dmose)
(Assignee)

Comment 8

9 years ago
Comment on attachment 406992 [details] [diff] [review]
patch to fix the button size problem

forgot to ask for review!
The first two stanzas of the patch look fine.  

I'm a little concerned about the 1.9em thing in the third stanza: it's hard to tell whether this is really the right fix, as there's no comment explaining why it's there in the first place.  It appears to have originally showed up with the "email bubbles" implemention in bug 456818.

I have this vague recollection of davida, Standard8, and clarkbw wanting extra space for when the mouseover bubbles were drawn around email addresses.  I've added them to the CC of this bugs in the hopes that one of them can confirm, deny, or, even better, clarify...

Note that the differing line heights bug, at least, happens on the other platforms too, not just windows.

Once it is clearer what the intended fix should be on this bug, I'd suggest checking whether all headerValueBoxes are guaranteed to have a singleline attribute, since, if not, this would likely only fix part of the problem.

Finally, since I'm travelling for the next little while, I suggest bwinton as a reviewer for your next iteration.

Updated

9 years ago
Attachment #406992 - Flags: review?(dmose) → review-
I'm not seeing any effects of this patch on the 'bubbles' (which is good)

I am seeing that I think we've regressed in that the keyboard focus ring shifts things down when we tab over the header values.  So somewhere we lost a margin of 1px in the height computation, which I guess is bug 460647.

I too don't understand the third chunk.  What happens w/o it?
(Assignee)

Comment 11

9 years ago
Created attachment 407752 [details] [diff] [review]
new patch without the last part

Here is a new patch without the last part. Still trying to figure out why the gap is so big.
Attachment #406992 - Attachment is obsolete: true
Attachment #406992 - Flags: ui-review?(clarkbw)
(Assignee)

Comment 12

9 years ago
(In reply to comment #9)
> Once it is clearer what the intended fix should be on this bug, I'd suggest
> checking whether all headerValueBoxes are guaranteed to have a singleline
> attribute, since, if not, this would likely only fix part of the problem.

It seems like the subject line does not have a headerValueBox and that might explain why the gap isn't at big between that and the surrounding line is smaller than others.
(Assignee)

Comment 13

9 years ago
Created attachment 408018 [details]
comparing with and without height removed

Screenshot of height: 1.9em; in .headerValueBox[singleline="true"] removed
(Assignee)

Comment 14

9 years ago
Created attachment 408845 [details] [diff] [review]
patch for gnomestripe

This makes all the entrys the same height (as far as I can see in DOM inspector). Qute and pinstripe patches not all done yet, as my Linux system is the only one with a dom inspector that works and it needs some more testing first.
Attachment #407752 - Attachment is obsolete: true
(Assignee)

Comment 15

9 years ago
Created attachment 408850 [details] [diff] [review]
qute + gnomestripe

Only need to make look how things look in pinstripe
Attachment #408845 - Attachment is obsolete: true
(Assignee)

Comment 16

9 years ago
Created attachment 408859 [details] [diff] [review]
gnomestripe+qute+pinstripe

Here is the patch for all three systems. Need to finish compiling the pinstripe part first to try it out in action first though.
Attachment #408850 - Attachment is obsolete: true
(Assignee)

Comment 17

9 years ago
Comment on attachment 408859 [details] [diff] [review]
gnomestripe+qute+pinstripe

Looks good on OSX to it seems, marking for review!
Attachment #408859 - Flags: ui-review?(clarkbw)
Attachment #408859 - Flags: review?(bwinton)
Whiteboard: [no l10n impact] → [no l10n impact][needs review bwinton,clarkbw]
Target Milestone: --- → Thunderbird 3.0rc1
Comment on attachment 408859 [details] [diff] [review]
gnomestripe+qute+pinstripe

>+++ b/mail/themes/gnomestripe/mail/messageHeader.css
>@@ -389,6 +389,9 @@
>   -moz-margin-start: 3px !important;
>   border: none !important;
>   background-color: transparent;
>+  padding-top: .1em;
>+  padding-bottom: .1em;
>+  margin-bottom: 3px !important;

Maybe put these padding and margin values up a couple of lines to be next to the padding and margin values that are already there.  (This applies to all three themes.)

>+++ b/mail/themes/pinstripe/mail/messageHeader.css
>@@ -414,12 +414,13 @@
>   -moz-appearance: none !important;
> 
>   padding: 0px !important;
>   margin: 0px !important;
>+  

I don't know if we need this extra line, but if we do, we should really remove the two trailing spaces.  ;)

>+++ b/mail/themes/qute/mail/messageHeader.css
>@@ -401,13 +400,16 @@
>   padding: 0px;
[snip…]
>+  padding-top: .1em;
>+  padding-bottom: .1em;

Also I think there's an easier way to set these...

From http://www.w3schools.com/css/css_padding.asp
  padding: .1em 0px;
should do the same thing.

r=me with these nits fixed.

Thanks,
Blake.
Attachment #408859 - Flags: review?(bwinton) → review+
Whiteboard: [no l10n impact][needs review bwinton,clarkbw] → [no l10n impact][needs ui-review clarkbw]
This seems like it got bitrotted for me :(
Whiteboard: [no l10n impact][needs ui-review clarkbw] → [no l10n impact][needs updated patch]
(Assignee)

Comment 20

9 years ago
Created attachment 409195 [details] [diff] [review]
updated patch

this should work better
Attachment #408859 - Attachment is obsolete: true
Attachment #409195 - Flags: ui-review?(clarkbw)
Attachment #409195 - Flags: review+
Attachment #408859 - Flags: ui-review?(clarkbw)
Whiteboard: [no l10n impact][needs updated patch] → [no l10n impact][needs review clarkbw]
(Assignee)

Comment 21

9 years ago
Created attachment 409318 [details] [diff] [review]
small update

Seems that when trying to fix the bitrot issues, I reintroduced the issue with the junk button not being big enough. This should work better.
Attachment #409195 - Attachment is obsolete: true
Attachment #409195 - Flags: ui-review?(clarkbw)
(Assignee)

Comment 22

9 years ago
Created attachment 409319 [details] [diff] [review]
sorry, last patch from wrong tree!

I suck. :)
This should work better.
Attachment #409318 - Attachment is obsolete: true
(Assignee)

Comment 23

9 years ago
Created attachment 409321 [details]
screenshots of header in gnomestripe, pinstripe and qute

to ease the UI review.
(Assignee)

Updated

9 years ago
Attachment #409319 - Flags: review+
(Assignee)

Comment 24

9 years ago
Comment on attachment 409319 [details] [diff] [review]
sorry, last patch from wrong tree!

whups! forgot to ask for ui-r again!
Attachment #409319 - Flags: ui-review?(clarkbw)
Attachment #409319 - Flags: ui-review?(clarkbw) → ui-review+
Keywords: checkin-needed
Hardware: x86 → All
Whiteboard: [no l10n impact][needs review clarkbw] → [no l10n impact]
Version: 3.0 → Trunk
Checked in:
http://hg.mozilla.org/comm-central/rev/32feaad3e42b
http://hg.mozilla.org/releases/comm-1.9.1/rev/6f3e060b0f8d
Status: NEW → RESOLVED
Last Resolved: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED

Updated

9 years ago
Depends on: 526646
You need to log in before you can comment on or make changes to this bug.