Closed
Bug 348614
Opened 17 years ago
Closed 17 years ago
Add generic arrow/dropmarker buttons
Categories
(Toolkit :: XUL Widgets, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: enndeakin, Assigned: enndeakin)
References
Details
Attachments
(2 files, 1 obsolete file)
3.78 KB,
patch
|
roc
:
first-review+
|
Details | Diff | Splinter Review |
67.35 KB,
patch
|
neil
:
first-review+
asaf
:
second-review+
|
Details | Diff | Splinter Review |
For the date/time pickers, a native themed dropbutton is needed and/or some simple arrow buttons. It would be useful to have these available as stock buttons.
Updated•17 years ago
|
Assignee: enndeakin → nobody
Assignee | ||
Comment 2•17 years ago
|
||
Not sure if this approach is a good idea. It might be more practical though to just add a new element.
Attachment #234417 -
Flags: second-review?(neil)
Attachment #234417 -
Flags: first-review?(neil)
Assignee | ||
Updated•17 years ago
|
Attachment #234417 -
Flags: second-review?(neil) → second-review?(bugs.mano)
Assignee | ||
Comment 3•17 years ago
|
||
Attachment #234422 -
Flags: first-review?(roc)
Comment 4•17 years ago
|
||
Comment on attachment 234417 [details] [diff] [review] Style default <dropmarker> Gosh, I hadn't realised that our dropmarkers were in such a mess. This is just a inspection review, I haven't actually tried this out. > .toolbarbutton-menu-dropmarker { >- list-style-image: url("chrome://global/skin/arrow/arrow-dn.gif"); >+ -moz-appearance: none !important; >+ border: none !important; >+ background-color: transparent !important; > -moz-image-region: auto; /* cut off inheritance */ > } Please set -moz-image-region when you set list-style-image (except to none or to override another rule) so that you don't inherit a region by mistake. > .autocomplete-history-dropmarker { >- -moz-appearance: menulist-button; > min-width: 17px; I don't think we need to keep this width (multiple occurrences). > .button-menu-dropmarker, > .button-menubutton-dropmarker { >- margin: 1px; >- background-image: url("chrome://global/skin/arrow/arrow-dn.gif"); >+ -moz-appearance: none !important; >+ margin: 1px; >+ list-style-image: none !important; >+ background-image: url("chrome://global/skin/arrow/arrow-dn.gif"); >+ background-repeat: no-repeat; >+ background-position: center center; >+ background-color: transparent !important; >+ border: none !important; Does native themeing conflict with the use of a list-style-image? If so, your indent on the margin and background-image is wrong, and you could consider setting all the background properties at once i.e. background: url("chrome://global/skin/arrow/arrow-dn.gif") center no-repeat; >+ padding-top: 2px; >+ padding-bottom: 0px; >+ -moz-padding-start: 2px; >+ -moz-padding-end: 0px; XPFE isn't RTL-ised so you can get away with padding: 2px 0px 0px 2px; here. [I'm hoping for someone to implement -moz-border-padding-direction: rtl;] >+dropmarker[disabled="true"] { >+ list-style-image: url("chrome://global/skin/arrow/arrow-dn-dis.gif"); Don't you already inherit that? > .button-menu-dropmarker, > .button-menubutton-dropmarker { > margin: 1px; >+ list-style-image: none; >+ background-color: transparent; > background-image: url("chrome://global/skin/arrow/arrow-dn.gif"); >+ border: none; > min-width:9px; > min-height:5px; > } Does this element not work as a list-style-image either? >+ -moz-border-radius-topleft: 2px; >+ -moz-border-radius-bottomleft: 0; I think you meant to set them both to zero? (I'd also prefer 0px)
Attachment #234417 -
Flags: first-review?(neil) → first-review-
Updated•17 years ago
|
Attachment #234417 -
Flags: second-review?(bugs.mano)
Assignee | ||
Comment 5•17 years ago
|
||
Attachment #234417 -
Attachment is obsolete: true
Attachment #234931 -
Flags: second-review?(bugs.mano)
Attachment #234931 -
Flags: first-review?(neil)
Comment 6•17 years ago
|
||
Comment on attachment 234931 [details] [diff] [review] Fix review comments >+ padding-top: 2px 0 0 2px; [needs fixing twice] I'm sure I said "padding:" ;-) >+dropmarker[disabled="true"] Missing { (In reply to comment #4) >>+dropmarker[disabled="true"] { >>+ list-style-image: url("chrome://global/skin/arrow/arrow-dn-dis.gif"); >Don't you already inherit that? Oops, I must have been asleep :-[
Attachment #234931 -
Flags: first-review?(neil) → first-review+
Comment 7•17 years ago
|
||
Comment on attachment 234931 [details] [diff] [review] Fix review comments Pinstripe doesn't support RTL chrome either (Winstripe does), please simplify the style rules there too. r=mano with that fixed.
Attachment #234931 -
Flags: second-review?(bugs.mano) → second-review+
Attachment #234422 -
Flags: first-review?(roc) → first-review+
Assignee | ||
Updated•17 years ago
|
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•