search

kitasuke / PagingMenuController

Paging view controller with customizable menu in Swift
MIT License
2.49k stars 448 forks source link

Add support for didScrollStart and didScrollEnd. #318

Closed spsammy closed 7 years ago

spsammy commented 7 years ago

From comments from Pull Request https://github.com/kitasuke/PagingMenuController/pull/316

kitasuke commented 7 years ago

Thanks for your PR! I have one concern about this change. I think these event should also be triggered by menu view. Is there any inconvenience reason for this case?

spsammy commented 7 years ago

Hi sorry for the delay. I'll make the changes requested and resubmit.

kitasuke commented 7 years ago

Thanks for pointing out. What do you think whether these event should be triggered by MenuView?

spsammy commented 7 years ago

To be honest, I'm not sure what the problem is there. Things are working for me in my app with these changes.

kitasuke commented 7 years ago

In terms of consistency, it's better to trigger these event by any scrollable view which is PagingViewController and MenuView in this library. Your PR looks good for PagingViewController usage, but this feature should be released with support for both usages. I can take over the rest of work. However, this change might not work after I made some changes for MenuView usage since I have to modify current delegate method of UIScrollView.

What I'm saying is that this PR looks good, but I can't merge without support for MenuView usage.

kitasuke commented 7 years ago

I thought this should work when MenuView is paged, but maybe we can leave it as it is. I apologize for any confusion I mentioned about earlier. I'll merge this PR since it works great!

spsammy commented 7 years ago

Thanks !