Closed Bug 944945 Opened 7 years ago Closed 7 years ago

Improve vcard styling

Categories

(Thunderbird :: Theme, defect)

x86
All
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 28.0

People

(Reporter: jsbruner, Assigned: jsbruner)

Details

Attachments

(3 files, 3 obsolete files)

Attached image The issue. β€”
The current vcard box styling isn't very attractive. Not to mention the fact that on OS X there is a pretty major icon glitch. Let's make things nicer.
Attached patch Patch. (obsolete) β€” β€” Splinter Review
This updates the styling to make it a little more attractive, fixes the OS X bug, and makes the code 100% consistent on all platforms.

Flagging Richard for review.
Attachment #8340705 - Flags: ui-review?(richard.marti)
Attachment #8340705 - Flags: review?(richard.marti)
Attached image New Look. (obsolete) β€”
Here's a screenshot of the change.
Comment on attachment 8340705 [details] [diff] [review]
Patch.

Review of attachment 8340705 [details] [diff] [review]:
-----------------------------------------------------------------

I've only commented on the Linux changes as all three files are changing the same. r- because of the missing RTL rules and it isn't working as on your screenshot.

I like the use of the new image and it fixes the issue.

I'm not so happy with the box-shadow and only one rounded corner. On your screenshot it looks good on the bottom left, but on all my systems I have a 8px margin in body (do you have a userContent.css rule to set this or is this message setting 0px margin?). Also if the message is shorter than the view the vcard isn't at the bottom and makes it looking weird. ui-r- because of those.

What about giving all corners a 5px radius and a 1px ThreeDLightShadow border? And giving a background-color: rgba(0, 0, 0, 0.05) althought this wouldn't be visible on black message backgrounds.

::: mail/themes/linux/mail/messageBody.css
@@ +92,5 @@
>  
>  /* ::::: vcard ::::: */
>  
>  .moz-vcard-table {
> +  border-radius: 0px 5px 0px 0px; /* Round the top right corner. */

border-bottom-right-radius: 5px; would do the same. The other corners are already 0px.

This rule would need a additional rule for RTL languages.

@@ +97,2 @@
>    margin-top: 10px;
> +  box-shadow: -4px 4px 4px lightgray inset;

Maybe it would be better to use a system color to work properly on all themes.

@@ +107,5 @@
> +  margin-top: 2px;
> +  height: 32px;
> +  width: 32px;
> +  background-image: url("chrome://messenger/skin/addressbook/icons/contact-generic.png");
> +  background-size: 100% 100%; /* This allows us to ignore any @2x resolutions */

Nit: this comment isn't really needed on non-OS X platforms.
Attachment #8340705 - Flags: ui-review?(richard.marti)
Attachment #8340705 - Flags: ui-review-
Attachment #8340705 - Flags: review?(richard.marti)
Attachment #8340705 - Flags: review-
This screenshot shows the left-margin and how it looks with a short message not filling the view. When it would fill the view on bottom is the same margin.
Attached patch Patch. (obsolete) β€” β€” Splinter Review
Hopefully this looks a little better.

(And turns out the margin problem was related to that specific email, they overrode the margin)
Attachment #8340705 - Attachment is obsolete: true
Attachment #8340706 - Attachment is obsolete: true
Attachment #8340799 - Flags: ui-review?(richard.marti)
Attachment #8340799 - Flags: review?(richard.marti)
Attached patch Patch. β€” β€” Splinter Review
This patch switches to a flatter design. I think it does look a little nicer.
Attachment #8340799 - Attachment is obsolete: true
Attachment #8340799 - Flags: ui-review?(richard.marti)
Attachment #8340799 - Flags: review?(richard.marti)
Attachment #8340801 - Flags: ui-review?(richard.marti)
Attachment #8340801 - Flags: review?(richard.marti)
Comment on attachment 8340801 [details] [diff] [review]
Patch.

Review of attachment 8340801 [details] [diff] [review]:
-----------------------------------------------------------------

Yeah, this looks good.

Thank you Josiah!
Attachment #8340801 - Flags: ui-review?(richard.marti)
Attachment #8340801 - Flags: ui-review+
Attachment #8340801 - Flags: review?(richard.marti)
Attachment #8340801 - Flags: review+
https://hg.mozilla.org/comm-central/rev/47cc6378ca23
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 28.0
You need to log in before you can comment on or make changes to this bug.