Closed
Bug 426745
Opened 17 years ago
Closed 17 years ago
Increase padding for prefpane icons (options, page info, addon manager)
Categories
(Firefox :: Theme, defect)
Tracking
()
RESOLVED
FIXED
Firefox 3
People
(Reporter: faaborg, Assigned: beltzner)
References
Details
Attachments
(3 files, 1 obsolete file)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
mconnor
:
review+
mconnor
:
approval1.9+
|
Details | Diff | Splinter Review |
(deleted),
image/png
|
Details |
Right now the dialog boxes that contain 32x32 prefpane icons (options, page info, addons manager) do not have enough space above the icon. There is currently 6 pixels of visual space below the text, and 1 pixel above the icons. The whitespace below the text should match the white space above the icon, and every dialog box that uses this type of interface should have the same height for the prefpane area.
Flags: blocking-firefox3?
Comment 1•17 years ago
|
||
If you see carefully, it's actually 3px below the text, considering the bottom pixel of 'y'. I've DOM-inspect the prefpanes and found that the labels have bottom margin of 2px. Add it to the bottom padding of the 'pane' (radio[pane]), it totals up 3px.
So, this should be the fix, which adds extra 2px at the top of the 'pane':
radio[pane]{
padding: 3px 3px 1px !important;
}
This code only works for 'Options' window and to be put into 'browser/preferences.css'. For 'Add-ons', 'Page Info' window, it's this:
#viewGroup radio{
padding: 3px 3px 1px !important;
}
This code should be in 'mozapps/extensions/extensions.css'.
I don't understand why this is not standardized, for example, put all 'prefpane' codes under 'global/prefpane.css'.
Assignee | ||
Updated•17 years ago
|
Flags: blocking-firefox3? → blocking-firefox3+
Comment 2•17 years ago
|
||
Assignee: nobody → ehsan.akhgari
Status: NEW → ASSIGNED
Attachment #314131 -
Flags: ui-review?(beltzner)
Attachment #314131 -
Flags: review?(mano)
Updated•17 years ago
|
Attachment #314131 -
Attachment is patch: true
Attachment #314131 -
Attachment mime type: application/octet-stream → text/plain
Updated•17 years ago
|
Whiteboard: [has patch][needs review mano]
Assignee | ||
Updated•17 years ago
|
Attachment #314131 -
Flags: ui-review?(beltzner) → ui-review+
Assignee | ||
Comment 3•17 years ago
|
||
Comment on attachment 314131 [details] [diff] [review]
Patch (v1)
After applying the bug and playing with it a bit, I actually have a couple of easy nits:
> /* Global Styles */
> #BrowserPreferences radio[pane] {
> list-style-image: url("chrome://browser/skin/preferences/Options.png");
>+ padding: 3px 3px 1px;
Make this 5px 3px 1px - I know what Kai was going for, here, but I actually think we want the balance to be between the top of the icon and the edge of the white space and the text baseline and the bottom of the space, not the descender. This ends up looking more visually balanced.
> #viewGroup radio {
> -moz-appearance: none;
> margin: 0px 1px 0px 1px;
>- padding: 1px 3px 1px 3px;
>+ padding: 3px 3px 1px !important;
> min-width: 4.5em;
> list-style-image: url("chrome://mozapps/skin/extensions/viewButtons.png");
> }
Is the !important needed?
Attachment #314131 -
Flags: ui-review-
Attachment #314131 -
Flags: ui-review+
Attachment #314131 -
Flags: review?(mano)
Attachment #314131 -
Flags: review-
Assignee | ||
Comment 4•17 years ago
|
||
Will attach screenshots next to show what this does, before and after. I also noticed that the extensions.css has what looks to be a superfluous -moz-appearance: none, but decided to leave it be.
Assignee: ehsan.akhgari → beltzner
Attachment #314131 -
Attachment is obsolete: true
Attachment #317199 -
Flags: review?(mconnor)
Assignee | ||
Comment 5•17 years ago
|
||
Comment 6•17 years ago
|
||
Comment on attachment 317199 [details] [diff] [review]
5px 3px 1px, no !important
looks good, ship it
Attachment #317199 -
Flags: review?(mconnor)
Attachment #317199 -
Flags: review+
Attachment #317199 -
Flags: approval1.9+
Assignee | ||
Updated•17 years ago
|
Flags: wanted-firefox3+
Flags: blocking-firefox3-
Flags: blocking-firefox3+
Keywords: checkin-needed
Whiteboard: [has patch][needs review mano] → [has patch][has reviews]
Comment 7•17 years ago
|
||
Beltzner: did you test the Page Info dialog as well?
Comment 8•17 years ago
|
||
mozilla/toolkit/themes/winstripe/mozapps/extensions/extensions.css 1.57
mozilla/browser/themes/winstripe/browser/preferences/preferences.css 1.24
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [has patch][has reviews]
Target Milestone: --- → Firefox 3
You need to log in
before you can comment on or make changes to this bug.
Description
•