Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request: show how to use with a CollapsingToolbarLayout #7

Closed
AndroidDeveloperLB opened this issue Dec 18, 2019 · 15 comments
Closed

Request: show how to use with a CollapsingToolbarLayout #7

AndroidDeveloperLB opened this issue Dec 18, 2019 · 15 comments
Assignees
Labels
question Further information is requested

Comments

@AndroidDeveloperLB
Copy link

Currently for some reason when you scroll to the bottom, the fast-scroll can either stop before the RecyclerView:

image

Or it can reach too far:

image

@zhanghai
Copy link
Owner

What is the condition to reproduce each case?

This library doesn't have any magic for drawing, and just respects the RecyclerView bounds (and padding) by default.

@AndroidDeveloperLB
Copy link
Author

It's the same one.
I tried to make a sample now.
I think it's similar to what I have on the real app.

Please advise what could be done, and also consider to add something similar in your sample.

device-2019-12-18-131421.zip
My Application.zip

@zhanghai
Copy link
Owner

The first case is caused by your bottom padding on RecyclerView - the padding is respected by this library by default, and to override it you need to call FastScrollerBuilder.setPadding(). This is actually coverd in the sample, where part of the bottom padding is for FAB and part of it is for navigation bar.

The second problem is likely caused by CollapsingToolbarLayout pushing the RecyclerView down. There is no elegant way to do it, and my workaround in my own app is to add to the bottom padding upon RecyclerView being pushing down (link). But this also means triggering re-layout, etc.

@zhanghai zhanghai self-assigned this Dec 18, 2019
@zhanghai zhanghai added the question Further information is requested label Dec 18, 2019
@AndroidDeveloperLB
Copy link
Author

AndroidDeveloperLB commented Dec 18, 2019

The padding is done because there is a FAB, and it's in the guidelines to avoid having a FAB hiding content on the last items.

Do you know of a nice solution? How do other, similar libraries handle those issues?

I used to use this solution, but it had issues with fragments

@zhanghai
Copy link
Owner

I didn't say there can't be padding for FAB, and I mentioned the sample app has that padding as well. Could you try the solutions in my last comment?

@AndroidDeveloperLB
Copy link
Author

Can't you put the fast-scroller and its bubble on a new View that draws them, which can have behavior that will be respected by the CoordinatorLayout ?

As for padding, how come it has "setPadding" but no "getPadding" ? How could I know what to put to it? I don't want to change all of the padding. Just probably need to change the bottom one, no?

As for the sample, only placed I see it's used is in ScrollingViewOnApplyWindowInsetsListener.
So I tried to use it instead.

Attached the sample now and a video of it. Do you think it is ok now? I hope I can use a similar solution on the real app.

device-2019-12-18-225741.zip
MyApplication2.zip

BTW, in your sample I see the FAB on top of the bubble, and you wrote on the repository explanation that it can be fixed. Can you please update the sample to show how ?

@zhanghai
Copy link
Owner

Can't you put the fast-scroller and its bubble on a new View that draws them, which can have behavior that will be respected by the CoordinatorLayout ?

I can, but didn't. (They are actually in new views, just in the overlay view group of the RecyclerView.) Having to specify where to put views are cumbersome, complicates layout code and may trigger unnecessary re-layouts. It's just a desgin decision.

As for padding, how come it has "setPadding" but no "getPadding" ? How could I know what to put to it? I don't want to change all of the padding. Just probably need to change the bottom one, no?

Because it's kind of confusing if we have a getPadding(). The padding in FastScroller works by first respecting a user-set padding via setPadding(), then falling back to the RecyclerView's padding. When there is no user-set padding, the result of getPadding() can either be null (unexpected null) or the RecyclerView padding (but no padding was provided by setPadding(), and it might change with RecyclerView without setPadding() being called). Meanwhile, the developer wrote the UI so they must know what the padding should be, either 0, or RecyclerView.getPadding*(), or combined with window insets.

As for the sample, only placed I see it's used is in ScrollingViewOnApplyWindowInsetsListener.
So I tried to use it instead.

ScrollingViewOnApplyWindowInsetsListener mostly handles window insets. In your case, you can call FastScrollerBuilder.setPadding(0, 0, 0, 0) to tell the impl not to respect any RecyclerView padding. For RecyclerView being pushed down by CoordinatorLayout, please check out the link I posted in my previous comment.

BTW, in your sample I see the FAB on top of the bubble, and you wrote on the repository explanation that it can be fixed. Can you please update the sample to show how ?

I don't remember saying that, and it can be cumbersome to fix. Even Google Contacts app (the only Google app I know to have fast scrolling) shows the popup under FAB, so I just didn't bother.

Apart from the code, could you please refrain from using rhetorical questions, like "Can't you", ", no?", or even "How come", etc... I feel unpleasant about them and I'm voluntarily answering questions here, so a nicer tone would be appreciated.

@AndroidDeveloperLB
Copy link
Author

Already reported about the bubble issue here :
https://issuetracker.google.com/issues/37065069

As for the rest, I don't understand. You mean that it's already fixed? Or that I should try something and let you know? The sample is already attached. You can try it too... :)

@zhanghai
Copy link
Owner

If Google Contacts fixed the popup & FAB issue I'll consider fixing it in this library as well.

For your issue with AppBarLayout.ScrollingViewBehavior, It's not a bug in this library and thus can't get fixed. I already pointed out a solution in my previous comment, which includes a link to source code of my app dealing with the CoordinatorLayout and view behavior pushing RecyclerView down, and my app works well, so please read my previous comment again.

@AndroidDeveloperLB
Copy link
Author

I believe they will fix it, eventually, or just use a simple solution such as hiding the FAB.

As for the issue, I still don't understand what is the solution. I already looked at the code. You want me to use ScrollingViewOnApplyWindowInsetsListener ?

@zhanghai
Copy link
Owner

ScrollingViewOnApplyWindowInsetsListener mostly handles window insets. In your case, you can call FastScrollerBuilder.setPadding(0, 0, 0, 0) to tell the impl not to respect any RecyclerView padding. For RecyclerView being pushed down by CoordinatorLayout, please check out the link I posted in my previous comment.

@zhanghai zhanghai closed this as completed Jan 3, 2020
@AndroidDeveloperLB
Copy link
Author

AndroidDeveloperLB commented Jan 4, 2020

The link is about 1300 lines of code. Which part exactly in it do you handle CoordinatorLayout ? I don't see FastScrollerBuilder/CoordinatorLayout there.
But I do see ScrollingViewOnApplyWindowInsetsListener, again...

@zhanghai
Copy link
Owner

zhanghai commented Jan 4, 2020

The link contains a line number so you don't need to read 1000 lines of code.

@AndroidDeveloperLB
Copy link
Author

OK I hope this will work on the big project. Thank you.

@herbeth1u
Copy link

The trick with adding an OffsetChangedListener implemented here worked like a charm for me! Saved my day. The good thing is it also works for any AppBarLayout, even when its children don't have scrollFlags="scroll", and it also works for NestedScrollViews.

For anyone interested, I made an extension out of it :

fun AppBarLayout.fixForFastScroll(container: ViewGroup) {
    val contentLayoutInitialPaddingBottom = container.paddingBottom
    addOnOffsetChangedListener(AppBarLayout.OnOffsetChangedListener { _, offset ->
        container.setPaddingBottom(contentLayoutInitialPaddingBottom + totalScrollRange + offset)
    })
}

Even if it's not included in the library, it might be a good idea to document it somewhere. I was actually going to create a new issue before stumbling upon this one.
Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants