348614 - Add generic arrow/dropmarker buttons

Status: RESOLVED FIXED

(Toolkit :: XUL Widgets, defect)

Description

[Neil Deakin]

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.

Comment 1

[alexander :surkov (:asurkov)]

Sorry :(

Comment 2

[Neil Deakin]

Attachment: Style default <dropmarker>

Not sure if this approach is a good idea. It might be more practical though to just add a new element.

Comment 3

[Neil Deakin]

Attachment: Mac Theme Changes

Comment 4

[[e-mail]]

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)

Comment 5

[Neil Deakin]

Attachment: Fix review comments

Comment 6

[[e-mail]]

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 :-[

Comment 7

[Asaf Romano (gone)]

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.