[OS X] Visual polish for social provider user menu

VERIFIED FIXED in Firefox 17

Status

()

Firefox
SocialAPI
--
minor
VERIFIED FIXED
5 years ago
4 years ago

People

(Reporter: MattN, Assigned: MattN)

Tracking

unspecified
Firefox 19
All
Mac OS X
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(firefox17 verified, firefox18 verified)

Details

(Whiteboard: [Fx17])

Attachments

(2 attachments, 1 obsolete attachment)

Created attachment 668746 [details]
Screenshot of proposed changes

1) There is no menu separator below my name and profile photo on the social provider menu.  This makes the menu look unusual since the menu items have different heights.  Adding a menu separator would align more with the native OS X user switcher (see attachment 665650 [details]).

2) The hover effect for the menu item with my name and photo does not align with platform or website conventions with the hyperlink text-decoration. IMO it should highlight the whole menu item background with no text-decoration (like it does for the other items). See the native OS X user switcher.

3) IMO the left edge of the profile photo should align with the left edge of the text in the menu items below i.e. leave a gutter on the left for the checkmarks. (see attachment 665650 [details]).
Created attachment 668748 [details] [diff] [review]
WIP patch for OS X only

This patch turned out well as it was mostly deleting lines. I still need to get the XUL changes to work with Windows/Linux.  Having the large menu item (before and after my patch) doesn't seem natural for Windows so I think it may be best to switch to normal <menuitem>s there.  Let me know if this is going against existing designs/requirements.
Thanks Matt, this looks much better! Do you think we can we take this patch without affecting Windows/Linux?
Whiteboard: [Fx17]
(In reply to Matthew N. [:MattN] from comment #1)
> Having the large menu item
> (before and after my patch) doesn't seem natural for Windows so I think it
> may be best to switch to normal <menuitem>s there.  Let me know if this is
> going against existing designs/requirements.

It is the design that Boriss created, please touch base with her prior to making any changes on that.

Looks good, should be tried on win/lin.
Created attachment 669304 [details] [diff] [review]
v.1 Use normal menuitem hover and separator for OS X + Linux

Windows styling left the same. Linux and OS X work like normal menuitems and use a real separator. I would do the same for Windows but -moz-appearance: menuitem wouldn't show the XUL children and this bug was primarily for OS X styling.
Attachment #668748 - Attachment is obsolete: true
Attachment #669304 - Flags: review?(jaws)
Comment on attachment 669304 [details] [diff] [review]
v.1 Use normal menuitem hover and separator for OS X + Linux

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

r=me with the one nit below addressed.

It's worth noting that this patch also fixes a small issue with keyboard navigation in this menu. On Windows, the username item previously could not be reached by pressing up & down on the keyboard while the menu was open with the mouse hovering over the Show Sidebar menuitem. With this patch, the username can now be reached.

::: browser/themes/pinstripe/browser.css
@@ +4094,5 @@
>  #social-statusarea-user-portrait {
>    width: 32px;
>    height: 32px;
> +  margin: 4px;
> +  margin-left: 0;

This would most likely need to be -moz-margin-start: 0;
Attachment #669304 - Flags: review?(jaws) → review+
Thanks

https://hg.mozilla.org/integration/mozilla-inbound/rev/38ca5a784d75

Try results: https://tbpl.mozilla.org/?tree=Try&rev=9895d8cf22b2
Status: NEW → ASSIGNED
Flags: in-testsuite-

Comment 7

5 years ago
https://hg.mozilla.org/mozilla-central/rev/38ca5a784d75
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 19
Comment on attachment 669304 [details] [diff] [review]
v.1 Use normal menuitem hover and separator for OS X + Linux

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A
User impact if declined: User menuitem will look non-native.
Testing completed (on m-c, etc.): m-c and locally on three platforms
Risk to taking this patch (and alternatives if risky): low risk styling glitches on the social provider menu. Social only.
String or UUID changes made by this patch: none
Attachment #669304 - Flags: approval-mozilla-beta?
Attachment #669304 - Flags: approval-mozilla-aurora?

Comment 9

5 years ago
Comment on attachment 669304 [details] [diff] [review]
v.1 Use normal menuitem hover and separator for OS X + Linux

In support of our new feature.
Attachment #669304 - Flags: approval-mozilla-beta?
Attachment #669304 - Flags: approval-mozilla-beta+
Attachment #669304 - Flags: approval-mozilla-aurora?
Attachment #669304 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/e0979fb694aa
https://hg.mozilla.org/releases/mozilla-beta/rev/17264d533910
status-firefox17: --- → fixed
status-firefox18: --- → fixed
Verified fixed across all branches.
Status: RESOLVED → VERIFIED
status-firefox17: fixed → verified
status-firefox18: fixed → verified
You need to log in before you can comment on or make changes to this bug.