Closed
Bug 348614
Opened 19 years ago
Closed 19 years ago
Add generic arrow/dropmarker buttons
Categories
(Toolkit :: UI 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•19 years ago
|
Assignee: enndeakin → nobody
Assignee | ||
Comment 2•19 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•19 years ago
|
Attachment #234417 -
Flags: second-review?(neil) → second-review?(bugs.mano)
Assignee | ||
Comment 3•19 years ago
|
||
Attachment #234422 -
Flags: first-review?(roc)
Comment 4•19 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•19 years ago
|
Attachment #234417 -
Flags: second-review?(bugs.mano)
Assignee | ||
Comment 5•19 years ago
|
||
Attachment #234417 -
Attachment is obsolete: true
Attachment #234931 -
Flags: second-review?(bugs.mano)
Attachment #234931 -
Flags: first-review?(neil)
Comment 6•19 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•19 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•19 years ago
|
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•