Closed Bug 413953 Opened 17 years ago Closed 16 years ago

Modern is missing global/richlistbox.css

Categories

(SeaMonkey :: Themes, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: philip.chee, Assigned: philip.chee)

Details

Attachments

(2 files, 4 obsolete files)

With a richlistbox in modern, selected richlistitems are not styled differently from unselected items. So selections are effectively invisible. This patch adds richlistbox.css to modern/global/

Note: in case Bugzilla messes up patches containing UTF-8 characters "Simon Bünzli" should be "Simon Bünzli"
>+richlistbox {
>+  margin: 2px 4px;
>+  background-color: #FFFFFF;
>+  color: #000000;
>+}
>+
>+richlistbox[disabled="true"] {
>+  color: #999999;
>+}
>+
>+richlistitem[selected="true"] {
>+  background-color: #C7D0D9;
>+  color: #000000;
>+}

The fg/bg colours in the first three rules are from modern/listbox.css. For the third rule winstripe uses:
  background-color: -moz-cellhighlight;
  color: -moz-cellhighlighttext;
But I have no idea what modern should use for these system colours.

>+richlistbox:focus > richlistitem[selected="true"] {
>+  background-color: #B7DBEB;
>+  color: #000000;
>+}

Winstripe uses:
  background-color: Highlight;
  color: HighlightText;

Initially I copied the following from listbox.css:
  background-color: #424F63;
  color: #FFFFFF;
But I found the effect unpleasant.

>+richlistbox[seltype="multiple"]:focus > richlistitem[current="true"] {
>+  outline: 1px dotted #424F63;
>+  -moz-outline-offset: -1px;
>+}

I just made a wild guess Highlight ==> #424F63 here.

>+richlistbox[seltype="multiple"]:focus > richlistitem[current="true"][selected="true"] {
>+  outline: 1px dotted #F3D982; /* from winstripe */
>+}
>+

winstripe/pinstripe just says "/* TODO: find a suitable system color */". What would be a suitable colour for modern?

Comment on attachment 299118 [details] [diff] [review]
Add richlistbox.css to modern/global

You need to add the file to the jar.mn file.
Component: Themes → General
Product: Core → Mozilla Application Suite
>  Add richlistbox.css to modern/global
Sigh. Done.

>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>           Component|Themes                      |General
>             Product|Core                        |Mozilla Application Suite

Why?
Attachment #299118 - Attachment is obsolete: true
> >            What    |Removed                     |Added
> > ----------------------------------------------------------------------------
> >           Component|Themes                      |General
> >             Product|Core                        |Mozilla Application Suite
> Why?

1) Modern is in mozilla/suite --> MAS
2) There is no component matching "themes" --> General

It's not optional, of course. But I think it's bettern than core. When it comes to stuff in toolkit/themes, things are getting more complicated. I tend to file toolkit themes bugs in toolkit:XUL Widgets (files are in toolkit/ and themes do style XUL Widgets), but I know there are people using Core:Themes (toolkit themes are used by all products).
(In reply to comment #0)
> Note: in case Bugzilla messes up patches containing UTF-8 characters "Simon
> Bünzli" should be "Simon Bünzli"
Which parts of his file are you using, and why?

(In reply to comment #1)
> The fg/bg colours in the first three rules are from modern/listbox.css. For the
> third rule winstripe uses:
>   background-color: -moz-cellhighlight;
>   color: -moz-cellhighlighttext;
> But I have no idea what modern should use for these system colours.
I see nothing wrong with your reuse of the listbox colours.

> Initially I copied the following from listbox.css:
>   background-color: #424F63;
>   color: #FFFFFF;
> But I found the effect unpleasant.
Looks fine to me.

> >+richlistbox[seltype="multiple"]:focus > richlistitem[current="true"] {
> >+  outline: 1px dotted #424F63;
> >+  -moz-outline-offset: -1px;
> >+}
> I just made a wild guess Highlight ==> #424F63 here.
Why not do what listbox.css does instead?
(In reply to comment 4)
> 1) Modern is in mozilla/suite --> MAS
> 2) There is no component matching "themes" --> General

