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

Expand and pinch fire in alternating order on pinch #45

Open
danielksato opened this issue Aug 14, 2018 · 7 comments
Open

Expand and pinch fire in alternating order on pinch #45

danielksato opened this issue Aug 14, 2018 · 7 comments

Comments

@danielksato
Copy link

In release 1.0.6, when an expand listener and a pinch listener are both bound, the listeners fire in alternating order on a pinch. They do not fire at all on an expand.

Downgrading to 1.0.5 solves this issue, but only when the expand listener is bound before the pinch listener.

@mvanderkamp
Copy link
Contributor

My guess is that this is because of the way Pinch and Expand are structured to extend the same Distance class, which knows about Pinch and Expand (this seems like a bad idea) and demultiplexes on the constructor name.

Why is there even a difference between Pinch and Expand in the first place? There should just be a single Pinch class which reports a "changeSinceLast" or some such piece of data. If it's positive, you've got an expand, if it's negative, you've got a pinch. Simpler, clearer, easier to use, more efficient.

I'm not sure this library is still being maintained actively, so I'm thinking of developing my own fork. I would prefer to just contribute to this project, but the testing pipeline is broken, the devDependencies are deprecated, and the maintainer doesn't seem to be responding to issues or pull requests.

@danielksato
Copy link
Author

It's frustrating, because my organization has been looking at adopting zingtouch as a replacement for Hammer.js, which hasn't been updated in a year. I want to avoid forking if I can.

@mike-schultz
Copy link
Collaborator

Hey @mvanderkamp and @danielksato, the Pinch/Expand class were previously different enough to warrant their own classes, but at some point it made more sense to perform the logic in the distance class. While maintaining this project is not a top priority we will look into these issues this week. Feel free to submit a pull request to help out.

@danielksato
Copy link
Author

@mike-schultz Thanks for responding! I'll try to submit a PR this week if I have time, but unfortunately I can't make any guarantees - I've got a deadline looming myself.

@mvanderkamp
Copy link
Contributor

I've submitted a PR that seems to fix things up for me: #48

@mvanderkamp
Copy link
Contributor

Do you mind if I do a bit of general cleanup in the code?

@emri99
Copy link

emri99 commented Sep 18, 2018

Is there a release planned soon with this update ?

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

No branches or pull requests

4 participants