787765 - Retheme about:home thumbnails

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: Theme and Visual Design, defect)

Description

[Wesley Johnston (:wesj)]

Attachment: Current about:home on Nexus 7

There's a mockup in bug 775334 and comments about thumbnail changes, but they never seemed to happen? Our thumbs on the Nexus 7 and tablets feel a bit funny to me. We should probably scale them to the available size.

Patch and some screenshots coming...

Comment 1

[Wesley Johnston (:wesj)]

Attachment: WIP Patch

Comment 2

[Wesley Johnston (:wesj)]

Attachment: Screenshot with patch

Screenshot with the patch. I think maybe on these larger screens we should default to three across rather than giving such huge click targets... ian?

Comment 4

[Ian Barlow (:ibarlow)]

Wes this is fantastic. I have a few comments below, but it's great to see these updates being applied. Also here is a flickr set you can use as reference for the thumbnail styling: http://www.flickr.com/photos/61892693@N03/sets/72157630730911182/detail/ -- as you can see the thumbnail layout adapts based on screen size and orientation

1. We should make sure we aren't generating thumbnails that are being stretched up in size. These look a pretty blurry.

2. Adjust thumbnail aspect ratio -- size can vary depending on screen, but ratio should be roughly 7w x 5h http://cl.ly/image/0v0R3P3I3j0r

3. Add drop shadow under thumbnails -- 2dp, no blur, black with 25% opacity

4. Enlarge page title text, bar overlay height

5. Bonus points for 2dp rounded corners :)

Comment 5

[Ian Barlow (:ibarlow)]

Oh, we should also check that our empty thumbnail box is updated as per the flickr set as well -- it should be a flat white icon, compared to the recessed looking one we used before. I believe I provided assets for this at some point, if not let me know!

Comment 6

[Wesley Johnston (:wesj)]

(In reply to Ian Barlow (:ibarlow) from comment #4)
> 1. We should make sure we aren't generating thumbnails that are being
> stretched up in size. These look a pretty blurry.

Yeah I should have explained that. Basically we cache these thumbnails. Since this is making them bigger (and Android really really REALLY sucks at resizing images), we initially get these bad looking ones until you visit the sites and they update to the correct size/aspect ratio. We could change this to just not show the thumbnails if they're the wrong size. Let me play with trying to find a way to turn on pretty resizing. I assume you would rather show white firefox icons than badly scaled thumbs?

Comment 7

[Sriram Ramasubramanian [:sriram]]

Drive by:
>>> int expandedHeightSpec = MeasureSpec.makeMeasureSpec(widthMeasureSpec,
Why does this use "width" to calculate the height?

Comment 8

[Wesley Johnston (:wesj)]

Attachment: WIP Patch v2

Getting better. Rounded corners are not Android's friend. Still some issues with 1.) rotation and 2.) gingerbread. Some screenies at: http://people.mozilla.org/~wjohnston/home/

I think I need that white firefox logo ian. I also rolled by own shadow + white background for the label.

Comment 9

[Eric Pang [:epang] UX]

Attachment: White Firefox Icon

Attached the white Firefox icon for empty thumbnails.

Comment 10

[Wesley Johnston (:wesj)]

Attachment: Patch v1

Looks pretty good to me. I'm sharing the same mask for all the thumbnails. I avoided using the CanvasDelegate stuff because I have a feeling that clipping using a Bitmap is faster than using a Path (but takes more memory).

Need to test on a tablet tomorrow. I'll post screenshots from two other devices first though.

Comment 11

[Wesley Johnston (:wesj)]

Attachment: Nexus 7

Screenshot on Nexus 7

Comment 12

[Wesley Johnston (:wesj)]

Attachment: Screenshot on DroidX

Comment 13

[Ian Barlow (:ibarlow)]

Attachment: UI tweaks

Looks awesome, we're almost there -- attached are a few tweaks.

Comment 14

[Ian Barlow (:ibarlow)]

Hm, one of my comments was about thumbnail spacing on the phone UI, but looking at the Nexus 7 screenshot, they actually look good there. So if it's possible to adjust the thumbnail spacing on the phone without affecting 7" tablets great. Otherwise maybe we can leave that part alone.

Comment 15

[Wesley Johnston (:wesj)]

Attachment: Patch v3

We can adjust phones, large, and xlarge ui separately with this.

Fixed up a bug in xlarge devices and adjusted for ians comments. Also found some annoying bugs when rotating xlarge devices that I fixed.

Tried hard to get the same or more padding on the left and right sides of the grid as there is between thumbs on a phone (right now there's 8dp padding around the thumbs which means there's 16dp between two thumbnails as well). Adding padding to the left and right of the grid view doesn't seem to fix that (maybe an issue with the onMeasure stuff, but none of my fixes seem to work).

I updated screenshots at:
http://people.mozilla.org/~wjohnston/home/

Comment 16

[Wesley Johnston (:wesj)]

Attachment: CanvasDelegate patch

This uses a canvasDelegate for the mask. Its slightly simpler, but requires some casting because we need to pass some objects around.

I've tried using Paint.setShadowLayer, but since we're not drawing these objects directly, we don't have the paint used for them. Maybe there's a technique I can't find?

Comment 17

[Wesley Johnston (:wesj)]

Comment on attachment 668671 [details] [diff] [review]
CanvasDelegate patch

This applies on top of the other patch.

Comment 18

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 668559 [details] [diff] [review]
Patch v3

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

static private int RADIUS = 2;

Please make it 0dp by default. Also use sRadius for a static "non-final" variable. The ALL-CAPS is usually used for a final variable.

Also, values-large/ and values-xlarge/ aren't needed.
Note behaves as a LARGE device on GB, and NORMAL on ICS. So, we force the v11 qualifier with it. Could you please change that?

The second patch had the null checks for static variables avoiding re-initialization. Please import them to this patch.

r+ with these changes.

Comment 19

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 668671 [details] [diff] [review]
CanvasDelegate patch

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

I'm sorry, but, this is getting more messier than I expected. We might not need this patch :(
Your patch sans this looks better than this. :(

Comment 20

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a7702d5a4834

Comment 21

[Ed Morley [:emorley]]

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

Comment 22

[Ioana Chiorean]

Attachment: Screenshot

Build ID: 19.0 (2012-10-16) Nightly Channel
Device: Samsung Galaxy Nexus
OS: Android 4.1

Now About:home thumbnails are bigger as intended. Setting the bug to Verified Fixed.

Comment 23

[Kartikaya Gupta (email:[e-mail])]

 
-    int getThumbnailWidth() {
-        int desiredWidth = (int) (GeckoApp.mAppContext.getResources().getDimension(R.dimen.tab_thumbnail_width));
-        return desiredWidth & ~0x1;
-    }
-
-    int getThumbnailHeight() {
-        int desiredHeight = (int) (GeckoApp.mAppContext.getResources().getDimension(R.dimen.tab_thumbnail_height));
-        return desiredHeight & ~0x1;
-    }
-

This code from Tab.java specifically returned even numbers from getThumbnailWidth and getThumbnailHeight to fix bug 776906. It looks like the new code doesn't do that any more, and so may introduce a regression on some devices.

Comment 24

[Wesley Johnston (:wesj)]

Can you file a bug for it?

Comment 25

[Kartikaya Gupta (email:[e-mail])]

Filed bug 803690 for it. Will also file a separate bug for the thumbnail corruption that bnicholson had STR for.