Restyle the Places window scope bar

RESOLVED FIXED in Firefox 3.7a1

Status

()

Firefox
Theme
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: mstange, Assigned: mstange)

Tracking

Trunk
Firefox 3.7a1
All
Mac OS X
Points:
---

Firefox Tracking Flags

(status1.9.2 beta4-fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Comment hidden (empty)
(Assignee)

Comment 1

9 years ago
Created attachment 392882 [details] [diff] [review]
v1

This patch is on top of the one in bug 508739.
Attachment #392882 - Flags: review?(dao)
(Assignee)

Updated

9 years ago
Attachment #392882 - Flags: review?(dao)
(Assignee)

Updated

9 years ago
Attachment #392882 - Flags: review?(dao)
This seems to make some images unused. Any reason to not remove them?
(Assignee)

Comment 4

9 years ago
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 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.
(Assignee)

Comment 6

9 years ago
Created attachment 395498 [details] [diff] [review]
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.
Attachment #392882 - Attachment is obsolete: true
Attachment #395498 - Flags: review?(dao)
Attachment #392882 - Flags: review?(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);
}
Attachment #395498 - Flags: review?(dao) → review+
(Assignee)

Comment 8

9 years ago
(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
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a1
(Assignee)

Updated

9 years ago
Attachment #395498 - Flags: approval1.9.2?
Comment on attachment 395498 [details] [diff] [review]
v2

a192=beltzner
Attachment #395498 - Flags: approval1.9.2? → approval1.9.2+
You need to log in before you can comment on or make changes to this bug.