[Mac default] editBMPanel css enhancements

RESOLVED FIXED in seamonkey2.1b3

Status

SeaMonkey
Themes
RESOLVED FIXED
7 years ago
7 years ago

People

(Reporter: stefanh, Assigned: stefanh)

Tracking

Trunk
seamonkey2.1b3
x86
Mac OS X

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

7 years ago
While doing bug 624295, some thing came up that I missed when I did the editBMPanel css for SM.
(Assignee)

Comment 1

7 years ago
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/ :-/
Attachment #509222 - Flags: review?(neil)

Comment 2

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

Comment 3

7 years ago
(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

7 years ago
(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

7 years ago
Oops, I think I'm missing a # on those two lines.
(Assignee)

Comment 6

7 years ago
> 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

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

Comment 8

7 years ago
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 ;-)
Attachment #509222 - Attachment is obsolete: true
Attachment #513486 - Flags: review?(neil)
Attachment #509222 - Flags: review?(neil)
(Assignee)

Updated

7 years ago
Status: NEW → ASSIGNED

Comment 9

7 years ago
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.)
Attachment #513486 - Flags: review?(neil) → review+
(Assignee)

Comment 10

7 years ago
http://hg.mozilla.org/comm-central/rev/ea72ccd57b2e
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Target Milestone: seamonkey2.0b2 → seamonkey2.1b3
You need to log in before you can comment on or make changes to this bug.