1164959 - Remove empty private tabs tray content, replace with simple empty state.

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

If we do indeed consolidate the two empty states we currently have. I think this one in the "private tabs tray" could be a lot simpler. 

Designs to follow.

Comment 1

[Martyn Haigh (:mhaigh)]

Did you get a chance to have a look at this one?

Comment 2

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

Attachment: prev_pb_tray2.png

While I'd like to take this opportunity here to do something more whimsical here, I don't want to block on that. 

So in the mean time, let's reuse the icon from the empty state and place it in the same spot.

Comment 3

[Martyn Haigh (:mhaigh)]

Attachment: Remove empty private tabs tray content, replace with simple empty state

A load of cleanup in regards to layout, styles and strings around the removal of the private tabs tray empty state.

Comment 4

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

I'd like to point out that in the mock in comment 2, the mask in the panel selector and the empty state are different - expected?

Comment 5

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

(In reply to Michael Comella (:mcomella) from comment #4)
> I'd like to point out that in the mock in comment 2, the mask in the panel
> selector and the empty state are different - expected?

Keen eye! the one that serves as the navigational element was a placeholder. I must've forgotten to replace it with the new masq icon I made. Since we weren't dealing with that part of the UI for this bug, I'll just leave that here.

This bug is still just about the contents under that bar, the empty state. And maybe changing that highlight to our PB purple if easy.

Comment 6

[Martyn Haigh (:mhaigh)]

reminder to self to change image name to private-masq before pushing

Comment 7

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Comment on attachment 8617319 [details] [diff] [review]
Remove empty private tabs tray content, replace with simple empty state

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

Sorry for the delay!

Those styles I wrote were pretty garbage - I'm glad we have the opportunity to get rid of them. :)

Make sure the new assets are trimaged.

::: mobile/android/base/resources/layout/private_tabs_panel.xml
@@ +7,5 @@
>         xmlns:gecko="http://schemas.android.com/apk/res-auto">
>  
> +    <FrameLayout android:id="@+id/private_tabs_empty"
> +                 android:layout_width="fill_parent"
> +                 android:layout_height="fill_parent">

nit: -> match_parent

@@ +13,5 @@
> +        <ImageView android:layout_height="wrap_content"
> +                   android:layout_width="wrap_content"
> +                   android:src="@drawable/private_mask"
> +                   android:layout_gravity="center"
> +                   android:layout_marginTop="-10dp"/>

Negative margins are not recommended by lint:

Margin values should be positive. Negative values are generally a sign that you are making assumptions about views surrounding the current one, or may be tempted to turn off child clipping to allow a view to escape its parent. Turning off child clipping to do this not only leads to poor graphical performance, it also results in wrong touch event handling since touch events are based strictly on a chain of parent-rect hit tests. Finally, making assumptions about the size of strings can lead to localization problems. 

---

While I don't think we fall into this category, I'd rather not mute lint warnings for correctness. Could this be marginBottom=10dp instead?

Comment 8

[Martyn Haigh (:mhaigh)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ebef56aba3c5

Comment 9

[Martyn Haigh (:mhaigh)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5c2e2baa75d5

Comment 10

[Martyn Haigh (:mhaigh)]

https://hg.mozilla.org/integration/fx-team/rev/6d8206038e6c

Comment 11

[Martyn Haigh (:mhaigh)]

I ended up leaving the -10 margin in there as it was very convoluted to achieve the same result any other way.  I've added a TODO as this hack won't been needed once we land Bug 1161638 and get rid of the browser chrome.

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/6d8206038e6c

Comment 13

[Teodora Vermesan (:TeoVermesan)]

Attachment: Screenshot_2015-06-17-10-41-05.png

Tested with:
Device: Alcatel One Touch (Android 4.1.2)
Build: Firefox for Android 41.0a1 (2015-06-16)