508728 - Restyle the Places window scope bar

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Comment 1

[Markus Stange [:mstange]]

Attachment: v1

This patch is on top of the one in bug 508739.

Comment 2

[Markus Stange [:mstange]]

Screenshots of native scope bars are
http://developer.apple.com/documentation/UserExperience/Conceptual/AppleHIGuidelines/art/wn_dictionaryscopebar.jpg
http://developer.apple.com/documentation/UserExperience/Conceptual/AppleHIGuidelines/art/wn_finderscopebarrows.jpg

Comment 3

[Dão Gottwald [:dao]]

This seems to make some images unused. Any reason to not remove them?

Comment 4

[Markus Stange [:mstange]]

Nope, I probably just forgot it.
Though I remember that finding out which images are used and which are not is pretty confusing in Places code, since there's a bunch of #ifdef'd stuff that's hard to see through.

Comment 5

[Dão Gottwald [:dao]]

Comment on attachment 392882 [details] [diff] [review]
v1

>+#organizerScopeBar > toolbarbutton:hover,
>+#organizerScopeBar > toolbarbutton[checked="true"] {
...
>+  background-color: rgba(0, 0, 0, .32);
> }
> 
>+#organizerScopeBar > toolbarbutton[checked="true"],
>+#organizerScopeBar > toolbarbutton:active:hover {
>+  background-color: rgba(0, 0, 0, .37);
...
>+}
>+
>+#organizerScopeBar > toolbarbutton:active:hover {
>+  background-color: rgba(0, 0, 0, .5);
> }

I'd prefer this to be reorganized so that the background-colors aren't assigned twice for [checked="true"] and :active:hover, even if that results in a few more rules.

mxr searches will tell you whether the images from the removed lines are used elsewhere, no need to understand places code for that.

Comment 6

[Markus Stange [:mstange]]

Attachment: v2

Something must have changed in the meantime; these images are clearly unused now.
I remember that about a year ago I was having difficulties finding out where e.g. round-button-middle.png was used because there were still some CSS rules floating around that used that image, but I couldn't find out whether there was any XUL construct where they'd apply. But these rules are apparently gone now :)

> I'd prefer this to be reorganized so that the background-colors aren't assigned
> twice for [checked="true"] and :active:hover, even if that results in a few
> more rules.

Good call, the code is much clearer now.

Comment 7

[Dão Gottwald [:dao]]

Comment on attachment 395498 [details] [diff] [review]
v2

>+#organizerScopeBar > toolbarbutton:hover {
>+  color: #FFF;
>+  text-shadow: 0 1px rgba(0, 0, 0, .4);
>+  background-color: rgba(0, 0, 0, .32);
> }
> 
>+#organizerScopeBar > toolbarbutton[checked="true"] {
>+  color: #FFF !important; /* !imp because of [checked="true"] (see toolbarbutton.css) */
>+  text-shadow: 0 1px rgba(0, 0, 0, .4);
>+  background-color: rgba(0, 0, 0, .32);
>+  -moz-box-shadow: inset 0 1px 1px rgba(0, 0, 0, 0.4), 0 1px rgba(255, 255, 255, 0.4);
>+}
>+
>+#organizerScopeBar > toolbarbutton:active:hover {
>+  background-color: rgba(0, 0, 0, .5);
>+  -moz-box-shadow: inset 0 1px 1px rgba(0, 0, 0, 0.4), 0 1px rgba(255, 255, 255, 0.4);
> }

#organizerScopeBar > toolbarbutton:hover,
#organizerScopeBar > toolbarbutton[checked="true"] {
  color: #FFF !important; /* !imp because of [checked="true"] (see toolbarbutton.css) */
  text-shadow: 0 1px rgba(0, 0, 0, .4);
  background-color: rgba(0, 0, 0, .32);
}

#organizerScopeBar > toolbarbutton:active:hover,
#organizerScopeBar > toolbarbutton[checked="true"] {
  -moz-box-shadow: inset 0 1px 1px rgba(0, 0, 0, 0.4), 0 1px rgba(255, 255, 255, 0.4);
}

#organizerScopeBar > toolbarbutton:active:hover {
  background-color: rgba(0, 0, 0, .5);
}

Comment 8

[Markus Stange [:mstange]]

(In reply to comment #5)
> I'd prefer this to be reorganized so that the background-colors aren't assigned
> twice for [checked="true"] and :active:hover

Uhm, what was unclear about this? No idea why I had misunderstood it completely.

http://hg.mozilla.org/mozilla-central/rev/b1389d68fbd0

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 395498 [details] [diff] [review]
v2

a192=beltzner

Comment 10

[Markus Stange [:mstange]]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/f54f192817b7