Closed Bug 961506 Opened 8 years ago Closed 8 years ago

Add a margin for .menulist-label on menulist.folderMenuItem

Categories

(Thunderbird :: Theme, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 29.0

People

(Reporter: Paenglab, Assigned: Paenglab)

Details

Attachments

(2 files, 1 obsolete file)

In several folderlist menulists like in message filters the label is directly attached to the icon. #locationFolders had already a fix for this. Moving this to a global place like messenger.css would allow to fix this on all menulists with class folderMenuItem.
Attached patch patch (obsolete) β€” β€” Splinter Review
This patch moves the rule to messenger.css and makes it more global by using the selector menulist.folderMenuItem.
The second rule is to make the folderlist menuitems without a icon as tall as the ones with icons.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8362258 - Flags: ui-review?(josiah)
Attachment #8362258 - Flags: review?(josiah)
Could you give me a screenshot or something for the ui-review. Thanks.
Attached image comparison β€”
Comparison between no patch (left) and with patch (right). In the upper half the menuitem without icon (now as tall as the ones with icons). On bottom the the gap between icon and text on menulist button.

I need to say this will have full effect on all folder menulists when bug 878805 is also checked-in.
Comment on attachment 8362258 [details] [diff] [review]
patch

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

Looks good to me. ui-r+ and r+ with one comment.

::: mail/themes/windows/mail/messenger.css
@@ +158,5 @@
> +%ifndef WINDOWS_AERO
> +  padding-top: 2px;
> +  padding-bottom: 2px;
> +%endif
> +%ifdef WINDOWS_AERO

You should be able to shorten this to:

%ifndef WINDOWS_AERO
  stuff
%else
  stuff
%endif

But double check that it works. We only use the %else in IM theme code, so I can't be sure.
Attachment #8362258 - Flags: ui-review?(josiah)
Attachment #8362258 - Flags: ui-review+
Attachment #8362258 - Flags: review?(josiah)
Attachment #8362258 - Flags: review+
Attached patch Patch for check-in β€” β€” Splinter Review
(In reply to Josiah Bruner [:JosiahOne] from comment #4)
> You should be able to shorten this to:
> 
> %ifndef WINDOWS_AERO
>   stuff
> %else
>   stuff
> %endif

Yeah, this is a lot cleaner.
Attachment #8362258 - Attachment is obsolete: true
Attachment #8363137 - Flags: ui-review+
Attachment #8363137 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/bbdeef3a2990
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 29.0
You need to log in before you can comment on or make changes to this bug.