Issues with spring animation and collection view

[heltisace]

Hello again. At this moment you are probably already sick of me, though I found a few more problems. Sorry :P

1. Now if you enable bounce, with collection view having enough objects to scroll, and start scrolling from some, not the top point, you will see this.
   I checked that is caused by custom Spring animation. Good news is that it can be fixed if you check if the duration is 0 and if so don't animate. Though probably it could be better if pullUpControllerAnimate won't just be called if duration is 0? 🤔
2. That probably has nothing to do with you, but here is a thing: for scroll view not to scroll when reaching top like this I disable scroll in collection view when reaching the middle point, and enable back when reaching the top point. After that sometimes cells stop tapping at all, usually at the middle position. I tried detaching and attaching back the collection view from PullUpController as well, checked using Debug View Hierarchy and all objects are User Interactable. Just can't get what's going on. Probably some collection view issue, but it would be very nice if you could help in any way 🤔

Hoping that you don't hate me too much T.T,
Heltisace

[MarioIannotta]

Hi @heltisace, don't worry, I don't hate you :)
Regarding this first issue thank you for proposing a solution already, I'm not 100% sure that it will work though.
Check this quote from the UIView.animate(..'s docs

duration: The total duration of the animations, measured in seconds. If you specify a negative value or 0, the changes are made without animating them.

I think I know where the issue is and I'll try to fix it in the next couple of days.
Regarding the second point, I'm not quite sure to have understood the issue, but I think that you shouldn't disable the collection view scroll, everything should be handled by the library as it is in the demo project.
Maybe you could provide a really basic where I can reproduce this last issue?

Cheers,
Mario.

[heltisace]

@MarioIannotta Thanks :) Though adding guard animation > 0.0 seemed to solve my issue, at least issue definitely stopped appearing. Nevertheless, thank you.

Regarding the second question, shortly speaking, if you expand a pull controller that has some collection view, containing enough for scrolling items, you will continue the scroll. That means that user either has to scroll very carefully or will have to return to the top. Disabling scroll while the pull view controller is not fully expanded solves this issue and the user will be able to scroll only when he reached the top position. If it's not clear enough, don't worry, I will try to explain using some demo project tomorrow because it's kinda late already.

Thanks for trying to help :)

Edit: I meant guard duration > 0.0, sorry.

[MarioIannotta]

That's right, the guard solve the issue but I don't think is conceptually right (sorry I'm a little bit picky :))

Maybe I've found another way to solve the issue in the library itself. Do you mind trying it?
You should remove the guard and add

if !pullUpControllerIsBouncingEnabled {
   yTranslation -= initialInternalScrollViewContentOffset.y
} 

instead of

yTranslation -= initialInternalScrollViewContentOffset.y

In the method handlePanGestureRecognizer(_ gestureRecognizer: UIPanGestureRecognizer).
Does that solve your issue?

If you're using xcode 10 and you've installed the library using cocoapod, a clean could be necessary.

I'll tackle the other issue tomorrow, you're right it's late 😴
Thanks!

[heltisace]

Okay. Will let you know tomorrow. Good night.

Cheers :D
Heltisace

[heltisace]

Hi. This code indeed solves the issue.

if !pullUpControllerIsBouncingEnabled {
   yTranslation -= initialInternalScrollViewContentOffset.y
} 

Now back to the question I mentioned yesterday. I downloaded your test project and decided to show you what I want in it. First of all, I found an issue. Look at the bottom animation when the add button is pressed. Secondly, as you can see on this video if you try to expand PullUpController, starting the pan gesture on the table, in your case, it will continue scrolling farther. Imagine if you have only a table in your drawer. You will either have to return to the top or expand it very carefully. So what I want, is for drawer not to be scrolled in that case. My solution was to disable scroll at the middle position and then enable back at the top, though it's not only not the best decision but also sometimes cases such issues as some items stop to be clickable at the middle position. Do you have any ideas on how to solve this?

Thanks,
Heltisace

[heltisace]

Hi. Wanted to add, that it could be useful to be able to see the current point of PullUpController not only in willMove, or didMove, but in some property.

[MarioIannotta]

Hi,
I checked your video and now I've understood the problem, try to use the code i just pushed on develop and let me know how that works, if everything is okay I'll release a new versions.
A note:
Instead of pullUpControllerIsBouncingEnabled now there's pullUpControllerBounceOffset, a CGFloat that represents how much the view will bounce.
Regarding this thing, I've created another issue to keep track of it.

Cheers,
Mario.

[heltisace]

@MarioIannotta hi. Thanks for your work! If you are still able to listen to my complaints, this indeed makes things better, though if you do fast small scrolls you will get this. The idea with bouncing offset is brilliant and I also wanted to propose something like this.

UPD: In case you wonder what the heck this transparent button is doing in the bottom that is not your bug, I just decided to respond as fast as could, while was in process of implementing a new feature.

[heltisace]

@MarioIannotta also I'm not sure how much moveToVisiblePoint is sticky point(because functions are called willMoveToStickyPoint and didMoveToStickyPoint), because I have no idea how it works, want to get through all the code at some moment, though now have no time for it. So if after going to, for example, 100, and the sticky point is 120 and user will be processed further to 120 then is better to return 120 and not the visible point.
Sorry if I'm wrong or just such a pain :<

[MarioIannotta]

You're one hell of a tester! Thanks for finding out all of these bugs :)
I've pushed a little refactoring of the gestures handling on develop that looks so much better, could you please check that out and let me know?
Regarding this last issue I'm not sure I've understood your point, could you please it in the proper issue?

Thanks!
Mario

[heltisace]

Hi, thanks :) Surely I will look through changes tomorrow. Will try to explain my point in #35 in a moment.

