992308 - Tab thumbnails in tab-sharing tab picker should be larger and have the right aspect ratio

Status: RESOLVED WONTFIX

(Firefox for Android Graveyard :: General, defect, P5)

Comment 1

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: icon_size.patch

Comment 2

[Brad Lassey [:blassey] (use needinfo?)]

Attachment: Screenshot_2014-04-04-15-01-01.png

Comment 3

[Wesley Johnston (:wesj)]

Comment on attachment 8401996 [details] [diff] [review]
icon_size.patch

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

We also need to text-align to the top of the row. I think the right way to do this will be to create (another) layout that we can use here. Then let you pass up a row type, either attached to the PromptListItem or to the PromptListAdapter (i.e. either every row in the list must match or we let them be different). Unfortunately, we use some native android layouts here, and most of them are checkedTextViews with compound drawables in them. i.e. that rabbit hole keeps getting deeper and deeper....

Comment 4

[Brad Lassey [:blassey] (use needinfo?)]

(In reply to Wesley Johnston (:wesj) from comment #3)
> Comment on attachment 8401996 [details] [diff] [review]
> icon_size.patch
> 
> Review of attachment 8401996 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> We also need to text-align to the top of the row. I think the right way to
> do this will be to create (another) layout that we can use here. Then let
> you pass up a row type, either attached to the PromptListItem or to the
> PromptListAdapter (i.e. either every row in the list must match or we let
> them be different). Unfortunately, we use some native android layouts here,
> and most of them are checkedTextViews with compound drawables in them. i.e.
> that rabbit hole keeps getting deeper and deeper....

These things sound out of scope of this bug

Comment 5

[Wesley Johnston (:wesj)]

The way I want to fix this bug is by using a separate layout (that will also the dpi bugs this patch has), which will require those other things.

Comment 6

[Wesley Johnston (:wesj)]

argh. I missed the metrics.density piece in this patch :)

I still want to use a separate layout. In fact, I just want to use the layout from the tabs tray so that they stay consistent I think....

Comment 7

[Brad Lassey [:blassey] (use needinfo?)]

Do you want to take over this bug then?

Comment 8

[Wesley Johnston (:wesj)]

Yes. Thanks :)

Comment 9

[Ian Barlow (:ibarlow)]

Comment on attachment 8401999 [details]
Screenshot_2014-04-04-15-01-01.png

I'll hold off on feedback until Wes takes another pass here, sounds like he knows what needs refinement

Comment 10

[Wesley Johnston (:wesj)]

Attachment: WIP 1/2 - Move tab styles into real styles

Getting closer...

It bothers me how much style info we have in our layouts. I also renamed TabTextAppearance to just TabText since it isn't used as a TextAppearance (and includes more than just that). I also added a contentDescription to the tab close button since I was kinda shocked we don't already have one.

I need to figure out a better way to interact with tablets here, since we'll likely NOT want to share these styles with tabs there. That layout is reused even on landscape phones though, so I think we're best to just make it its own style.

Comment 11

[Wesley Johnston (:wesj)]

Attachment: WIP 2/2

And this implements viewTypes for the promptlistitems. They're per item for now.

Comment 12

[Wesley Johnston (:wesj)]

Attachment: WIP 3/2 - Bonus cleanup

This is stuff Eclipse found for me to cleanup along the way.

Comment 13

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

Ian - We assume you would want this fixed before shipping

Comment 14

[Ian Barlow (:ibarlow)]

_yes_

Comment 15

[Brad Lassey [:blassey] (use needinfo?)]

filter on [mass-p5]

Comment 16

[Wesley Johnston (:wesj)]

I can't remember what happened here. I think we used a different UI in the end? Sounds like this feature is on the way out a bit anyway. Can we close this?

Comment 17

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

(In reply to Wesley Johnston (:wesj) from comment #16)
> I can't remember what happened here. I think we used a different UI in the
> end? Sounds like this feature is on the way out a bit anyway. Can we close
> this?

I think we can close this bug. Tab Mirroring is being reviewed to determine if we can realistically continue to ship it as a feature.