706667 - Make about:home pretty

Status: VERIFIED FIXED

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

Description

[Ian Barlow (:ibarlow)]

Attachment: UI Specs

Specs for the first version of the start page that includes Top Sites and Add-ons. Assets to follow in the next comment. 

NOTE: The logo header area is being reworked, so just focus on the actual content, and we can plug in an updated header when I have it ready.

Comment 1

[Ian Barlow (:ibarlow)]

Attachment: Graphic assets for start page

Includes list arrow and thumbnail shadow

Comment 2

[Ian Barlow (:ibarlow)]

Background colours

Light blue: d5e8f7

List Selection background colour: 8fc0e2

Dark blue form top sites: 4e77ae

Comment 3

[Naoki Hirata :nhirata (please use needinfo instead of cc)]

Attachment: screenshot

note: on the droid 2 w/ 20111201 build, the about:home does not fit in landscape.  see attachment screenshot.

Comment 4

[Lucas Rocha (:lucasr)]

Ian, we probably need specs for landscape orientation too? How is layout supposed to look in landscape?

Comment 5

[Ian Barlow (:ibarlow)]

Attachment: Mockups of start page

Some updates to the layout attached here.

Changes to note:

- New URL bar colour (this will be filed in a separate bug) 
- Background merges seamlessly into browser header. This will be a treatment we begin applying to all our in-content UI, like about:firefox, add-ons, about:config and so on (of course, those pages will appear in separate bugs)
- New branding treatment at top of page. Looks nicer, and also uses less space than before
- Tweaked styling of thumbnails
- Empty container design for thumbnails that don't have screenshots
- Some small colour and type style tweaks


Ill post more detailed specs soon and check in with Lucas in the morning.

Comment 6

[Lawrence Mandel [:lmandel] (use needinfo)]

This is a much nicer looking mockup of about:home. Three questions:

1. Is the empty container visible enough? Should we use a more visible FF logo?
2. My Samsung Galaxy S II came prepopulated with bookmarks. We don't have screenshots for any of them. How does the page look with 4 empty containers? Can we make the startup experience prettier in this situation?
3. If I have not set up sync (so I have no remote tabs) will I see more top sites to fill the space or will I have an empty area on my about:home?

Comment 7

[Ian Barlow (:ibarlow)]

Attachment: Graphic assets for start page

Comment 8

[Ian Barlow (:ibarlow)]

Attachment: UI Specs

Comment 9

[Lucas Rocha (:lucasr)]

Attachment: (1/4) Change indentation of about:home layout files for consistency

Cosmetic, no functional change.

Comment 10

[Lucas Rocha (:lucasr)]

Attachment: (2/4) Remove unused properties from AboutHomeContent

Comment 11

[Lucas Rocha (:lucasr)]

Attachment: (3/4) Organize/cleanup imports on AboutHomeContent

Comment 12

[Lucas Rocha (:lucasr)]

Attachment: (4/4) Implement AboutHomeContent layout as per design

Looks pretty close to design. A few known issues/missing bits:

- Rotating screen while about:home is visible sometimes causes thumbnails to disappear. Will investigate.
- Missing icon and version for addons list (we can do it in a follow-up patch).
- Layout looks a bit weird before the thumbnails are finally shown. Still not sure how to handle it design-wise.
- Not handling the case of having no addons or no top sites. Ian, I need design for those. We can do it in a follow-up patch.
- I didn't add the little arrow chars to the underlined links to see all addons and top sites. I think this might cause issues for translators.

I didn't notice any performance regression caused by the more complex about:home layout. Need to test in more devices.

Comment 13

[Lucas Rocha (:lucasr)]

Attachment: Screenshot (portrait)

Ignore the white thumbnails, this is an unrelated issue.

Comment 14

[Lucas Rocha (:lucasr)]

Attachment: Screenshot (landscape)

Ignore the white thumbnails, this is an unrelated issue.

Comment 15

[Lucas Rocha (:lucasr)]

Forgot to mention another follow-up patch: make about:home branding aware. For now, it hardcodes firefox branding (no images for aurora, nightly, or beta).

Comment 16

[Ian Barlow (:ibarlow)]

This is looking *great*, Lucas!

A couple of comments:

1. I notice the thumbnail frames don't have the "shadow" graphic applied, so we'll want to make sure those get added.
2. the "Browse all your top sites" text should have a "»" character next to it.
3. Can you nudge the firefox logo to the right, so it is partly cut off the screen, as in the mocks? Or, if it is easier I can make you a graphic that is already cut off. Let me know.

