Closed Bug 348614 Opened 18 years ago Closed 18 years ago

Add generic arrow/dropmarker buttons

Categories

(Toolkit :: UI Widgets, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: enndeakin, Assigned: enndeakin)

References

Details

Attachments

(2 files, 1 obsolete file)

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.
Blocks: 92174
Assignee: enndeakin → nobody
Sorry :(
Assignee: nobody → enndeakin
Attached patch Style default <dropmarker> (obsolete) — Splinter Review
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)
Attachment #234417 - Flags: second-review?(neil) → second-review?(bugs.mano)
Attachment #234422 - Flags: first-review?(roc)
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-
Attachment #234417 - Flags: second-review?(bugs.mano)
Attachment #234417 - Attachment is obsolete: true
Attachment #234931 - Flags: second-review?(bugs.mano)
Attachment #234931 - Flags: first-review?(neil)
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 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+
Depends on: 350739
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.