559978 - Top align icons in places lists (bookmarks, awesomebar)

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Bookmarks, defect)

Description

[Matt Brubeck (:mbrubeck)]

Attachment: patch

The icons in our bookmark and autocomplete lists are vertically off-center.  This patch centers the icon within the list item.

(It also causes the tags and star to move down slightly.  I can move those back up if desired, but first I want to see if this change is wanted at all.)

Comment 1

[Matt Brubeck (:mbrubeck)]

Attachment: Screenshot: Before

Comment 2

[Matt Brubeck (:mbrubeck)]

Attachment: Screenshot: After

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

I'm a little hesitant to add the extra vbox. We did something like this in the past and moved away from the extra XUL to make XBL creation as streamlined as possible.

Also, can we be more explicit in the CSS? It's suppose to be faster:

.autocomplete-item-label label   -->   .autocomplete-item-label > vbox > label


The new look is nice though. Do we want to change?

Comment 4

[Matt Brubeck (:mbrubeck)]

Attachment: patch v2: minor CSS changes

Updated to use more precise selectors e.g. ".autocomplete-item-label > vbox > label".

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Actually the display from the results of the awesomebar and the display of the bookmarks are synced.

Madhava, if we choose to apply this do we want to break this behavior or apply also this style to the awesome bar?

Comment 6

[Matt Brubeck (:mbrubeck)]

Attachment: patch v3: align to top

At Madhava's request, changing this to align icons to the top of the title text.  I hoped I could get rid of the new vbox for this version, but alas the icon is too tall to sit inline with the title.

Comment 7

[Matt Brubeck (:mbrubeck)]

Attachment: Screenshot: After (updated)

Comment 8

[Matt Brubeck (:mbrubeck)]

Attachment: Screenshot: After - bookmarks

Comment 9

[Matt Brubeck (:mbrubeck)]

Attachment: patch v4: align to top

Minor cleanup: Remove a superfluous line-height (from my debugging), and push the stars back down a couple of pixels.

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 439954 [details] [diff] [review]
patch v4: align to top

We are adding a vbox to the XBL structure which could affect display performance.

Comment 11

[Matt Brubeck (:mbrubeck)]

Attachment: Screenshot: After (updated)

Fix those stars.

Comment 12

[Mark Finkle (:mfinkle) (use needinfo?)]

Vivien mentioned that we could try using some CSS to position the image without needing the extra <vbox>

Comment 13

[Matt Brubeck (:mbrubeck)]

Attachment: patch v5

Fix a bizarre height issue with the "allbookmarks" icon by adding a max-height  (visible as a difference between the Before and After screenshots).  Why is this necessary?  I'm not quite sure, but we have the same thing in the normal bookmark icon CSS from bug 523708.

Comment 14

[Matt Brubeck (:mbrubeck)]

Attachment: patch v6

Better fix for icon stretching, thanks to Mark Finkle.

Comment 15

[Mark Finkle (:mfinkle) (use needinfo?)]

pushed:
http://hg.mozilla.org/mobile-browser/rev/7a4d3865b316

Comment 16

[Aakash Desai [:aakashd]]

verified FIXED on builds: 

Mozilla/5.0 (X11; U; Linux armv7l; Nokia N900; en-US; rv:1.9.2.5pre) Gecko/20100420 Namoroka/3.6.5pre Fennec/1.1a2pre

and

Mozilla/5.0 (X11; U; Linux armv6l; en-US; rv:1.9.3a5pre) Gecko/20100420 Namoroka/3.7a5pre Fennec/1.1a2pre