Give editMessageButton button appearance

RESOLVED FIXED in Thunderbird 3.3a3

Status

RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: Paenglab, Assigned: Paenglab)

Tracking

Trunk
Thunderbird 3.3a3
x86
Windows 7

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(7 attachments)

(Assignee)

Description

8 years ago
The editMessageButton, the button under message header in Drafts folder, looks under Win7 not like a system button.
(Assignee)

Comment 1

8 years ago
Created attachment 504812 [details] [diff] [review]
Give button system appearance

Patch giving -moz-appearance: button
Assignee: nobody → richard.marti
Attachment #504812 - Flags: review?(nisses.mail)
Why is there "padding-top: 0;" and "padding-bottom: 0;"?
Couldn't spot any difference without them on my system.
I also wonder if there is a good reason we don't style this bar as an infobar. Filed bug #632377 about that.
One thing with this button is that it looks very tall and thin compared to other similar buttons, removing the msgHeaderbutton in DOM inspector makes this goes away (and also removes the need to set it's appearence as a button I think), so we might be able to fix this in the xul.
Created attachment 510605 [details] [diff] [review]
Removing the class from the xul file instead

Will have to test this approach on the other systems (Linux and OSX) first.
Attachment #510605 - Attachment is patch: true
Attachment #510605 - Attachment mime type: application/octet-stream → text/plain
(Assignee)

Comment 6

8 years ago
(In reply to comment #2)
> Why is there "padding-top: 0;" and "padding-bottom: 0;"?
> Couldn't spot any difference without them on my system.

It's my fault, the padding's are needing the !important flag.

(In reply to comment #5)
> Created attachment 510605 [details] [diff] [review]
> Removing the class from the xul file instead
> 
> Will have to test this approach on the other systems (Linux and OSX) first.

This looks as the better approach and look good on Win and Linux.
Comment on attachment 510605 [details] [diff] [review]
Removing the class from the xul file instead

Setting the Amazing Extraordinaire Duo to up as reviewers.
Attachment #510605 - Flags: ui-review?(clarkbw)
Attachment #510605 - Flags: review?(bwinton)
Comment on attachment 504812 [details] [diff] [review]
Give button system appearance

Cancelling this review request whilst we find out if the other approach is the better option - you can always re-request later if need be.
Attachment #504812 - Flags: review?(nisses.mail)
Comment on attachment 510605 [details] [diff] [review]
Removing the class from the xul file instead

This seems reasonable to me, if Bryan likes the look of it on all three platforms.  (I'll post screenshots when it's done compiling to help his ui-r.)
Attachment #510605 - Flags: review?(bwinton) → review+
Created attachment 512855 [details]
New Linux styling.
Created attachment 512856 [details]
Old and new Mac styling.

This one looks a little odd to me.  The Junk button is also system-native, but it's on a different coloured background, so it's not as jarring an effect.  Changing the background for the Draft bar might be a solution.
(In reply to comment #11)
> Created attachment 512856 [details]
> Old and new Mac styling.
> 
> This one looks a little odd to me.  The Junk button is also system-native, but
> it's on a different coloured background, so it's not as jarring an effect. 
> Changing the background for the Draft bar might be a solution.

Agreed. Looking into it.
Created attachment 513477 [details]
New Windows 7 Styling.

And finally, Windows 7.  This seems a little off to me, too, for the same reason as the Mac (same background colour, different button style).
(Assignee)

Comment 14

8 years ago
(In reply to comment #13)
> Created attachment 513477 [details]
> New Windows 7 Styling.
> 
> And finally, Windows 7.  This seems a little off to me, too, for the same
> reason as the Mac (same background colour, different button style).

The button looks like before the patch. I'll post how it looks on my Win7.
(Assignee)

Comment 15

8 years ago
Created attachment 513504 [details]
New Windows7 Basic Styling

Buttons looks a lot better, but still the same background color.
(Assignee)

Comment 16

8 years ago
Created attachment 513506 [details]
New Windows7 Aero Styling

Now under Win7 Aero. The draft background is a slightly different.
(In reply to comment #13)
> And finally, Windows 7.  This seems a little off to me, too, for the same
> reason as the Mac (same background colour, different button style).

I wonder why you get that ugly looking button on your system. I have Vista here and I get the same result as Richard in my TB with the patch applied.
I think it would look good if we made this whole area an <infobar> as I mentioned in comment #3. I think that would take care of the button styling issue on OS X.
Might have some relation with bug #562048, but not necessary.
It could easily be something weird on my side.  I tried compiling three times in the VM, and each time it crashed the program.  :P  I'll re-compile on Tuesday (Monday is a holiday), making sure I've got the right patch, and see what I get.
Yeah, it took a couple more VM crashes, but I've got a build with this patch now, and it looks like the new-style button.  So we just need to wait until Bryan gets back, and get that ui-r.  :)

Later,
Blake.
Status: NEW → ASSIGNED
Comment on attachment 510605 [details] [diff] [review]
Removing the class from the xul file instead

Yeah, I like this approach best.
Attachment #510605 - Flags: ui-review?(clarkbw) → ui-review+
(Assignee)

Updated

8 years ago
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/0a76a05c3dfd
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.3a3
You need to log in before you can comment on or make changes to this bug.