Closed Bug 776135 Opened 12 years ago Closed 12 years ago

Social toolbar styling on Windows

Categories

(Firefox Graveyard :: SocialAPI, defect)

16 Branch
x86_64
Windows 7
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Firefox 17

People

(Reporter: todd, Assigned: jaws)

Details

(Whiteboard: [Fx16])

Attachments

(3 files, 5 obsolete files)

Attached image Screenshot
The Social API toolbar is not displaying properly on Windows.
Attachment says it all.
Attached patch css patch (obsolete) — Splinter Review
fixes css for provider button icon
Assignee: nobody → mixedpuppy
Attachment #644531 - Flags: review?(gavin.sharp)
Whiteboard: [Fx16]
Attachment #644531 - Flags: review?(gavin.sharp) → review?(jaws)
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?
Attachment #644531 - Flags: review?(jaws) → review-
Component: SocialAPI → Theme
Taking this as requested.
Assignee: mixedpuppy → jaws
Status: NEW → ASSIGNED
(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)
Assignee: jaws → nobody
Component: Theme → SocialAPI
Summary: Toolbar styling on Windows → Social toolbar styling on Windows
Assignee: nobody → jaws
Attached patch WIP Patch (obsolete) — Splinter Review
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.
Attachment #644531 - Attachment is obsolete: true
Attached patch Patch (obsolete) — Splinter Review
This patch cleans up some of the CSS and gets the button to match the other Australis toolbar buttons in terms of styling.
Attachment #645959 - Attachment is obsolete: true
Attachment #646233 - Flags: review?(gavin.sharp)
Attachment #646233 - Flags: review?(dao)
Attached patch Patch v1.1 (obsolete) — Splinter Review
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.
Attachment #646233 - Attachment is obsolete: true
Attachment #646233 - Flags: review?(gavin.sharp)
Attachment #646233 - Flags: review?(dao)
Attachment #646363 - Flags: review?(dao)
Attached patch Patch v1.2 (obsolete) — Splinter Review
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.
Attachment #646363 - Attachment is obsolete: true
Attachment #646363 - Flags: review?(dao)
Attachment #646425 - Flags: review?(dao)
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.
Attached patch Patch v1.3Splinter Review
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;
Attachment #646425 - Attachment is obsolete: true
Attachment #646425 - Flags: review?(dao)
Attachment #646460 - Flags: review?(gavin.sharp)
Attachment #646460 - Flags: review?(dao)
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).
Attachment #646460 - Flags: review?(gavin.sharp)
Attachment #646460 - Flags: review?(dao)
Attachment #646460 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/8b96a33ecbd2
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
Product: Firefox → Firefox Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: