1278489 - consolidate styling of indent/icons/radio bullets on menupopup items for Thunderbird in css classes

Status: RESOLVED FIXED

(Thunderbird :: Theme, enhancement)

Description

[:aceman]

After bug 1278127 it appeared to me if we could somehow consolidate styling of indent/icons/radio bullets on menupopup items in Thunderbird via some classes, instead of styling each menupopup individually by id.

The conditions are:
- each menupopup may have different needs per platform as on some platform it may have icons or radio bullets and on another it may want to hide them.
- Also, on one platform, one menupopup may want to hide radio bullets, another one wants them shown (e.g. when in <menu>)

Continuing from bug 1260697, my suggestion is to create 3 classes, e.g.:
noIconPopupWin, noIconPopupLinux, noIconPopupOSX that would be applied to each menupop as needed.

E.g.:
- menupopup with id=msgIdentityPopup would have all classes set.
- menupopup with id=FontFacePopup would have class="noIconPopupWin noIconPopupLinux" set (OS X wants to show radio bullet or checkmark).
- menupopup with id=viewPickerPopup would have class=noIconPopupWin set (not sure about OS X), as Linux shows the radio bullets

Each mail/themes/<platform>/messenger.css would define only the class for that <platform>.

And the class would have merged all the styling for what is needed to hide icons (.menu-iconic-left)/bullets (type="radio")/indents/make menuseparators full width, etc.

Then, individual styling could go away, like:
#viewPickerPopup > menuitem[type="radio"] {
 -moz-appearance: none;
}
#FontFacePopup > menuitem > .menu-iconic-left {
  display: none;
}

I am not sure if we also need inverse classes, to allocate space for bullets/icons, as there are also rules in that direction:
http://mxr.mozilla.org/comm-central/source/mail/themes/linux/mail/messenger.css#65
http://mxr.mozilla.org/comm-central/source/mail/themes/linux/mail/cloudfile/addAccountDialog.css#88

This is an experiment, I don't know if this saves us some code, or makes things complicated.

But the idea is to have the style merged on one class (per platform) that can be used on any menupopup as needed. Not for each new menupopup having to sprinkle its id into various theme files.

Comment 1

[Richard Marti (:Paenglab)]

Attachment: noIconPopup.patch

This patch uses now noIconPopupWin and noIconPopupLinux as classes. I saw no need for OS X to add a class for it.

Comment 2

[:aceman]

On OS X there are no menupopups without icons?

Where is the Windows equivalent of '.menu-iconic-left { display: none}'? Linux theme has it.

Why does e.g. ParagraphPopup not need the noIconPopupWin class? But FontFacePopup on the same formatting toolbar does.
Also why is it not needed for the menupopup under <menulist id=defaultFont> (in options->display->formatting) and the other menupopups in the Options?

Comment 3

[Richard Marti (:Paenglab)]

