Last Comment Bug 776135 - Social toolbar styling on Windows
: Social toolbar styling on Windows
Status: RESOLVED FIXED
[Fx16]
:
Product: Firefox
Classification: Client Software
Component: SocialAPI (show other bugs)
: 16 Branch
: x86_64 Windows 7
: -- normal (vote)
: Firefox 17
Assigned To: Jared Wein [:jaws] (please needinfo? me)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-07-20 15:06 PDT by Todd
Modified: 2012-07-26 21:50 PDT (History)
3 users (show)
jaws: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Screenshot (7.12 KB, image/jpeg)
2012-07-20 15:06 PDT, Todd
no flags Details
css patch (1.28 KB, patch)
2012-07-20 16:57 PDT, Shane Caraveo (:mixedpuppy)
dao+bmo: review-
Details | Diff | Splinter Review
WIP Patch (4.83 KB, patch)
2012-07-25 17:15 PDT, Jared Wein [:jaws] (please needinfo? me)
no flags Details | Diff | Splinter Review
Patch (4.90 KB, patch)
2012-07-26 11:43 PDT, Jared Wein [:jaws] (please needinfo? me)
no flags Details | Diff | Splinter Review
Patch v1.1 (4.89 KB, patch)
2012-07-26 15:14 PDT, Jared Wein [:jaws] (please needinfo? me)
no flags Details | Diff | Splinter Review
Patch v1.2 (5.02 KB, patch)
2012-07-26 17:28 PDT, Jared Wein [:jaws] (please needinfo? me)
no flags Details | Diff | Splinter Review
Patch v1.3 (4.93 KB, patch)
2012-07-26 20:44 PDT, Jared Wein [:jaws] (please needinfo? me)
gavin.sharp: review+
Details | Diff | Splinter Review
Patch for checkin (4.85 KB, patch)
2012-07-26 21:48 PDT, Jared Wein [:jaws] (please needinfo? me)
no flags Details | Diff | Splinter Review

Description Todd 2012-07-20 15:06:01 PDT
Created attachment 644492 [details]
Screenshot

The Social API toolbar is not displaying properly on Windows.
Attachment says it all.
Comment 1 Shane Caraveo (:mixedpuppy) 2012-07-20 16:57:41 PDT
Created attachment 644531 [details] [diff] [review]
css patch

fixes css for provider button icon
Comment 2 Dão Gottwald [:dao] 2012-07-21 04:11:39 PDT
Comment on attachment 644531 [details] [diff] [review]
css patch

>+#social-provider-image .button-icon {
>+  max-height: 16px;
>+  max-width: 16px;
>+}
>+#social-provider-image .button-box {
>+  padding: 0;
>+  margin: 0;
>+  background: transparent;
>+  border: none;
>+}

>+#social-provider-image .button-menu-dropmarker {
>+  display: none;
>+}

Use the child selector.

>+#social-provider-image:-moz-focusring > .button-box {
>+  border: none;
>+}

Why is this button focusable? Keyboard access?
Comment 3 Jared Wein [:jaws] (please needinfo? me) 2012-07-23 11:08:52 PDT
Taking this as requested.
Comment 4 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-07-25 15:11:57 PDT
(easier to track this in the social component, since we're going to need to keep track of the bugs that we need to uplift)
Comment 5 Jared Wein [:jaws] (please needinfo? me) 2012-07-25 17:15:39 PDT
Created attachment 645959 [details] [diff] [review]
WIP Patch

This gets it close, but the borders on the [open=true] state aren't completely matching the :active state of other buttons on the toolbar yet.

There is potentially a lot of cleanup in the CSS for the social toolbar button but I don't want to include it all for this bug.
Comment 6 Jared Wein [:jaws] (please needinfo? me) 2012-07-26 11:43:32 PDT
Created attachment 646233 [details] [diff] [review]
Patch

This patch cleans up some of the CSS and gets the button to match the other Australis toolbar buttons in terms of styling.
Comment 7 Jared Wein [:jaws] (please needinfo? me) 2012-07-26 15:14:49 PDT
Created attachment 646363 [details] [diff] [review]
Patch v1.1

I noticed that the border-width was being increased in the "open" state, so I moved it to apply to all states of the button with a default "transparent" border-color.
Comment 8 Jared Wein [:jaws] (please needinfo? me) 2012-07-26 17:28:28 PDT
Created attachment 646425 [details] [diff] [review]
Patch v1.2

Sorry for the multiple patches, this one moves the border-radius to apply for all states and fixes the values being used in the :hover state.
Comment 9 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-07-26 19:58:06 PDT
Comment on attachment 646425 [details] [diff] [review]
Patch v1.2

>diff --git a/browser/themes/winstripe/browser.css b/browser/themes/winstripe/browser.css

>+#social-toolbar-button[open="true"] > .social-statusarea-container {

>+  box-shadow: 0 1px 1px hsla(210,54%,20%,.1) inset,
>+              0 0 1px hsla(210,54%,20%,.2) inset,
>+              /* allows winstripe-keyhole-forward-clip-path to be used for non-hover as well as hover: */

This comment seems unlikely to be relevant here?

> #social-statusarea-user {

>+  background-color: -moz-Dialog;
>+  color: black;

-moz-dialogtext?

>+  cursor: default;

Unnecessary.
Comment 10 Jared Wein [:jaws] (please needinfo? me) 2012-07-26 20:44:26 PDT
Created attachment 646460 [details] [diff] [review]
Patch v1.3

Thanks for the feedback Gavin, I've addressed the issues you found.

I also removed the cursor:pointer that was set on the #social-notification-icon-container since our toolbar buttons don't usually have cursor:pointer;
Comment 11 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-07-26 21:41:57 PDT
Comment on attachment 646460 [details] [diff] [review]
Patch v1.3

r=me with the change we discussed on IRC (remove the extra two lines from the box-shadow style on #social-toolbar-button[open="true"] > .social-statusarea-container).
Comment 12 Jared Wein [:jaws] (please needinfo? me) 2012-07-26 21:48:27 PDT
Created attachment 646467 [details] [diff] [review]
Patch for checkin
Comment 13 Jared Wein [:jaws] (please needinfo? me) 2012-07-26 21:50:10 PDT
https://hg.mozilla.org/mozilla-central/rev/8b96a33ecbd2

Note You need to log in before you can comment on or make changes to this bug.