390270 - [Modern theme only] SuiteRunner extension dialogs come up transparent or invisible and unusable

Status: RESOLVED FIXED

(SeaMonkey :: Themes, defect)

Description

[Philip Chee]

Attachment: Draft patch v1.0

How to reproduce:
1. Switch theme to Modern.
2. Install Infolister or DownThemAll or AutohideStatusbar or ...
3. Invoke this extensions options dialog via Tools->Addons->[Ext]->Options.

Actual Results:
A transparent unclickable box with some text.

Expected Results:
A working options dialog.

This is due to modern missing one file (global/preferences.css) and one style in global/global.css.

Proposed patch attached.

Comment 1

[Philip Chee]

Attachment: Patch v1.1

Fixed the following nits:

> a few issues with your patch; firstly, modern never uses appearance;
> secondly, modern never uses system colours; thirdly, modern isn't
> rtl-aware, and I don't want to have bits of it rtl-aware because it
> will only confuse people

Comment 2

[[e-mail]]

Comment on attachment 274667 [details] [diff] [review]
Patch v1.1

>+.prefWindow-dlgbuttons {
>+  padding-right: 5px;
>+  padding-bottom: 5px;
>+  padding-left: 5px;
>+}
I'm not sure padding is correct here; wizard doesn't use it.

>+radio[pane]:hover {
>+  background-color: #E0E8F6;
>+}
>+
>+radio[pane][selected="true"] {
>+  background-color: #C1D2EE;
>+}
>+
>+radio[pane][selected="true"]:hover {
>+  color: #FFFFFF;
>+}
Where did you get these colours?
Also, the Modern theme doesn't seem to go in for hover effects (except menus).
:hover:active and [selected="true"] effects might be sufficient.

Comment 3

[Robert Kaiser]

(In reply to comment #1)
> thirdly, modern isn't
> rtl-aware, and I don't want to have bits of it rtl-aware because it
> > will only confuse people

We may want to make Modern rtl-aware on trunk (hebrew currently installs its own RTL-aware Modern on localized branch builds), but I think that might be best a coordinated effort across the whole theme.

Comment 4

[Philip Chee]

(In reply to comment #2)
>>+.prefWindow-dlgbuttons {
>>+  padding-right: 5px;
>>+  padding-bottom: 5px;
>>+  padding-left: 5px;

> I'm not sure padding is correct here; wizard doesn't use it.

In classic/winstripe the dialog buttons have padding - probably to distinguish them from the dialog buttons in the child windows which have padding of 0px. I'll remove the padding if you don't think that the differentiation is needed.

>>+radio[pane]:hover {
>>+  background-color: #E0E8F6;
>>+}
>>+
>>+radio[pane][selected="true"] {
>>+  background-color: #C1D2EE;
>>+}
>>+
>>+radio[pane][selected="true"]:hover {
>>+  color: #FFFFFF;
>>+}
> Where did you get these colours?

The first two are unchanged from classic/winstripe.

> Also, the Modern theme doesn't seem to go in for hover effects (except menus).
> :hover:active and [selected="true"] effects might be sufficient.

The last I added using the tabbox as the closest analogy I could find. Although as you say it uses: tab[selected="true"]:hover:active  {}

I'll remove the last radio[pane][selected="true"]:hover {} but otherwise I'd like to follow the <prefwindow> paradigm as long as it's practicable to do so.

Comment 5

[[e-mail]]

(In reply to comment #4)
>In classic/winstripe the dialog buttons have padding - probably to distinguish
>them from the dialog buttons in the child windows which have padding of 0px.
Hmm... is there an example of this somewhere, so I can see the effect?

>>>+radio[pane]:hover {
>>>+  background-color: #E0E8F6;
>>>+}
>>>+
>>>+radio[pane][selected="true"] {
>>>+  background-color: #C1D2EE;
>>>+}
>>>+
>>>+radio[pane][selected="true"]:hover {
>>>+  color: #FFFFFF;
>>>+}
>>Where did you get these colours?
>The first two are unchanged from classic/winstripe.
I think at the very least the selected colour should be #C7D0D9 and the active (not just hover) colour should be #90A1B3 (text #FFFFFF)

The other thing I noticed was that Modern radio buttons have a 2px transparent border which turns #98A5B2 when the radiogroup has focus. I'd like to keep this focus effect, but I think this border radius is wrong:

>+radio[pane] {
>+  margin: 0px 1px 0px 1px;
>+  padding: 1px 3px 1px 3px;
>+  min-width: 4.5em;
>+  -moz-border-radius: 2px;

Instead I'd like the radio[focused="true"] border radius for all radios.

Comment 6

[Philip Chee]

Attachment: Patch v 1.2

> I think at the very least the selected colour should be #C7D0D9 and the active
> (not just hover) colour should be #90A1B3 (text #FFFFFF)
Done.

> Instead I'd like the radio[focused="true"] border radius for all radios.
Done

Also done: Removed hover effects except for :hover:active on the pane selector buttons.

> Hmm... is there an example of this somewhere, so I can see the effect?

Looking closer at the css I think this is what happens:
The main prefwindow has a padding of 0px so the dialog button box is given some padding to move it away from the border. The child prefwindows have some padding so the padding of the dialog button boxes in these prefwindows is removed. What is the Modern paradigm here? I suppose I could standardize on one style for consistency.

Comment 7

[[e-mail]]

Comment on attachment 274873 [details] [diff] [review]
Patch v 1.2

>+.prefWindow-dlgbuttons {
>+  padding-right: 5px;
>+  padding-bottom: 5px;
>+  padding-left: 5px;
>+}
Please change this to padding: 0px 5px 5px 5px;

>+radio[pane] {
>+  margin: 0px 1px 0px 1px;
>+  padding: 1px 3px 1px 3px;
>+  min-width: 4.5em;
>+  -moz-border-radius: 4px 5px;
>+  background-color: #FFFFFF;
>+  color: #000000;
>+}
Please move the -moz-border-radius into radio.css
(and remove the one in there for radio:focus)

r+sr=me with these fixed.

Comment 8

[Philip Chee]

Attachment: Patch v 1.3

(In reply to comment #7)
> r+sr=me with these fixed.

What's the proper protocol here, just carry forward the r+sr?

Comment 9

[[e-mail]]

Comment on attachment 274917 [details] [diff] [review]
Patch v 1.3

I'll just bugspam myself...

Comment 10

[[e-mail]]

(In reply to comment #9)
>I'll just bugspam myself...
Ah, it looks like they at least managed to fix that request spam issue.

Comment 11

[[e-mail]]

Fix checked in.