Can't wait to try this on a phone!

Comment 17

[Ian Barlow (:ibarlow)]

Attachment: Updated logo assets with cropped edge

Comment 18

[Lucas Rocha (:lucasr)]

(In reply to Ian Barlow (:ibarlow) from comment #16)
> This is looking *great*, Lucas!
> 
> A couple of comments:
> 
> 1. I notice the thumbnail frames don't have the "shadow" graphic applied, so
> we'll want to make sure those get added.
> 2. the "Browse all your top sites" text should have a "»" character next to
> it.
> 3. Can you nudge the firefox logo to the right, so it is partly cut off the
> screen, as in the mocks? Or, if it is easier I can make you a graphic that
> is already cut off. Let me know.

All done. Sending a new patch now.

Comment 19

[Lucas Rocha (:lucasr)]

Attachment: (4/4) Implement AboutHomeContent layout as per design

Comment 20

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

Comment on attachment 580926 [details] [diff] [review]
(4/4) Implement AboutHomeContent layout as per design

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

::: mobile/android/base/AboutHomeContent.java
@@ +77,5 @@
> +    private static final int NUMBER_OF_TOP_SITES_PORTRAIT = 4;
> +    private static final int NUMBER_OF_TOP_SITES_LANDSCAPE = 3;
> +
> +    private static final int NUMBER_OF_COLS_PORTRAIT = 2;
> +    private static final int NUMBER_OF_COLS_LANDSCAPE = 3;

defining the number of rows makes sense, but it would seem better to make the number of columns depend on the screen width. No?

@@ +124,5 @@
> +                    mUriLoadCallback.callback(spec);
> +            }
> +        });
> +
> +        mAddonsList = (ListView) findViewById(R.id.recommended_addons_list);

no click listener for the addons list?

@@ +289,5 @@
>  
> +    public class TopSitesCursorAdapter extends SimpleCursorAdapter {
> +        public TopSitesCursorAdapter(Context context, int layout, Cursor c,
> +                String[] from, int[] to) {
> +            super(context, layout, c, from, to);

drop the Cursor if its not used

::: mobile/android/base/resources/layout/abouthome_content.xml
@@ +26,5 @@
>  
> +            <TextView android:id="@+id/top_sites_title"
> +                        android:layout_width="fill_parent"
> +                        android:layout_height="wrap_content"
> +                        android:layout_marginLeft="12dip"

indentation is off

Comment 21

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

Comment on attachment 580806 [details] [diff] [review]
(1/4) Change indentation of about:home layout files for consistency

I like having all the arguments at the same indentation level (like it is now), but I'm not going fight this fight.

Comment 22

[Lucas Rocha (:lucasr)]

(In reply to Brad Lassey [:blassey] from comment #20)
> Comment on attachment 580926 [details] [diff] [review]
> (4/4) Implement AboutHomeContent layout as per design
> 
> Review of attachment 580926 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mobile/android/base/AboutHomeContent.java
> @@ +77,5 @@
> > +    private static final int NUMBER_OF_TOP_SITES_PORTRAIT = 4;
> > +    private static final int NUMBER_OF_TOP_SITES_LANDSCAPE = 3;
> > +
> > +    private static final int NUMBER_OF_COLS_PORTRAIT = 2;
> > +    private static final int NUMBER_OF_COLS_LANDSCAPE = 3;
> 
> defining the number of rows makes sense, but it would seem better to make
> the number of columns depend on the screen width. No?

Good point. You mean not hardcoding a number of columns and simply relying on the scape available for the grid, right? I'll try that.

> @@ +124,5 @@
> > +                    mUriLoadCallback.callback(spec);
> > +            }
> > +        });
> > +
> > +        mAddonsList = (ListView) findViewById(R.id.recommended_addons_list);
> 
> no click listener for the addons list?

Oops, forgot this bit. Is this supposed to go to the corresponding AMO page? Will do it now.

