Closed Bug 704784 Opened 8 years ago Closed 8 years ago

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

Categories

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

ARM
Android
defect

Tracking

()

VERIFIED FIXED
Tracking Status
firefox11 --- fixed
fennec 11+ ---

People

(Reporter: steffen.wilberg, Assigned: kats)

References

Details

(Keywords: uiwanted)

Attachments

(2 files, 3 obsolete files)

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.
(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.
Assignee: nobody → kgupta
Status: NEW → ASSIGNED
Keywords: uiwanted
We need to establish the look here; I think we just want to emulate some generation (gingerbread) of native ones?
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.
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.
Priority: -- → P2
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 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.
Attachment #580421 - Flags: review?(pwalton) → review+
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?
Attachment #580423 - Flags: review?(pwalton) → review-
Attached patch (2/2) fade scrollbars away (obsolete) — Splinter Review
Upload correct patch.
Attachment #580423 - Attachment is obsolete: true
Attachment #580495 - Flags: review?(pwalton)
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.
Attachment #580495 - Flags: review?(pwalton) → review+
Updated as per comments, carrying forward r+
Attachment #580421 - Attachment is obsolete: true
Attachment #580537 - Flags: review+
Updated as per comments, carrying forward r+
Attachment #580495 - Attachment is obsolete: true
Attachment #580541 - Flags: review+
Blocks: 709560
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.
Status: RESOLVED → VERIFIED
tracking-fennec: --- → 11+
You need to log in before you can comment on or make changes to this bug.