(In reply to :aceman from comment #2)
> On OS X there are no menupopups without icons?

OX X has either icons or a checkmark.

> Where is the Windows equivalent of '.menu-iconic-left { display: none}'?
> Linux theme has it.

https://dxr.mozilla.org/comm-central/source/mozilla/toolkit/themes/windows/global/menu.css#215

Windows never shows checkmarks or radios in menulist popups. So the rule to show .menu-iconic-left can be done simply with
https://dxr.mozilla.org/comm-central/source/mail/themes/windows/mail/messenger.css#31
but on Linux also radios can be shown and the rule needs to be wider. But with menupopup:not(.noIconPopupLinux) we can steer it now.

> Why does e.g. ParagraphPopup not need the noIconPopupWin class? But
> FontFacePopup on the same formatting toolbar does.

The ParagraphPopup has no menuseparator we need the adjustment on Windows.

> Also why is it not needed for the menupopup under <menulist id=defaultFont>
> (in options->display->formatting) and the other menupopups in the Options?

The messenger.css rule doesn't apply on Preferences window.

Comment 4

[:aceman]

(In reply to Richard Marti (:Paenglab) from comment #3)
> Windows never shows checkmarks or radios in menulist popups. So the rule to
> show .menu-iconic-left can be done simply with
> https://dxr.mozilla.org/comm-central/source/mail/themes/windows/mail/
> messenger.css#31

Ah, then why isn't the default style to have a full width menuseparator when the default is to have no icons?

Doesn't it mean there should be a iconsPopupWin what would allocate the space for icons/radios and shift the menuseparator (inverse of noIconPopupWin)?

> > Why does e.g. ParagraphPopup not need the noIconPopupWin class? But
> > FontFacePopup on the same formatting toolbar does.
> 
> The ParagraphPopup has no menuseparator we need the adjustment on Windows.

NOW it doesn't have one. But the intention of the class is to be generic and be applied to all menulists that are intended to have no icons (regardless of menuseparators). So I would add it here. But you say the default on Win is "no icons" so we first need to solve the previous question of mine.

(In reply to :aceman from comment #0)
> Then, individual styling could go away, like:
> #viewPickerPopup > menuitem[type="radio"] {
>  -moz-appearance: none;
> }

OK, you removed this one in bug 1282280.

Comment 5

[:aceman]

(In reply to :aceman from comment #4)
> > > Why does e.g. ParagraphPopup not need the noIconPopupWin class? But
> > > FontFacePopup on the same formatting toolbar does.
> > 
> > The ParagraphPopup has no menuseparator we need the adjustment on Windows.

On Linux this menupopup has indented items, but no checkmarks/radios. So maybe it needs noIconPopupLinux .

Comment 6

[Richard Marti (:Paenglab)]

Attachment: noIconPopup.patch v2

This is a completely different approach without setting classes and only with CSS.

On Linux only the items of menupopup[type="folder"] with their .menuitem-iconic and the items of [type="radio"] show the space for the radios and icons. I've found no menulist with checkmarks. Thus the change in the comment.

On Windows only the items of menupopup[type="folder"] with their .menuitem-iconic show the space for the icons.

This patch makes the one from bug 1280429 obsolete.

Comment 7

[:aceman]

Comment on attachment 8766880 [details] [diff] [review]
noIconPopup.patch v2

Review of attachment 8766880 [details] [diff] [review]:
-----------------------------------------------------------------

So in this version you assume Windows defaults to no icon (at least outside of <menu>) and make it have a full width separator. Only exception is type="folder". Hopefully if somebody adds a new menulist with radios or checkmarks on Windows, you notice it and make him add an additional selector to this rule, not write some new css to override your separator css :)

On Linux, you leave space automatically if there is any item with type="radio". It is possible to also add the same rule for type="checkbox" to future-proof it?
This also fixes the surplus indent in the "All of"/"Any of" menulist in the Quick filter bar.

I think I like this now.

Comment 8

[Richard Marti (:Paenglab)]

(In reply to :aceman from comment #7)
> Comment on attachment 8766880 [details] [diff] [review]
> noIconPopup.patch v2
> 
> Review of attachment 8766880 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> So in this version you assume Windows defaults to no icon (at least outside
> of <menu>) and make it have a full width separator. Only exception is
> type="folder". Hopefully if somebody adds a new menulist with radios or
> checkmarks on Windows, you notice it and make him add an additional selector
> to this rule, not write some new css to override your separator css :)

Windows has no rules in toolkit to show radio or checkmarks. The folderpane-mode-selector-menulist has radios and Windows doesn't show any. It hides the .menu-iconic-left. So, if one want to show them, he needs custom code.

> On Linux, you leave space automatically if there is any item with
> type="radio". It is possible to also add the same rule for type="checkbox"
> to future-proof it?

I added the type="checkbox" rules. But Linux doesn't have all rules to show checkbox icons.

> I think I like this now.

Thank you, aceman

Comment 9

[Richard Marti (:Paenglab)]

https://hg.mozilla.org/comm-central/rev/e65ab8074a5b