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

RESOLVED FIXED

Status

RESOLVED FIXED
11 years ago
10 years ago

People

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

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

11 years ago
Created attachment 274588 [details] [diff] [review]
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.
(Assignee)

Updated

11 years ago
Assignee: nobody → philip.chee
(Assignee)

Comment 1

11 years ago
Created attachment 274667 [details] [diff] [review]
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
Attachment #274588 - Attachment is obsolete: true
Attachment #274667 - Flags: superreview?(neil)
Attachment #274667 - Flags: review?(neil)

Comment 2

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

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

Comment 4

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

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

Comment 6

11 years ago
Created attachment 274873 [details] [diff] [review]
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.
Attachment #274667 - Attachment is obsolete: true
Attachment #274667 - Flags: superreview?(neil)
Attachment #274667 - Flags: review?(neil)

Comment 7

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

Comment 8

11 years ago
Created attachment 274917 [details] [diff] [review]
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?
Attachment #274873 - Attachment is obsolete: true

Comment 9

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

I'll just bugspam myself...
Attachment #274917 - Flags: superreview+
Attachment #274917 - Flags: review+
(Assignee)

Updated

11 years ago
Keywords: checkin-needed

Comment 10

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

11 years ago
Fix checked in.
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED

Updated

11 years ago
Depends on: 412167
Product: Core → SeaMonkey
You need to log in before you can comment on or make changes to this bug.