Open Bug 515967 Opened 11 years ago Updated 10 years ago

Add icons to addressbook menulist in Search Addresses

Categories

(MailNews Core :: Address Book, defect, minor)

defect
Not set
minor

Tracking

(Not tracked)

ASSIGNED

People

(Reporter: iann_bugzilla, Assigned: iann_bugzilla)

Details

(Keywords: polish, Whiteboard: [patchlove][has draft patch])

Attachments

(1 file, 1 obsolete file)

At the moment the addressbook menulist does not give any indication what sort of addressbook each entry is.
This patch:
* Adds isRemote and isSecure attributes to menuitems
* Adds css to take advantage of above attributes
Attachment #400136 - Flags: ui-review?(stefanh)
Attachment #400136 - Flags: superreview?(neil)
Attachment #400136 - Flags: review?(bugzilla)
It's a nice idea, but there are actually 7 windows (6 in Thunderbird) with addressbook menulists... and they are inconsistent with their CSS files...
pref-addressing.xul [no addrbook CSS]
abMailListDialog.xul
	cardDialog.css [via abListOverlay.xul]
ABSearchDialog.xul
	searchDialog.css
	abResultsPane.css [via abResultsPaneOverlay.xul]
addressbook-panel.xul
	sidebarPanel.css
		addressbook.css
	addressPanes.css (again!)
	abResultsPane.css
	addressPanes.css [via abDirTreeOverlay.xul]
abSelectAddressesDialog.xul
	selectAddressesDialog.css
	addressPanes.css [via abDirTreeOverlay.xul]
	abResultsPane.css [via abResultsPaneOverlay.xul]
abNewCardDialog.xul [side note: includes global.css!]
	cardDialog.css [via abCardOverlay.xul]
am-addressingOverlay.xul [no addrbook CSS]
Attachment #400136 - Flags: superreview?(neil) → superreview-
Comment on attachment 400136 [details] [diff] [review]
Icons in search addresses' menulist patch v0.1

>+            menuitem.setAttribute("class", "menuitem-iconic");
Nit: you can use .className

>             menulist.insertItemAt(0, this.getAttribute("none"), "");
Would this item line up with the other items? (It doesn't appear on the search addresses dialog but I think one of the pref panels has it.)

>-      setSearchScope(GetScopeForDirectoryURI(selectedAB));
>+      SelectDirectory(selectedAB);
Eww, that's a bit of overkill just to avoid calling GetScopeForDirectory

>+    abPopup.setAttribute("isRemote", directory.isRemote);
>+    abPopup.setAttribute("isSecure", directory.isSecure);
You could consider copying the attributes from the menulist's selected item.

>+#abPopup,
>+menupopup.addrbooksPopup > menuitem.menuitem-iconic {
I would rather add a new class that you would apply to both the menulist and each menuitem that would style using the appropriate addressbook image.

>+menulist > menupopup > menuitem {
>+  -moz-margin-end: 2px;
> }
> 
>-menulist > menupopup > menuitem {
>-  -moz-padding-end: 2px;
Whatever the point of this was, changing it to a margin is wrong.
Attachment #400136 - Flags: ui-review?(stefanh) → ui-review-
Comment on attachment 400136 [details] [diff] [review]
Icons in search addresses' menulist patch v0.1

There are 2 problems here:

1) On mac, A selected menuitem will have a checkmark instead of the relevant icon, see bug 384340 for how to handle that.


2) Icons that will eventually appear in the menulist are of different sizes (addrbook.gif is 12x13...)
Attachment #400136 - Flags: review?(bugzilla)
(In reply to comment #3)
>(From update of attachment 400136 [details] [diff] [review])
>>+    abPopup.setAttribute("isRemote", directory.isRemote);
>>+    abPopup.setAttribute("isSecure", directory.isSecure);
>You could consider copying the attributes from the menulist's selected item.
In fact assuming it is possible it would be neater to do this in the XBL, because that would then avoid copying the code to every instance.
Bah, you can't encode the address book type in to the description because it always displays in the menuitems (it only displays in the menulist if it has type="description"). Now, if we could only get the style rule changed to menulist[type="description"] > menupopoup > menuitem[description] ...
One of the Modern icons was also the incorrect size.
Severity: normal → minor
Whiteboard: [patchlove][has draft patch]
Comment on attachment 400136 [details] [diff] [review]
Icons in search addresses' menulist patch v0.1

Setting as obsolete, due to sr- and ui-review-..
Attachment #400136 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.