Ooh, good idea. There's a plan in place to show things in the Synced Tabs tray, and great progress in place: https://bugzilla.mozilla.org/show_bug.cgi?id=958889.

This bug isn't quite ready for action, but I'm interested in mentoring this. We'll need to get UX feedback to get a visual mockup to see what the description would look like; and we should wait for Bug 958889 to land, since that's going to provide a model for how to implement this "panel that sometimes looks very different".

Since this is actionable, I'd like UX to take a crack at a mockup and have someone start on it ASAP. Ian, Anthony: Can one of you take a crack.

(In reply to Nick Alexander :nalexander from comment #1) > Ooh, good idea. There's a plan in place to show things in the Synced Tabs > tray, and great progress in place: > https://bugzilla.mozilla.org/show_bug.cgi?id=958889. To Nick's point, I would definitely like to use a similar layout for the private tab (when it's empty). I'll mock something up for you guys but I want to ask what the "goal" of this page would be? so that I can write appropriate content.

At the very least, just a simple "Your private tabs show up here" message to provide a hint what this is. I wondered for a bit if we'd be better to put the data from about:privatebrowsing in here (and show Top Sites like normal in private mode instead of showing about:privatebrowsing?). i.e. it reads something like: ==== Private Browsing Firefox won't keep any browser history, search history, download history, web form history, cookies, or temporary internet files for tabs in here. However, files you download and bookmarks you make will be kept. While this computer won't have a record of your browsing history, your internet service provider or employer can still track the pages you visit. ==== it also has a "Learn more" link. That interaction likely won't work well for people who create private tabs other ways though and never see the empty tabs tray.

Desktop also has a nifty about:privatebrowsing page if you open in in a non-private window: ===== Would you like to start Private Browsing? You are not currently in a private window. In a Private Browsing window, Nightly won't keep any browser history, search history, download history, web form history, cookies, or temporary internet files. However, files you download and bookmarks you make will be kept. To start Private Browsing, you can also select New Private Window from the menu. =====

(In reply to Anthony Lam from comment #4) > To Nick's point, I would definitely like to use a similar layout for the > private tab (when it's empty). I'll mock something up for you guys but I > want to ask what the "goal" of this page would be? so that I can write > appropriate content. I think the main goal is to let people know what Private Tabs can do and how they block history. We have some text in about:privatebrowsing that might be useful.

Here is a mock for content! It would ideally just follow the same style template for text that we are using on the sync side. Note: The 'add a new tab' icon is different to what we have but it's not final at all, i just included it to see what it would look like. I also want to point out that we have this bug: https://bugzilla.mozilla.org/show_bug.cgi?id=864958

Note, in particular, my changes to TabsPanelItem.Button.

Same styles as sync. Removed the header because there wasn't much room.

Comment on attachment 8432901 [details] Phone portrait Looking good!

Comment on attachment 8432903 [details] Phone landscape This looks a bit off. The line-height of the copy on the left definitely needs to be increased. Could we also try vertically raising the "Want to learn more?" link to be more centered? I would also like to move the "active" tab a bit further down to open up these horizontal screens but I feel like that is going to open up a nest of other problems.

Comment on attachment 8432893 [details] [diff] [review] Part 1: Rename RemoteTabs* styles to be used with all tabs panels. Review of attachment 8432893 [details] [diff] [review]: ----------------------------------------------------------------- Nice.

Design questions: - Isn't this empty state a bit too verbose? Maybe the text should be more concise? - It would be nice if we replaced the longer description with a simpler empty state once the user has created and closed a few private tabs. Seeing this description all the time might get a bit annoying. Maybe in a follow-up?

(In reply to Lucas Rocha (:lucasr) from comment #15) > Design questions: > - Isn't this empty state a bit too verbose? Maybe the text should be more > concise? > - It would be nice if we replaced the longer description with a simpler > empty state once the user has created and closed a few private tabs. Seeing > this description all the time might get a bit annoying. Maybe in a follow-up? I agree - I would like to follow up these visual inconsistencies and establish a more solid design framework in a later bug.

Comment on attachment 8433696 [details] [diff] [review] Part 0: Add empty private browsing view description strings. Hey, Ryan. Anthony said you might know where to point the "Want to learn more" link in his mock from comment 8 - where should it link the user to? Clearing review until this link is updated.

WIP.

Comment on attachment 8433722 [details] [diff] [review] Part 2: Show a description page when there are no private tabs. These styles are getting stupidly messy because I'm trying to make them inherit where they perhaps should not be inheriting. Assuming antlam likes my tablet design, TabsPanelItem.TextAppeareance.Header.HiddenInLandscape.PrivateTabs is ambiguous - on phone, we hide the title in landscape, not in portrait. On tablet, it's the reverse! I don't know how to make this obvious without "...HiddenInLandscape.ReversedOnTablet", which seems really dumb. Alteratively, don't make it an inheritance (i.e. remove the "."). Similiar issues are abound with the 2ColCentered layout, and anything appended with "PrivateTabs". Do you have any suggestions?

Comment on attachment 8433722 [details] [diff] [review] Part 2: Show a description page when there are no private tabs. Review of attachment 8433722 [details] [diff] [review]: ----------------------------------------------------------------- Looks generally good but yes, try to avoid all these style inheritances as they are not easy to follow and likely to be broken by the next person changing them due all the dependencies.

Removed inheritance as much as I felt reasonable, as per comment 23.

(In reply to Anthony Lam (:antlam) from comment #13) > Comment on attachment 8432903 [details] > Phone landscape > > This looks a bit off. The line-height of the copy on the left definitely > needs to be increased. I didn't touch this because it may get stomped on with l10n and small screen sizes. I filed bug 1021356 for the visual refinements. > Could we also try vertically raising the "Want to > learn more?" link to be more centered? Done.

Comment on attachment 8435375 [details] Phone landscape v2 This is looking better

Comment on attachment 8435370 [details] [diff] [review] Part 2: Show a description page when there are no private tabs. Review of attachment 8435370 [details] [diff] [review]: ----------------------------------------------------------------- Looking good, just needs to change the structure of PrivateTabsPanel a bit. ::: mobile/android/base/resources/layout/private_tabs_panel.xml @@ +12,5 @@ > + android:visibility="gone"> > + > + <org.mozilla.gecko.tabspanel.TabsTray android:id="@+id/private_tabs_tray" > + style="@style/TabsList" > + android:layout_width="fill_parent" fill_parent -> match_parent for consistency? ::: mobile/android/base/tabspanel/PrivateTabsPanel.java @@ +28,5 @@ > + super(context, attrs); > + } > + > + @Override > + public void onFinishInflate() { This composite view looks good overall but I'd rather avoid relying on onFinishInflate() as I've seen device-specific bugs where the inflation callbacks are not very reliable (e.g. callbacks called more than once, not called at all, etc). So, let's tweak the structure of this view to be more like, say, TwoLinePageRow: 1. Inflate the inner layout in the constructor (change private_tabs_panel.xml to have a 'merge' toplevel tag) 2. Initialize view members in the constructor 3. Use view tag (org.mozilla.gecko.tabspanel.PrivateTabsPanel) directly in tabs_panel.xml instead of using an include

Comment on attachment 8433692 [details] Tablet portrait I think this is good for now.. as mentioned before, we're probably still going to have to take a look at all these trays at a later date.

Hey, Ryan. Anthony said you might know where to point the "Want to learn more" link in his mock from comment 8 - where should it link the user to?

Updated for lucas' comments.

Comment on attachment 8439607 [details] [diff] [review] Part 2: Show a description page when there are no private tabs. Review of attachment 8439607 [details] [diff] [review]: ----------------------------------------------------------------- Nice, thanks. ::: mobile/android/base/resources/layout-large-land-v11/tabs_panel.xml @@ -39,5 @@ > android:choiceMode="singleChoice" > android:visibility="gone" > gecko:tabs="tabs_normal"/> > > - <org.mozilla.gecko.tabspanel.TabsTray android:id="@+id/private_tabs" It seems you forgot to update this file.

Got the link from Ian via email.

Updated the patch to fill in the locale of the link.

Comment on attachment 8440178 [details] [diff] [review] Part 2: Show a description page when there are no private tabs. Review of attachment 8440178 [details] [diff] [review]: ----------------------------------------------------------------- Looks good with the fix in layout-large-land-v11/tabs_panel.xml ::: mobile/android/base/resources/layout-large-land-v11/tabs_panel.xml @@ +39,5 @@ > android:choiceMode="singleChoice" > android:visibility="gone" > gecko:tabs="tabs_normal"/> > > + <include layout="@layout/private_tabs_panel"/> This should be <org.mozilla.gecko.tabspanel.PrivateTabsPanel ...> directly here instead of the include.

Updated for nits.

remote: https://hg.mozilla.org/integration/fx-team/rev/c69d0ec8f5de remote: https://hg.mozilla.org/integration/fx-team/rev/e8993855b30f remote: https://hg.mozilla.org/integration/fx-team/rev/1a18c37601b7

https://hg.mozilla.org/mozilla-central/rev/c69d0ec8f5de https://hg.mozilla.org/mozilla-central/rev/e8993855b30f https://hg.mozilla.org/mozilla-central/rev/1a18c37601b7

>+<!ENTITY private_tabs_panel_description "Your private tabs will show up here. While we don\'t keep any form of your browsing history or cookies, files that you download will still be saved on your device."> Files that you download, and bookmarks you make will be kept. Also, I suggest dropping 'form' to not mix it up with 'form history'.

"Your private tabs will show up here. While we don\'t keep any of your browsing history or cookies, bookmarks and any files that you download will still be saved on your device." Anthony, what do you think of the adjustments I made to the issues brought up in comment 38?

(In reply to Michael Comella (:mcomella) from comment #39) > "Your private tabs will show up here. While we don\'t keep any of your > browsing history or cookies, bookmarks and any files that you download will > still be saved on your device." > > Anthony, what do you think of the adjustments I made to the issues brought > up in comment 38? "Your private tabs will show up here. While we don't keep any of your browsing history or cookies, bookmarks and files that you download will still be saved on your device." I removed the "any" because I don't really think that's necessary :)

Comment on attachment 8443651 [details] [diff] [review] Part 3: Update empty private tabs panel description. Review of attachment 8443651 [details] [diff] [review]: ----------------------------------------------------------------- Actually... :) See the above comments on the reasons for this change.

Oops!

Comment on attachment 8443688 [details] [diff] [review] Part 3: Update empty private tabs panel description. Review of attachment 8443688 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me -- text matches the text from antlam's previous comment

Nice catch, Aaron: https://hg.mozilla.org/integration/fx-team/rev/0f335eff972f