[heltisace]

Hello. If you are still subscribed to this issue, then... Here's a vid.

Shortly:

1. I'm not quite sure if content should bounce when you swipe to the bottom if it doesn't bounce when you swipe to the top.
2. If you drug it slowly to the bottom, you will get this struggling thing.

Your's hell of a tester :P

UPD: I wasn't quite sure if you could see the second issue in the first video, so here's another one.
UPD 2: Sometimes when you scroll down top starts bouncing for some reason.

[MarioIannotta]

Hi again,
I've fixed the first issue.
I think the second issue (scroll lag when swiping to bottom), is related to the unexpected bounce when scrolling from the top to the bottom a little bit. It looks like there's some kind of offset that I'm not taking into account :/
I'll let you know if I can find something.

Update: It was a problem with the bouncing calculation, check out my last commit on develop and let me know!

Super many thanks!! :)

[heltisace]

H-h-h-hi.. I'm pretty much sure that I've downloaded the last version with last commit.. I've even checked new code lines that you added. Though.. Here.

[heltisace]

I've accidentally have written this to the wrong issue. I deal with content scrolling like this for now. If you try just to change collectioView's scroll enabled variable you will see that in the middle position collevtionView's items can't be pressed.

This in the subclass of my CustomPullController.

//MARK: The function that is called each time user is about to scroll
    func scrollViewWillBeginDragging(_ scrollView: UIScrollView) {
        //Setting scrolling value according to opened state
        collectionView.set(scrolling: state == .open)
    }

This is custom collection view.

//MARK: Custom collectin view to work with PullUp
class PullUpCollectionView: UICollectionView {
    //MARK: Variable saying if should be scrollable
    private var scrolling: Bool = false
    //MARK: Variable that saves initial content offset value
    private var initialContentOffset: CGPoint = .zero
    
    //MAKR: The function for setting if scrolling is enabled
    func set(scrolling: Bool) {
        //Setting scrolling
        self.scrolling = scrolling
        //And initial content offset
        self.initialContentOffset = contentOffset
    }
    
    //MARK: Overriding content offset
    override var contentOffset: CGPoint {
        //On set
        didSet {
            //Checking if value is not the initial and shouldn't scrolling
            if contentOffset != initialContentOffset && !scrolling {
                //Then setting value back to starter one
                contentOffset = initialContentOffset
            }
        }
    }
}

[heltisace]

In new the version, content keeps scrolling, if there are enough of elements when you scroll down PullUpController (this and plus those two bugs I've already named above). That's how it works in the old version + my custom collection view and that's what I wanted. In new the version, collection view doesn't help anymore. At least without any changes :(

[MarioIannotta]

Maybe I could revert that commit and you could create a PR adding your collection view subclass to fix the issue?

[heltisace]

@MarioIannotta maybe, though it seems to me like not quite a right type of solution, you trying to use in your lib. The only thing my collection view does is disabling scroll the way it doesn't cause any issues further. And then I just call it when PullUp starts moving and disable when it stops.

UPD: Plus, the code is already above. I just call set(scrolling: Bool) when needed, so if you think it will help you, for sure you may use it.