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.

Sign up for GitHub

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

Jump to bottom

Feature/disable zoom #31

Merged
merged 5 commits into from Jun 12, 2018
Merged

Feature/disable zoom #31

merged 5 commits into from Jun 12, 2018

Conversation

markusressel
Copy link
Collaborator

@markusressel markusressel commented Jun 7, 2018 •
edited

  • implemented zoomEnabled, horizontalPanEnabled and verticalPanEnabled options

Note that this code only controls zoom and pan input from the user. Methods like zoomIn() or panBy() will still work even if any of the above mentioned attributes are set to false.

This is a PR for Issue #16 and #15

@markusressel
added zoomEnabled attribute
5099b5e 
added setZoomEnabled() method to ZoomApi
added implementations for ZoomLayout and ZoomImageView
@markusressel
added xml attributes "verticalPanEnabled" and "horizontalPanEnabled"
0f50fd3 
added api methods for "setXxxPanEnabled" methods
added implementation for "setXxxPanEnabled" in ZoomEngine
@markusressel
changed documentation to include info about the attribute only affect…
4544ef1 
…ing touch input

added xml examples to README
@markusressel
added description about new api methods to README
9ae3b73
@markusressel
moved zoom api description to appropriate section
3ef7dbf
@markusressel
Copy link
Collaborator Author

Well there's a nother travis issue in this one... hope you can fix it easily.

@natario1
Copy link
Owner

natario1 commented Jun 11, 2018 •
edited

Thanks for this one as well 👍

About the from the user part, is this by choice? Do you think it's more useful than disabling it for all uses? (it probably would be easy to implement by returning false in setState()).

I'm just asking, I have not thought about this.

@natario1
Copy link
Owner

I have taken a look at the source now. Yeah, let's not block the API usage.

But about implementation in this PR, could you use setState() and return false there for scroll / pinch / fling based on the flag? So you can get rid of if branches, 0 velocity and so on. Would it work?

@markusressel
Copy link
Collaborator Author

As for the "from the user" part, yes this is intentionally.
I don't think it's a good design to block the programmer from calling things as he should know about the state of his application anyway.

A more sophisticated implementation could allow disabling things for the user and the programmer separately, but I don't think this really is a necessity.

I think I could use the setState() method to completely block scrolling and pinching, but to block vertical and horizontal scroll separately I need the "0 velocity" thingy, or am I missing something?

@markusressel
Copy link
Collaborator Author

Just to add to the last section:
If the state only has control over full block/no block imho the current solution is the simpler and therefore better one, as the boolean that affects the behaviour is used (accessed) at the exact spots where the behaviour is changed, and doesn't change some "abstract" state that implicitly changes the behaviour.

Imho changing the behaviour partly via state and partly using 0 values that should come from the same "preference" (the enabled flag) would complicate things unnecessarily.

But of course you are welcome to disagree with me on this.

@natario1
Copy link
Owner

No, I agree, I didn't think about the vertical / horizontal issue.

natario1
natario1 approved these changes Jun 12, 2018
View reviewed changes
Copy link
Owner

@natario1 natario1 left a comment

Choose a reason for hiding this comment

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

Thanks!

@natario1 natario1 merged commit ebda547 into natario1:master Jun 12, 2018
This was referenced Jun 12, 2018
Closed
Closed
@markusressel markusressel deleted the feature/disable_zoom branch June 15, 2018 21:29
markusressel added a commit that referenced this pull request Dec 26, 2018
@markusressel
Disable zoom, verticalPan and horizontalPan (#31)
1131545 
* added zoomEnabled attribute
added setZoomEnabled() method to ZoomApi
added implementations for ZoomLayout and ZoomImageView

* added xml attributes "verticalPanEnabled" and "horizontalPanEnabled"
added api methods for "setXxxPanEnabled" methods
added implementation for "setXxxPanEnabled" in ZoomEngine

* changed documentation to include info about the attribute only affecting touch input
added xml examples to README

* added description about new api methods to README

* moved zoom api description to appropriate section
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@natario1 natario1 natario1 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@markusressel @natario1