> @@ +289,5 @@
> >  
> > +    public class TopSitesCursorAdapter extends SimpleCursorAdapter {
> > +        public TopSitesCursorAdapter(Context context, int layout, Cursor c,
> > +                String[] from, int[] to) {
> > +            super(context, layout, c, from, to);
> 
> drop the Cursor if its not used

It's still used on the SimpleCursorAdapter side.

> ::: mobile/android/base/resources/layout/abouthome_content.xml
> @@ +26,5 @@
> >  
> > +            <TextView android:id="@+id/top_sites_title"
> > +                        android:layout_width="fill_parent"
> > +                        android:layout_height="wrap_content"
> > +                        android:layout_marginLeft="12dip"
> 
> indentation is off

Fixed.

Comment 23

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

(In reply to Lucas Rocha (:lucasr) from comment #22)
> (In reply to Brad Lassey [:blassey] from comment #20)
> > Comment on attachment 580926 [details] [diff] [review]
> > (4/4) Implement AboutHomeContent layout as per design
> > 
> > Review of attachment 580926 [details] [diff] [review]:
> > -----------------------------------------------------------------
> > 
> > ::: mobile/android/base/AboutHomeContent.java
> > @@ +77,5 @@
> > > +    private static final int NUMBER_OF_TOP_SITES_PORTRAIT = 4;
> > > +    private static final int NUMBER_OF_TOP_SITES_LANDSCAPE = 3;
> > > +
> > > +    private static final int NUMBER_OF_COLS_PORTRAIT = 2;
> > > +    private static final int NUMBER_OF_COLS_LANDSCAPE = 3;
> > 
> > defining the number of rows makes sense, but it would seem better to make
> > the number of columns depend on the screen width. No?
> 
> Good point. You mean not hardcoding a number of columns and simply relying
> on the scape available for the grid, right? I'll try that.
Yup, hard code the number of rows, but let the columns be determined by the space available (roughly screen-width / (tile width + min space between tiles).

> 
> > @@ +124,5 @@
> > > +                    mUriLoadCallback.callback(spec);
> > > +            }
> > > +        });
> > > +
> > > +        mAddonsList = (ListView) findViewById(R.id.recommended_addons_list);
> > 
> > no click listener for the addons list?
> 
> Oops, forgot this bit. Is this supposed to go to the corresponding AMO page?
> Will do it now.
Yup, the corresponding AMO page for now anyway. That should be in the json file we read in. 

> 
> > @@ +289,5 @@
> > >  
> > > +    public class TopSitesCursorAdapter extends SimpleCursorAdapter {
> > > +        public TopSitesCursorAdapter(Context context, int layout, Cursor c,
> > > +                String[] from, int[] to) {
> > > +            super(context, layout, c, from, to);
> > 
> > drop the Cursor if its not used
> 
> It's still used on the SimpleCursorAdapter side.
wow... for the life of me I couldn't find the c in the super class's constructor call. I must have been tired.

Comment 24

[Lucas Rocha (:lucasr)]

Follow-ups bugs:
Bug 710323 - about:home - clicking on addons should go to their page in AMO
Bug 710325 - about:home - show icon and version for each addon entry
Bug 710327 - about:home - layout shifts around when top site thumbnails are finally loaded
Bug 710332 - about:home - rotating device continuously causes thumbnails to disappear 
Bug 710333 - about:home doesn't fill whole screen when its content is not tall enough
Bug 710335 - about:home - add UI for when there are no thumbnails or no addons

Comment 25

[Lucas Rocha (:lucasr)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/65485f06a977
http://hg.mozilla.org/mozilla-central/rev/a93bacda40f7
http://hg.mozilla.org/mozilla-central/rev/e80376fab448
http://hg.mozilla.org/mozilla-central/rev/bdbc244778bc

Comment 26

[Alex Limi (:limi) — Firefox UX Team]

I like this a lot, but the thumbnails are pretty much unusable at this size. Any chance we can do the "partial thumbnail" approach here? E.g. top left 30% of the site screenshot instead of the whole thing. 

Should I file a follow-up bug? :)

Comment 27

[Ian Barlow (:ibarlow)]

Hi Alex, thanks for the comment -- I started bug 709844 to get us to start exploring better cropping, so feel free to follow along and chime in there! :)

Comment 28

[Sriram Ramasubramanian [:sriram]]

I would like the text sizes to use Android's specified Small, Medium and Large sizes. Currently 9px (or 9sp) is barely readable even on Galaxy Nexus. (The Small size is 12sp if I'm right).

Comment 29

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

(In reply to Sriram Ramasubramanian [:sriram] from comment #28)
> I would like the text sizes to use Android's specified Small, Medium and
> Large sizes. Currently 9px (or 9sp) is barely readable even on Galaxy Nexus.
> (The Small size is 12sp if I'm right).

File it!

Comment 30

[Andreea Pod]

Verified fixed on:
-build: Firefox for Android 22.0a1 (2013-02-20)
-device: Samsung Galaxy Nexus
-OS: Android 4.2.2