As I understand it Core->Themes is for MAS (and now SeaMonkey) themes. This dates back to the days when Mozilla was the only product/project. The b.m.o. reorganization bug will rename MAS->SeaMonkey and rename Core->Themes to SeaMonkey->Themes.
> (In reply to comment #5)
>> Note: in case Bugzilla messes up patches containing UTF-8 characters "Simon
>> Bünzli" should be "Simon Bünzli"
> Which parts of his file are you using, and why?

I just copied the boilerplate from winstripe (he is also listed in pinstripe). In other bugs he's very picky about his name being mangled in the credits so I thought I'd mention it here.

>> But I have no idea what modern should use for these system colours.
> I see nothing wrong with your reuse of the listbox colours.

OK.

>> Initially I copied the following from listbox.css:
>>   background-color: #424F63;
>>   color: #FFFFFF;
>> But I found the effect unpleasant.
> Looks fine to me.

OK. I'll fix it in the next patch.

>> >+richlistbox[seltype="multiple"]:focus > richlistitem[current="true"] {
>> >+  outline: 1px dotted #424F63;
>> >+  -moz-outline-offset: -1px;
>> >+}
>> I just made a wild guess Highlight ==> #424F63 here.
> Why not do what listbox.css does instead?

modern listbox.css doesn't seem to have a rule for |listbox:focus > listitem[current="true"]|
In other places Highlight in winstripe->listbox.css seems to map to #424F63 in modern->listbox.css
(In reply to comment #6)
> (In reply to comment 4)
> > 1) Modern is in mozilla/suite --> MAS
> > 2) There is no component matching "themes" --> General
> As I understand it Core->Themes is for MAS (and now SeaMonkey) themes.

