Closed Bug 1164959 Opened 8 years ago Closed 8 years ago
Remove empty private tabs tray content, replace with simple empty state
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.
8 years ago
Did you get a chance to have a look at this one?
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.
A load of cleanup in regards to layout, styles and strings around the removal of the private tabs tray empty state.
Attachment #8617319 - Flags: review?(michael.l.comella)
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?
(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.
reminder to self to change image name to private-masq before pushing
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?
Attachment #8617319 - Flags: review?(michael.l.comella) → review+
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.
Tested with: Device: Alcatel One Touch (Android 4.1.2) Build: Firefox for Android 41.0a1 (2015-06-16)
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.