Status

()

Firefox for Android
General
P2
normal
VERIFIED FIXED
6 years ago
4 years ago

People

(Reporter: ibarlow, Assigned: lucasr)

Tracking

unspecified
All
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox11 fixed, fennec11+)

Details

Attachments

(11 attachments, 3 obsolete attachments)

79.43 KB, image/png
Details
1.13 MB, image/png
Details
550.45 KB, application/zip
Details
233.75 KB, image/png
Details
7.46 KB, patch
blassey
: review+
Details | Diff | Splinter Review
1.31 KB, patch
blassey
: review+
Details | Diff | Splinter Review
2.67 KB, patch
blassey
: review+
Details | Diff | Splinter Review
129.84 KB, image/png
Details
118.86 KB, image/png
Details
423.40 KB, application/zip
Details
456.75 KB, patch
blassey
: review+
Details | Diff | Splinter Review
(Reporter)

Description

6 years ago
Created attachment 578080 [details]
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.
(Reporter)

Comment 1

6 years ago
Created attachment 578082 [details]
Graphic assets for start page

Includes list arrow and thumbnail shadow
(Assignee)

Updated

6 years ago
Assignee: nobody → lucasr.at.mozilla
OS: Mac OS X → Android
Hardware: x86 → All
(Reporter)

Updated

6 years ago
Blocks: 706821
(Reporter)

Comment 2

6 years ago
Background colours

Light blue: d5e8f7

List Selection background colour: 8fc0e2

Dark blue form top sites: 4e77ae
Priority: -- → P2
Created attachment 578290 [details]
screenshot

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

Comment 4

6 years ago
Ian, we probably need specs for landscape orientation too? How is layout supposed to look in landscape?
(Reporter)

Comment 5

6 years ago
Created attachment 579832 [details]
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.
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?
(Reporter)

Comment 7

6 years ago
Created attachment 580053 [details]
Graphic assets for start page
Attachment #578082 - Attachment is obsolete: true
(Reporter)

Comment 8

6 years ago
Created attachment 580063 [details]
UI Specs
Attachment #578080 - Attachment is obsolete: true
(Assignee)

Comment 9

5 years ago
Created attachment 580806 [details] [diff] [review]
(1/4) Change indentation of about:home layout files for consistency

Cosmetic, no functional change.
Attachment #580806 - Flags: review?(blassey.bugs)
(Assignee)

Comment 10

5 years ago
Created attachment 580807 [details] [diff] [review]
(2/4) Remove unused properties from AboutHomeContent
Attachment #580807 - Flags: review?(blassey.bugs)
(Assignee)

Comment 11

5 years ago
Created attachment 580808 [details] [diff] [review]
(3/4) Organize/cleanup imports on AboutHomeContent
Attachment #580808 - Flags: review?(blassey.bugs)
(Assignee)

Comment 12

5 years ago
Created attachment 580809 [details] [diff] [review]
(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.
Attachment #580809 - Flags: review?(blassey.bugs)
(Assignee)

Comment 13

5 years ago
Created attachment 580810 [details]
Screenshot (portrait)

Ignore the white thumbnails, this is an unrelated issue.
(Assignee)

Comment 14

5 years ago
Created attachment 580811 [details]
Screenshot (landscape)

Ignore the white thumbnails, this is an unrelated issue.
(Assignee)

Updated

5 years ago
Attachment #580809 - Attachment description: Implement AboutHomeContent layout as per design → (4/4) Implement AboutHomeContent layout as per design
(Assignee)

Comment 15

5 years ago
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).
(Reporter)

Comment 16

5 years ago
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!
(Reporter)

Comment 17

5 years ago
Created attachment 580902 [details]
Updated logo assets with cropped edge
(Assignee)

Comment 18

5 years ago
(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.
(Assignee)

Comment 19

5 years ago
Created attachment 580926 [details] [diff] [review]
(4/4) Implement AboutHomeContent layout as per design
Attachment #580809 - Attachment is obsolete: true
Attachment #580809 - Flags: review?(blassey.bugs)
Attachment #580926 - Flags: review?(blassey.bugs)
Blocks: 704864
Attachment #580807 - Flags: review?(blassey.bugs) → review+
Attachment #580808 - Flags: review?(blassey.bugs) → review+
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
Attachment #580926 - Flags: review?(blassey.bugs) → review+
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.
Attachment #580806 - Flags: review?(blassey.bugs) → review+
(Assignee)

Comment 22

5 years ago
(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.
(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.
(Assignee)

Comment 24

5 years ago
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
(Assignee)

Comment 25

5 years ago
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
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED

Updated

5 years ago
Depends on: 710385

Updated

5 years ago
No longer depends on: 710385
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? :)
(Reporter)

Comment 27

5 years ago
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! :)
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).
(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!
tracking-fennec: --- → 11+
status-firefox11: --- → fixed

Comment 30

4 years ago
Verified fixed on:
-build: Firefox for Android 22.0a1 (2013-02-20)
-device: Samsung Galaxy Nexus
-OS: Android 4.2.2
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.