631007 - [Mac default] editBMPanel css enhancements

Status: RESOLVED FIXED

(SeaMonkey :: Themes, defect)

Description

[Stefan [:stefanh]]

While doing bug 624295, some thing came up that I missed when I did the editBMPanel css for SM.

Comment 1

[Stefan [:stefanh]]

Attachment: Change a few rules

Neil, I figured you could look at this, since it's very straight-forward and it's basically just some small code changes. Feel free to defer it to Karsten if you don't want to look at it.

Some explanations re the changes:
-.expander-up,
-.expander-down:hover:active {
-  list-style-image: url("chrome://navigator/skin/icons/hud-style-expander-open.png") !important;
+.expander-up#editBMPanel_foldersExpander,
+.expander-up#editBMPanel_tagsSelectorExpander {
+  list-style-image: url("chrome://navigator/skin/icons/hud-style-expander-open.png");

Dao pointed out that it's not so good using the same class as in the places part. He didn't liked this (class or id first?), but I prefer this over the variant with the descendant selector. Not having a hover:active icon is bug 624639.

-  list-style-image: url("chrome://navigator/skin/icons/hud-style-check-box-checked.png");
+  background-image: url("chrome://navigator/skin/icons/hud-style-check-box-checked.png");

I never noticed that the global/ rules used a background-image when I ported the css from browser/ :-/

Comment 2

[[e-mail]]

(In reply to comment #1)
> -.expander-up,
> -.expander-down:hover:active {
> -  list-style-image:
> url("chrome://navigator/skin/icons/hud-style-expander-open.png") !important;
> +.expander-up#editBMPanel_foldersExpander,
> +.expander-up#editBMPanel_tagsSelectorExpander {
> +  list-style-image:
> url("chrome://navigator/skin/icons/hud-style-expander-open.png");
> 
> Dao pointed out that it's not so good using the same class as in the places
> part. He didn't liked this (class or id first?), but I prefer this over the
> variant with the descendant selector.
So, you've got multiple expanders, but you only want to style two, and their class changes when you toggle them?

Comment 3

[Stefan [:stefanh]]

(In reply to comment #2)
> So, you've got multiple expanders, but you only want to style two, and their
> class changes when you toggle them?

Yes to both (I only want to style the 2 in the navigator bookmarks panel). Right now, I just override the the editBookmarksOverlay.css ones by using the same class. To avoid that anyone that uses those expanders (extension authors, for example) and relies on style rules from navigator.css, I want to make the styles editBookmarkPanel-unique.

Comment 4

[[e-mail]]

(In reply to comment #3)
> (In reply to comment #2)
> > So, you've got multiple expanders, but you only want to style two, and their
> > class changes when you toggle them?
> 
> Yes to both (I only want to style the 2 in the navigator bookmarks panel).

So might I suggest you use
editBMPanel_rows > row > hbox > .expander-up,
editBMPanel_rows > row > hbox > .expander-down:hover:active {

Comment 5

[[e-mail]]

Oops, I think I'm missing a # on those two lines.

Comment 6

[Stefan [:stefanh]]

> So might I suggest you use
> editBMPanel_rows > row > hbox > .expander-up,
> editBMPanel_rows > row > hbox > .expander-down:hover:active {

Out of curiosity, what's wrong with .myClass#myId { (as it seems much more unique)?

Comment 7

[[e-mail]]

(In reply to comment #6)
> > So might I suggest you use
> > editBMPanel_rows > row > hbox > .expander-up,
> > editBMPanel_rows > row > hbox > .expander-down:hover:active {
> Out of curiosity, what's wrong with .myClass#myId { (as it seems much more
> unique)?
This makes it clearer that we're only styling expanders in the bookmark panel.

Comment 8

[Stefan [:stefanh]]

Attachment: Now with "#editBMPanel_rows > row > hbox > .expander-*"

(In reply to comment #7)
> This makes it clearer that we're only styling expanders in the bookmark panel.

I think the editBMPanel prefix makes it clear... but sure, your suggestion saves one line of code for each rule:
#editBMPanel_rows > row > hbox > .expander-up
versus
.expander-up#editBMPanel_foldersExpander,
.expander-up#editBMPanel_tagsSelectorExpander"

I understand if you think the .class#Id combo is ugly, though ;-)

Comment 9

[[e-mail]]

Comment on attachment 513486 [details] [diff] [review]
Now with "#editBMPanel_rows > row > hbox > .expander-*"

Not sure whether I mentioned this but the other approach is to give those to expanders a second class you can use to help with the theming. (You would need to be careful to update the toggle code to be aware of the additional class.)

Comment 10

[Stefan [:stefanh]]

http://hg.mozilla.org/comm-central/rev/ea72ccd57b2e