AndroidDeveloperLB / LollipopContactsRecyclerViewFastScroller

A sample of how to mimic the way that the contacts app handles a Fast-Scroller for a RecyclerView
Apache License 2.0
540 stars 95 forks source link

Bubble and Handler jumping #2

Closed davideas closed 9 years ago

davideas commented 9 years ago

Hi, while dragging the handler, the bubble (and handler as well) jump below of few positions, to return to it's position immediately after. This happens at each element scrolled in/out of the RecyclerView. Always with few elements.

Also while scrolling normally on the RecyclerView, the Handler jumps to the "section" and the movement is not linear.

AndroidDeveloperLB commented 9 years ago

Can't reproduce. Maybe try to make a video of those issues? Also, on which Android version and device/emulator did you try it on? Have you tried the sample, or a customized code?

davideas commented 9 years ago

I nested it in my App, didn't change the code. Genymotion Api 16 and my device with Lollipop. Elements 7 - 8 - 9 depends the height of the items

I will try to make the video, however I think that dragging the handler, triggers the method onScrolled, which makes calculations every time for next position of the handler and bubble.

AndroidDeveloperLB commented 9 years ago

Please try the sample first. I suggest that if there are just a few items, you should hide the fast-scroller. That's how the normal one behaves too, I think.

davideas commented 9 years ago

Here the video with the 2 behaviours in API 16 emulated (Same with Lollipop): https://www.dropbox.com/s/kc3x7kvdeiv2y46/capture_Sun_Jul_05_09.54.14.mkv?dl=0

And this is the original with ListView in API 16 emulated: https://www.dropbox.com/s/td6vc0seny5tltl/capture_Sun_Jul_05_10.03.39.mkv?dl=0

AndroidDeveloperLB commented 9 years ago

Please try the sample I've provided, and try to reproduce it there. If you can't please make a minimal project that I can look at, that has this issue. Maybe I will be able to check it out and see what's the problem.

On Sun, Jul 5, 2015 at 10:56 AM, Davide Steduto notifications@github.com wrote:

Here the video with the 2 behaviours: https://www.dropbox.com/s/kc3x7kvdeiv2y46/capture_Sun_Jul_05_09.54.14.mkv?dl=0

— Reply to this email directly or view it on GitHub https://github.com/AndroidDeveloperLB/LollipopContactsRecyclerViewFastScroller/issues/2#issuecomment-118594378 .

davideas commented 9 years ago

Found the source of the problem:

When you reproduce, you should also use the new methods: some are deprecated...

I've added more logs in the code, you can clearly see the "jump": Y receives an high value suddenly when a new item is shown: LOG: https://www.dropbox.com/s/7mzg1c5lyo2iitj/log.txt?dl=0

davideas commented 9 years ago

This is the code with logs: I've improved a little bit the variables: use directly float at assignment and don't cast them multiple times in the calculation, however this has no influence for the bug I've reported. because the problem is the library version :-)

private static final String TAG = FastScroller.class.getSimpleName();

private void setBubbleAndHandlePosition(float y) {
    int bubbleHeight = bubble.getHeight();
    int handleHeight = handle.getHeight();
    handle.setY(getValueInRange(0, height - handleHeight, (int) (y - handleHeight / 2)));
    bubble.setY(getValueInRange(0, height - bubbleHeight - handleHeight / 2, (int) (y - bubbleHeight)));
    Log.d(TAG, "setBubbleAndHandlePosition Y=" + bubble.getY());
}

@Override
public void onScrolled(RecyclerView rv, int dx, int dy) {
    View firstVisibleView = recyclerView.getChildAt(0);
    float firstVisiblePosition = recyclerView.getChildAdapterPosition(firstVisibleView);
    float visibleRange = recyclerView.getChildCount();
    float lastVisiblePosition = firstVisiblePosition + visibleRange;
    float itemCount = recyclerView.getAdapter().getItemCount();
    float position;

    Log.d(TAG, "------------------------");
    if (firstVisiblePosition == 0) {
        position = 0;
        Log.d(TAG, "onScrolled position if=" + position);
    } else if (lastVisiblePosition == itemCount) {
        position = itemCount;
        Log.d(TAG, "onScrolled position elseif=" + position);
    } else {
        Log.d(TAG,  "onScrolled firstVisiblePosition=" + firstVisiblePosition);
        Log.d(TAG,  "onScrolled itemCount=" + itemCount + " visibleRange="+visibleRange);
        Log.d(TAG,  "onScrolled itemCount - visibleRange=" + (itemCount - visibleRange));
        position = (firstVisiblePosition / (itemCount - visibleRange)) * itemCount;
        Log.d(TAG, "onScrolled position else=" + position);
    }
        float proportion = position / itemCount;
    Log.d(TAG,  "onScrolled proportion="+proportion);
    setBubbleAndHandlePosition(height * proportion);
}
AndroidDeveloperLB commented 9 years ago

How many items do you have in the list? I can't reproduce this issue. Tested the sample on HTC One M8 with Android 5.1.1 . I will now commit the changes.

davideas commented 9 years ago

So you changed the RecyclerView library to the latest and you can't reproduce it? Also, have you seen the log I posted? I've 10 elements in my test.

AndroidDeveloperLB commented 9 years ago

I see now. I've changed to 20 items, and it occurs. Wonder why it happens. I don't remember seeing this issue. Trying out version "21.0.3" of the support library and RecyclerView, it doesn't occur. I've restored the original version, to let people use the one that still works.