One could see it in this way:
Core: Themes used to be for MAS (MAS doesn't exist as a "real" product on trunk/1.8). Then the files where moved to suite/, thus belonging to the application "SeaMonkey" which atm is MAS in bugzilla.

Anyway, regardless of this, I'd still file bugs for SeaMonkey themes in the MAS product, because in there you can set the appropiate flags.

Stefan: The SeaMonkey themes component in Bugzilla is actually Core::Themes, see bug 334964 and http://www.gerv.net/temp/bmo-reorg.html - Bugzilla reorg ftw!
Assignee: philip.chee → nobody
Component: General → Themes
Product: Mozilla Application Suite → Core
>> Why not do what listbox.css does instead?

> modern listbox.css doesn't seem to have a rule for |listbox:focus >
> listitem[current="true"]|
> In other places Highlight in winstripe->listbox.css seems to map to #424F63 in
> modern->listbox.css

It just occurred to me that you meant to use:
  border-top-color: #000000;
  border-bottom-color: #000000;
Instead of a dotted outline.
Assignee: nobody → philip.chee
(In reply to comment #7)
>> (In reply to comment #5)
>>> Note: in case Bugzilla messes up patches containing UTF-8 characters "Simon
>>> Bünzli" should be "Simon Bünzli"
>> Which parts of his file are you using, and why?
> I just copied the boilerplate from winstripe (he is also listed in pinstripe).
Ah, I was just thinking you might be better of basing it on listbox.css ...

> modern listbox.css doesn't seem to have a rule for listbox:focus >
> listitem[current="true"]
Line 70.
(In reply to comment #9)
> Stefan: The SeaMonkey themes component in Bugzilla is actually Core::Themes,
> see bug 334964 and http://www.gerv.net/temp/bmo-reorg.html - Bugzilla reorg
> ftw!

That doesn't imply that the SeaMonkey themes component is in Core:Themes atm.
The way I see it is that we don't have a component for themes. I can only see two reasons to file a SeaMonkey themes bug in Core:Themes: Bugs will automatically end up in the right place
when the "big move" is done and it's easier to track them. But when I file a
bug, I'm more concerned about the "Product" so I can set the right flags if I
want. 

If the "file a bug where you would have filed it when you used MAS 1.7x" logic
should be followed, help documentation bugs should be filed in "Documentation:
Help Viewer" and toolkit Help Viewer bugs should be filed in "MAS: Help" and
that would be absurd :-)
(In reply to comment #9)
> But when I file a bug

Stefan, in this case you didn't file the bug *I* did. Besides KaiRo was speaking Ex-Cathedra so your point is moot.
This is a screenshot of the permissions richlistbox in multiselect mode. In richlistbox I have:

richlistbox[seltype="multiple"]:focus > richlistitem[current="true"] {
  border-top: 2px solid;
  border-bottom: 2px solid;
  border-top-color: red;
  border-bottom-color: red;
}

richlistbox[seltype="multiple"]:focus > richlistitem[current="true"][selected="true"] {
  border-top: 2px solid;
  border-bottom: 2px solid;
  border-top-color: green;
  border-bottom-color: green;
}

pageInfo.css has:

#permList > richlistitem {
......
  border-bottom: 1px dotted #C7D0D9;
}

On the left the second row is current and selected. On the right the second row is current but not selected. As you can see the pageInfo.css rule: |border-bottom: 1px dotted #C7D0D9;| takes precedence over the richlistbox.css rules. I suspect that consumers of richlistbox will have a tendency to style their richlistitems with either top or bottom borders (e.g. PageInfo->Feeds, Toolkit->Addons Manager, Firefox->Download Manager, and Firefox->AwesomeBar)
I don't know how much difference it makes for the others but certainly for page info we have control over its CSS so we can adjust it if necessary.
> for page
> info we have control over its CSS so we can adjust it if necessary

What about all those (hypothetical) extensions out there that use richlistboxes and expect the toolkit *stripe UI?

If I use outline-top/botton: 1px solid #000000; I could fake top/bottom borders without clashing with richlistboxes that use top/bottom borders to style their rows. What do you think?
Attaching two alternate patches for this bug. This one uses outlines.
Attachment #299158 - Attachment is obsolete: true
Attachment #300593 - Flags: review?(neil)
This one uses borders.
Attachment #300594 - Flags: review?(neil)
Comment on attachment 300594 [details] [diff] [review]
richlistbox.css-border.diff: Uses borders for multiselect modes.

>   skin/modern/global/wizard.css                                    (global/wizard.css)
>   skin/modern/global/autocomplete.css                              (global/autocomplete.css)
>   skin/modern/global/global.css                                    (global/global.css)
>   skin/modern/global/popup.css                                     (global/popup.css)
>+  skin/modern/global/richlistbox.css                               (global/richlistbox.css)
>   skin/modern/global/scale.css                                     (global/scale.css)
>   skin/modern/global/splitter.css                                  (global/splitter.css)
>   skin/modern/global/tabbox.css                                    (global/tabbox.css)
>   skin/modern/global/textbox.css                                   (global/textbox.css)
>   skin/modern/global/toolbar.css                                   (global/toolbar.css)
>   skin/modern/global/dialog.css                                    (global/dialog.css)
I was wondering why you didn't put richlistbox between radio and scrollbox but that was before I noticed that this file isn't that well alphabetised... 

>+ * The Original Code is Richlistbox CSS.
Rather than formatting the file in the style of winstripe, can you please format the file in the style of modern; use listbox.css for an example.

>+richlistbox[seltype="multiple"]:focus > richlistitem[current="true"][selected="true"] {
>+  border-top-color: #FFFFFF;
>+  border-bottom-color: #FFFFFF;
>+}
Do you think we should add similar rules to listbox.css and tree.css?
> I was wondering why you didn't put richlistbox between radio and scrollbox but
> that was before I noticed that this file isn't that well alphabetised... 

Alphabetized the global section.

> Rather than formatting the file in the style of winstripe, can you please
> format the file in the style of modern; use listbox.css for an example.

Done.
Attachment #300593 - Attachment is obsolete: true
Attachment #300594 - Attachment is obsolete: true
Attachment #301011 - Flags: superreview?(neil)
Attachment #301011 - Flags: review?(neil)
Attachment #300593 - Flags: review?(neil)
Attachment #300594 - Flags: review?(neil)
Comment on attachment 301011 [details] [diff] [review]
richlistbox.css boilerplate nits fixed, jar.mn alphabetized.

I moved the richlistitem rule before checkin because I thought it was best to have it first, and I also added a couple of comments in the style of listbox.css
Attachment #301011 - Flags: superreview?(neil)
Attachment #301011 - Flags: superreview+
Attachment #301011 - Flags: review?(neil)
Attachment #301011 - Flags: review+
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: