Closed Bug 508728 Opened 11 years ago Closed 11 years ago

Restyle the Places window scope bar

Categories

(Firefox :: Theme, defect)

All
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 3.7a1
Tracking Status
status1.9.2 --- beta4-fixed

People

(Reporter: mstange, Assigned: mstange)

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Attached patch v1 (obsolete) — Splinter Review
This patch is on top of the one in bug 508739.
Attachment #392882 - Flags: review?(dao)
Attachment #392882 - Flags: review?(dao)
Attachment #392882 - Flags: review?(dao)
This seems to make some images unused. Any reason to not remove them?
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.
Attached patch v2Splinter Review
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+
(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
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3.7a1
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.