776135 - Social toolbar styling on Windows

Status: RESOLVED FIXED

(Firefox Graveyard :: SocialAPI, defect)

Description

[Todd]

Attachment: Screenshot

The Social API toolbar is not displaying properly on Windows.
Attachment says it all.

Comment 1

[Shane Caraveo (:mixedpuppy)]

Attachment: css patch

fixes css for provider button icon

Comment 2

[Dão Gottwald [:dao]]

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)]

Taking this as requested.

Comment 4

[:Gavin Sharp [email: [e-mail]]]

(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)]

Attachment: 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)]

Attachment: 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)]

Attachment: 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)]

Attachment: 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: [e-mail]]]

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)]

Attachment: 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: [e-mail]]]

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)]

Attachment: Patch for checkin

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

https://hg.mozilla.org/mozilla-central/rev/8b96a33ecbd2