Last Comment Bug 631007 - [Mac default] editBMPanel css enhancements
: [Mac default] editBMPanel css enhancements
Status: RESOLVED FIXED
:
Product: SeaMonkey
Classification: Client Software
Component: Themes (show other bugs)
: Trunk
: x86 Mac OS X
: -- normal (vote)
: seamonkey2.1b3
Assigned To: Stefan [:stefanh]
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-02-02 13:25 PST by Stefan [:stefanh]
Modified: 2011-02-21 08:39 PST (History)
0 users
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Change a few rules (2.37 KB, patch)
2011-02-02 13:38 PST, Stefan [:stefanh]
no flags Details | Diff | Splinter Review
Now with "#editBMPanel_rows > row > hbox > .expander-*" (2.29 KB, patch)
2011-02-18 08:55 PST, Stefan [:stefanh]
neil: review+
Details | Diff | Splinter Review

Description Stefan [:stefanh] 2011-02-02 13:25:49 PST
While doing bug 624295, some thing came up that I missed when I did the editBMPanel css for SM.
Comment 1 Stefan [:stefanh] 2011-02-02 13:38:58 PST
Created attachment 509222 [details] [diff] [review]
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 neil@parkwaycc.co.uk 2011-02-02 16:23:07 PST
(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] 2011-02-03 05:18:13 PST
(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 neil@parkwaycc.co.uk 2011-02-06 12:00:24 PST
(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 neil@parkwaycc.co.uk 2011-02-07 05:09:37 PST
Oops, I think I'm missing a # on those two lines.
Comment 6 Stefan [:stefanh] 2011-02-07 09:38:09 PST
> 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 neil@parkwaycc.co.uk 2011-02-11 04:42:06 PST
(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] 2011-02-18 08:55:34 PST
Created attachment 513486 [details] [diff] [review]
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 neil@parkwaycc.co.uk 2011-02-20 06:57:38 PST
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] 2011-02-21 08:39:07 PST
http://hg.mozilla.org/comm-central/rev/ea72ccd57b2e

Note You need to log in before you can comment on or make changes to this bug.