Closed Bug 626742 Opened 13 years ago Closed 13 years ago

Give editMessageButton button appearance

Categories

(Thunderbird :: Theme, defect)

x86
Windows 7
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 3.3a3

People

(Reporter: Paenglab, Assigned: Paenglab)

Details

Attachments

(7 files)

The editMessageButton, the button under message header in Drafts folder, looks under Win7 not like a system button.
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.
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
(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+
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.
Attached image 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).
(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.
Buttons looks a lot better, but still the same background color.
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+
Keywords: checkin-needed
Checked in: http://hg.mozilla.org/comm-central/rev/0a76a05c3dfd
Status: ASSIGNED → RESOLVED
Closed: 13 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.

Attachment

General

Created:
Updated:
Size: