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

Add toggle for enabling invisible scrollview (#1) #59

Closed
wants to merge 1 commit into from

Conversation

attyran
Copy link

@attyran attyran commented Jan 29, 2019

Hello,

I am using this library for a project, and needed a way to disable scrolling for the invisible scroll view. This doesn't seem to be available yet, so I forked the project and added functionality for it. Adding this PR in hope that it can go to the master branch eventually.

* Add toggle for enabling invisible scrollview

* Fix compiler error
@aataraxiaa
Copy link
Owner

Thanks for opening this @attyran 👍🏽. Is the intention that you can disable scrolling on the entire carousel?

i.e What is the use case for this?

Once I know this, I can review this better.

@attyran
Copy link
Author

attyran commented Jan 30, 2019

Hi @superpeteblaze,

Yes, that is the intention. For my project, I have a selected state for the centered cell. Upon clicking on the cell, I would magnify it and hide the left and right cells next to it. In addition, I would disable scrolling. Clicking on the cell again would scale it back, reshow the left and right cells, and enable scrolling.

Hope that helps.

@aataraxiaa
Copy link
Owner

Great, thanks for getting back to me.

In that case, I have a couple of suggested changes to the PR.

This method should ALWAYS be called from the ScalingCarousel delegate when you want to
disable the invisible scroller.
*/
public func toggleInvisibleScrolling(_ toggle: Bool) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than a new function, we could add a didSet on the UIScrollView isScrollEnabled, and in it set the value of the invisible scrollviews isScrollEnabled.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! That works also.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great. I think it accomplishes two things:

  • It makes the intent clearer
  • It does not expose the invisible scroll view

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To confirm, did you wanted me to make these changes :)?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did. 👍🏽

@aataraxiaa aataraxiaa closed this May 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants