973039 - Display message in HomePanelPicker when no Home panels are available for install

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: Awesomescreen, defect)

Description

[Sola Ogunsakin [:sola]]

When a user tries to add a panel and there are no panels available. Show an informative view on how to add a panel add-on.

Comment 1

[Chenxia Liu [:liuche]]

Attachment: WIP: Display "No panels"

Comment 2

[Chenxia Liu [:liuche]]

Attachment: Screenshot: placeholder "No panels"

We need something to put here, when the user has no new panels to install (because they have no new add-ons that support panels).

Comment 3

[Ian Barlow (:ibarlow)]

I could see us doing this in two stages. 

1. Just do a message that says "we couldn't find anything"

2. When we have more add-ons for about:home on AMO, we could provide a link out to let users do a search on their own. We probably should wait until such add-ons are available online though. 

So for now, I would suggest the text read something like:

------------------------
Add new panel
------------------------
Sorry, we couldn't 
find any panels for 
you to add.
------------------------

Comment 4

[Chenxia Liu [:liuche]]

Attachment: WIP: Display "No panels"

Rebase on top of new "Add panels" patches.

Comment 5

[Chenxia Liu [:liuche]]

Attachment: Patch: Display message when no panels available

Comment 6

[Chenxia Liu [:liuche]]

Attachment: Screenshot: Medium text

Comment 7

[Chenxia Liu [:liuche]]

Attachment: Screenshot: Small text

Ian, I added a couple of screenshots, let me know what text size or margin changes you want.

Comment 8

[Ian Barlow (:ibarlow)]

Chenxia this looks good. For the "sorry" text I would just use the same margins on the top and bottom as you have on the sides, like so: http://cl.ly/image/1L3K3F3q1u40

Comment 9

[:Margaret Leibovic]

Comment on attachment 8385579 [details] [diff] [review]
Patch: Display message when no panels available

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

Looks good, just a few questions.

::: mobile/android/base/home/HomePanelPicker.java
@@ +125,5 @@
>  
> +        if (availablePanels.isEmpty()) {
> +            setContentView(R.layout.home_panel_picker_empty);
> +            return;
> +        }

Trying to familiarize myself with HomePanelPicker a bit... am I correct that only one set of panelInfos ever gets loaded per fragment lifetime? I'm just trying to verify that it isn't possible to hit this code path once where there are no available panels, followed by one when there are.

::: mobile/android/base/resources/layout/home_panel_picker_empty.xml
@@ +15,5 @@
> +
> +     <TextView android:layout_width="match_parent"
> +               android:layout_height="match_parent"
> +               android:textAppearance="?android:attr/textAppearanceSmall"
> +               android:text="@string/home_add_panel_empty" />

Any reason why we can't just make this layout the TextView itself, and give it the desired padding/margin?

Comment 10

[Chenxia Liu [:liuche]]

Attachment: Patch: Display message when no home panels are available for install.

Carrying over r+.

(In reply to :Margaret Leibovic from comment #9)
> Comment on attachment 8385579 [details] [diff] [review]
> Patch: Display message when no panels available
> 
> Review of attachment 8385579 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Looks good, just a few questions.
> 
> ::: mobile/android/base/home/HomePanelPicker.java
> @@ +125,5 @@
> >  
> > +        if (availablePanels.isEmpty()) {
> > +            setContentView(R.layout.home_panel_picker_empty);
> > +            return;
> > +        }
> 
> Trying to familiarize myself with HomePanelPicker a bit... am I correct that
> only one set of panelInfos ever gets loaded per fragment lifetime? I'm just
> trying to verify that it isn't possible to hit this code path once where
> there are no available panels, followed by one when there are.

Yep, the panels are only loaded once, when the (dialog) Fragment is created, and the fragment is destroyed when you close the dialog.
> 
> ::: mobile/android/base/resources/layout/home_panel_picker_empty.xml
> @@ +15,5 @@
> > +
> > +     <TextView android:layout_width="match_parent"
> > +               android:layout_height="match_parent"
> > +               android:textAppearance="?android:attr/textAppearanceSmall"
> > +               android:text="@string/home_add_panel_empty" />
> 
> Any reason why we can't just make this layout the TextView itself, and give
> it the desired padding/margin?

For some reason I thought you always needed a *Layout, but that's actually not true. Updated!

Comment 11

[Chenxia Liu [:liuche]]

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

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/fc86509d1e22