Please, if you find a way to fix it, let me know, as I can't handle this repo right now. :( Maybe post about this on Google groups? I've posted about it here: https://code.google.com/p/android/issues/detail?id=179285 Please write there more information if you can.

davideas commented 9 years ago

I am also quite busy now, but I would like to help if I find some time, since this is a thing that strangely is missing and we need it.. Basically we need to review the calculation.

AndroidDeveloperLB commented 9 years ago

ok, I've also made a StackOverflow post here: http://stackoverflow.com/q/31279151/878126

On Wed, Jul 8, 2015 at 12:49 AM, Davide Steduto notifications@github.com wrote:

I am also quite busy now, but I would like to help if I find some time, since this is a thing that strangely is missing and we need it.. Basically we need to review the calculation.

— Reply to this email directly or view it on GitHub https://github.com/AndroidDeveloperLB/LollipopContactsRecyclerViewFastScroller/issues/2#issuecomment-119352732 .

WarrenFaith commented 9 years ago

I answered there, hope it helps

davideas commented 9 years ago

@WarrenFaith I saw the answer, I gave a try and it works, smoothly, Thank you! The author can mark resolved the question. I didn't analyze the "call" to that function, but indeed it seems that, with new version library it is called and before it wasn't.

Now I'm trying to make linear the movement of the handler while sliding the RecyclerView... Also that, it's not cool to see when 7 > elements <~ 17.

AndroidDeveloperLB commented 9 years ago

@davideas Wait, so there is still some kind of issue or not?

davideas commented 9 years ago

What you asked in StackOverflow: Bubble jumping while dragging, has been resolved by adding the early return in onScroll, but there's also the second point of my first post.

AndroidDeveloperLB commented 9 years ago

You mean this: "Also while scrolling normally on the RecyclerView, the Handler jumps to the "section" and the movement is not linear." ? If so, did you show this issue in one of the videos you've made? Can you please create a new post ? I don't understand it. I thought it was about the same issue.

davideas commented 9 years ago

Yes it's visible in the video.

AndroidDeveloperLB commented 9 years ago

@davideas Please post the video and the question in a new post. @WarrenFaith Do you understand the other issue? Can you guys talk together about this?

WarrenFaith commented 9 years ago

I do understand but honestly it is not a real issue for my, at max a nice to have.

WarrenFaith commented 9 years ago

I played a bit with the onScrolled() method and managed a bit process but: my solution does not work really good. Problem is the fact that the visibleRange is not constant. So each calculation to move the handle a few pixels further is ruined as soon as the visibleRange increases/decreases by one. In that case you see it jumping again.

Just some sample log output of visibleRange. You can clearly see that it differs...

 29358   16:52:17.186      ScrollListener  E  ::onScrolled 6
 29358   16:52:17.204      ScrollListener  E  ::onScrolled 7
 29358   16:52:17.217      ScrollListener  E  ::onScrolled 7
 29358   16:52:17.236      ScrollListener  E  ::onScrolled 6
 29358   16:52:17.251      ScrollListener  E  ::onScrolled 6
 29358   16:52:17.268      ScrollListener  E  ::onScrolled 6
 29358   16:52:17.286      ScrollListener  E  ::onScrolled 6
 29358   16:52:17.309      ScrollListener  E  ::onScrolled 7
 29358   16:52:17.318      ScrollListener  E  ::onScrolled 7
davideas commented 9 years ago

I've played as well :-) IMO, now it is much much better, scrolling RecyclerView is smooth in all cases!

I removed all the old calculation. Now I use dy (delta y) and I put it in the calculation with the new proportion. Please have a look and let me know, this is the new code:

    @Override
    public void onScrolled(RecyclerView rv, int dx, int dy) {
        //Only react on scroll events when not done by moving the handler by touch
        // prevents nervous jumping of the handler
        if (handler.isSelected()) return;

        float itemCount = recyclerView.getAdapter().getItemCount();
        float visibleRange = recyclerView.getChildCount();

        Log.d(TAG, "------------------------");
        Log.d(TAG, "onScrolled itemCount=" + itemCount);
        Log.d(TAG, "onScrolled visibleRange=" + visibleRange);
        Log.d(TAG, "onScrolled dy=" + dy);

        //This decreases the proportion to adjust the final multiplication with dy.
        float c = itemCount / visibleRange;
        Log.d(TAG, "onScrolled factor c=" + c);

        //Subtracting visibleRange value: these items must removed from calculation
        // (increase proportion and scroll faster with few items)
        float proportion = (itemCount - visibleRange < 1) ? height :
                visibleRange / (itemCount - visibleRange + c);
        Log.d(TAG, "onScrolled proportion=" + proportion);

        float y = handler.getY() + dy * proportion;
        //dy could be bigger then necessary (due to fling) fix min and max value
        if (y < 0) y = 0;
        else if (y > height - handler.getHeight())
            y = height - handler.getHeight();

        Log.d(TAG,  "onScrolled Y=" + y);
        handler.setY(y);
    }
AndroidDeveloperLB commented 9 years ago

@davideas Please post new issues in new threads. Also, I've noticed that the fix caused a new issue to occur. When you play with the fast scroller and then scroll a bit yourself, the fast scroller jumps a little, sometimes even in the wrong direction.

EDIT: what is the new code you've written? Does it have those issues too? I've tested it, and it seems to work quite well. BTW, it's called "handle" and not "handler". a Handler is something else...

davideas commented 9 years ago

I didn't see any jump anymore especially with this new code. I've opened a new issue for smoothness so you can close this one. Thanks!