803880 - vertical separator between buttons in social toolbar cause hovered buttons to appear to have a double wide right border

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Asa Dotzler [:asa]]

Attachment: image showing the separator and hovered button state

We've got a vertical separator line between each of the buttons in the social toolbar. When you hover over one of the buttons, the button border on the left side overlaps the separator and the effect is a slightly darkened left border. That's not terribly ugly, but the right border is another story. The right border looks doubled up, two pixels wide, because the button's hovered border is one pixel off from the vertical separator's dark line. 

If we can make the hover state's right border overlap the separator line like the left border does, that could mask this hack a bit better. If we can't do that, maybe we can make the separators to the right and left of the currently hovered button disappear on hover and pressed state.

It seems like we don't have a proper widget here and it would be nice to figure out how to get this right. I'm OK with refining the hack, but we probably want to make a better widget here if we're going to support this in the long run.

Comment 1

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

Attachment: Patch

This patch turns off the separator for the hovered button and the adjacent button so there is no double border. Thanks for filing this bug Asa :)

Comment 2

[Dão Gottwald [:dao]]

Comment on attachment 673755 [details] [diff] [review]
Patch

>+@navbarLargeIcons@ > #social-toolbar-item > .toolbarbutton-1:hover:not(:first-child)::before,
>+@navbarLargeIcons@ > #social-toolbar-item > .toolbarbutton-1:hover + .toolbarbutton-1:not(:first-child)::before {
>+  display: none;
>+}

In the second selector, a .toolbarbutton-1 next to .toolbarbutton-1:hover can never be :first-child, so there's no point in adding :not(:first-child) to the selector.

You can also avoid this whole rule by not adding the border in the first place where you don't want it.

Comment 3

[Dão Gottwald [:dao]]

Attachment: patch v2

This should do it, I think. Jared, can you verify? I'm on Linux right now.

Comment 4

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

Comment on attachment 673777 [details] [diff] [review]
patch v2

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

Yeah this fixes it. Thanks!

Comment 5

[Dão Gottwald [:dao]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c4c392a00afe

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c4c392a00afe

Comment 7

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

Comment on attachment 673777 [details] [diff] [review]
patch v2

[Triage Comment]
let's get this polish fix into 17 (first social release)

Comment 8

[Dão Gottwald [:dao]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f3d7a94306ee
https://hg.mozilla.org/releases/mozilla-beta/rev/6621681b8f0d

Comment 9

[Simona Badau, Desktop QA]

Mozilla/5.0 (Windows NT 6.1; rv:17.0) Gecko/17.0 Firefox/17.0

Verified on Firefox 17 beta 6 that the double wide right border created on the social toolbar by the vertical separator between buttons and mouse hover is gone.

Comment 10

[Simona Badau, Desktop QA]

Mozilla/5.0 (Windows NT 6.1; rv:18.0) Gecko/20100101 Firefox/18.0

Verified as fixed on Firefox 18 beta 3 - on mouse hover there is no double wide right border created on the social toolbar by the vertical separator between buttons.