Create a theme for the nsPromptService dialogs

VERIFIED FIXED in fennec1.0b5

Status

VERIFIED FIXED
10 years ago
5 years ago

People

(Reporter: mfinkle, Assigned: fabrice.desre)

Tracking

Trunk
fennec1.0b5
x86
Linux

Details

(Whiteboard: [polish])

Attachments

(4 attachments, 2 obsolete attachments)

Current theme for dialogs introduced in bug 489423 use a dark panel style.
tracking-fennec: --- → ?

Comment 1

10 years ago
what dialogs does this primarily effect?
Any window.alert, window.prompt and window.confirm (from web content) - as well as several dialogs displayed by platform code (confirm add-on install and auth prompt for example)

Updated

10 years ago
tracking-fennec: ? → 1.0+
Created attachment 401471 [details]
style revisions, for readability

Just using more spacing between the semantic blocks makes the dialogs more understandable/scannable.  I've dropped the font size for some of the sections to make the space (18point is the smallest).
Oh, I also made the upper corners rounded (they were squared before because the original designs had these hanging off of the titlebar) and moved the toggle from the left to the right -- this makes those buttons seem less primary on the dialog, which is appropriate (it's a secondary decision).
tracking-fennec: 1.0+ → ?
Whiteboard: [polish]
(Assignee)

Comment 5

10 years ago
Created attachment 405475 [details] [diff] [review]
fix

- made all corners rounded
- added a "prompt-message" class, with smaller text and top/bottom margins
- switched the positions of the message and yes/no radio control
(Assignee)

Comment 6

10 years ago
Created attachment 405477 [details]
screenshot
(Assignee)

Updated

10 years ago
Attachment #405475 - Flags: ui-review?(madhava)
Attachment #405475 - Flags: review?(mark.finkle)
Comment on attachment 405475 [details] [diff] [review]
fix

This patch is pretty good and is almost ready to land. Just needs a few tweaks:
* Padding is off for some dialogs:
  * Some of the dialogs have the main message and the checkbox message together (like alert.xul) and this creates a "double-width" padding between them.
  * Some have a hidden checkbox message so the control and buttons have no padding between them (select.xul and promptPassword.xul)

Because of this, I think we need to remove the padding from the "prompt-message" class. Instead, let's make rules for the main message (dialog > scrollbox.prompt-message), the checkbox message (dialog > hbox.prompt-message) and the buttons hbox (dialog > hbox.prompt-buttons)

The main message will have top & bottom margins and the checkbox and buttons will only have a top margin. That will allow the show/hide of the checkbox message to not break padding.

Make sense?

Next, let's cleanup the font-size a little. Remove the font-size: 100%; from dialog selector in chrome/content/browser.css and add font-size rules to platform.css:

dialog {
  font-size: 2.4mm !important; //( 9pt for wince)
}

dialog .prompt-message {
  font-size: 1.8mm !important; //(8pt for wince)
}

Let's see what we have with these changes
Attachment #405475 - Flags: ui-review?(madhava)
Attachment #405475 - Flags: review?(mark.finkle)
Attachment #405475 - Flags: review-
(Assignee)

Comment 8

10 years ago
Created attachment 405988 [details] [diff] [review]
updated patch

Implemented changes from comment #7. This looks good to me !
Assignee: nobody → fabrice.desre
Attachment #405475 - Attachment is obsolete: true
Attachment #405477 - Attachment is obsolete: true
Attachment #405988 - Flags: review?(mark.finkle)
(Assignee)

Comment 9

10 years ago
Created attachment 405989 [details]
prompt screenshot
(Assignee)

Comment 10

10 years ago
Created attachment 405990 [details]
unresponsive script dialog
Attachment #405988 - Flags: review?(mark.finkle) → review+
Comment on attachment 405988 [details] [diff] [review]
updated patch

Yes, this is better. I'll fix the wince font-size when checking in. You left the "mm" sizes in there.
pushed:
https://hg.mozilla.org/mobile-browser/rev/331a2cea3888
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → B5
verified FIXED On builds: 

Mozilla/5.0 (Windows; U; WindowsCE 5.2; en-US; rv:1.9.2b1pre) Gecko/20091013 Fennec/1.0a4pre

and

Mozilla/5.0 (X11; U; Linux armv7l; en-US; rv:1.9.2b1pre) Gecko/20091013
Fennec/1.0b5pre

and

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a1pre) Gecko/20091013
Fennec/1.0b5pre
Status: RESOLVED → VERIFIED
crap, this is not resolved just yet. The changes will be affected in the nightly build on 10/14
Status: VERIFIED → RESOLVED
Last Resolved: 10 years ago10 years ago
verified FIXED on builds with one caveat:

Mozilla/5.0 (X11; U; Linux armv7l; en-US; rv:1.9.2b1pre) Gecko/20091014
Fennec/1.0b5pre
Status: RESOLVED → VERIFIED
tracking-fennec: ? → ---
You need to log in before you can comment on or make changes to this bug.