704784 - Only show scrollbars while panning (finger down), fade away on finger up

Status: VERIFIED FIXED

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

Description

[Steffen Wilberg]

Mark Finkle (:mfinkle) said in bug 701351 comment 12:
> We need the scrollbars to fade away after panning is finished.Finger down == scrollbars visible, Finger up == scrollbars fade away.

Comment 1

[Steffen Wilberg]

(That didn't wrap nicely)

We need the scrollbars to fade away after panning is finished.
Finger down == scrollbars visible, Finger up == scrollbars fade away.

Comment 2

[Madhava Enros [:madhava]]

We need to establish the look here; I think we just want to emulate some generation (gingerbread) of native ones?

Comment 3

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

wesj pointed me to http://developer.android.com/reference/android/view/View.html#awakenScrollBars%28%29 which might let us just use the built-in scrollbars if we can get them to size correctly. I totally missed that when I looked for android scrollbars previously, so we might not need graphics after all.

Comment 4

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

I tried a bunch to get the built-in scrollbars working, but I was unsuccessful. The main reason is that we are using a GLSurfaceView, and instead of drawing itself, it delegates drawing to a GLSurfaceRenderer, so the code flow is completely different. The draw code in the view itself is never used and so the scrollbars are never drawn, even after I enabled them and got awakenScrollbars() to return true.

I'm going to have to fall back to our scrollbar layers, but make them look nicer.

Comment 5

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

Attachment: (1/2) make the scrollbars look nicer

Comment 6

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

Attachment: (2/2) fade scrollbars away

Comment 7

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

I chose viewport/page size changes as the trigger rather than touch events so the scrollbars stay up during flings and if the page does a scrollTo, as per discussionn with Madhava yesterday.

Comment 8

[Patrick Walton (:pcwalton)]

Comment on attachment 580421 [details] [diff] [review]
(1/2) make the scrollbars look nicer

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

r=me with changes

::: mobile/android/base/gfx/ScrollbarLayer.java
@@ +87,5 @@
>          return new ScrollbarLayer(image, vertical);
>      }
>  
> +    /* Returns the power of two that is greater than or equal to value */
> +    private int getPowerOfTwo(int value) {

A faster way:

http://acius2.blogspot.com/2007/11/calculating-next-power-of-2.html

Cwiiis needed this function too. Might want to factor this out into a protected static method of Layer.

@@ +114,5 @@
>  
> +            float viewHeight = context.viewport.height();
> +
> +            // for the body of the scrollbar, we take a 1x1 pixel from the center of the image ...
> +            int[] cropRect = new int[] { CAP_RADIUS, CAP_RADIUS, 1, 1 };

Could you factor these out into private static finals, since they're constant? Would save a few allocations and lower GC pressure a bit.

Comment 9

[Patrick Walton (:pcwalton)]

Comment on attachment 580423 [details] [diff] [review]
(2/2) fade scrollbars away

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

This is the same as the first patch -- did you upload the wrong one?

Comment 10

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

Attachment: (2/2) fade scrollbars away

Upload correct patch.

Comment 11

[Patrick Walton (:pcwalton)]

Comment on attachment 580495 [details] [diff] [review]
(2/2) fade scrollbars away

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

r=me with changes

::: mobile/android/base/gfx/LayerRenderer.java
@@ +128,5 @@
>          LayerController controller = mView.getController();
>          Layer rootLayer = controller.getRoot();
>          RenderContext screenContext = createScreenContext(), pageContext = createPageContext();
>  
> +        if (! pageContext.fuzzyEquals(mLastPageContext)) {

nit: if (!pageContext.fuzzyEquals(mLastPageContext))

@@ +277,5 @@
>          }
>      }
> +
> +    class FadeRunnable implements Runnable {
> +        private boolean mStartScheduled;

Maybe "mStarted" would be clearer?

@@ +282,5 @@
> +        private long mRunAt;
> +
> +        void scheduleStartFade(long delay) {
> +            mRunAt = SystemClock.elapsedRealtime() + delay;
> +            if (! mStartScheduled) {

nit: if (!mStartScheduled)

::: mobile/android/base/gfx/ScrollbarLayer.java
@@ +100,5 @@
>          }
>          return value * 2;
>      }
>  
> +    public boolean fade() {

Please document what the return value of this means.

Comment 12

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

Attachment: (1/2) make the scrollbars look nicer

Updated as per comments, carrying forward r+

Comment 13

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

Attachment: (2/2) fade scrollbars away

Updated as per comments, carrying forward r+

Comment 14

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

Landed on inbound:

https://hg.mozilla.org/integration/mozilla-inbound/rev/daeda7b6d249
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8dad67cd332

Comment 15

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/daeda7b6d249
https://hg.mozilla.org/mozilla-central/rev/a8dad67cd332

Comment 16

[Camelia Urian]

Build ID: Mozilla/5.0 (Android; Linux armv7l; rv:11.0a1) Gecko/20111211 Firefox/11.0a1 Fennec/11.0a1
Device: Samsung Nexus S
OS: Android 2.3

Scrollbars are displayed while panning (finger down), scrollbars fade away on finger up.
Marking bug as verified.