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

fix: trigger onZoomStart from pinchStart #810

Merged
merged 1 commit into from
Nov 14, 2024

Conversation

ZaLiTHkA
Copy link
Contributor

this simply ensures that the custom onZoomStart handler is also executed when zooming is initiated with "pinch".

there seems to be a fair difference in the logic structure between the handlers.js and hammer.js files, so I tried to keep this in line with what I already saw in hammer.js directly.

I did not add any tests for this, though I feel like we probably should.. perhaps one of the repo maintainers could help determine where this test should go? I don't see any others that cover the "pinch" functionality specifically.

this follows on from PR #487, and fixes issues #631 and #790.

@trullock
Copy link
Contributor

trullock commented Oct 1, 2024

This is merged into my fork at https://github.com/trullock/chartjs-plugin-zoom which is published on npm as @trullock/chartjs-plugin-zoom, as this fork seems abandoned

@ZaLiTHkA
Copy link
Contributor Author

ZaLiTHkA commented Oct 1, 2024

thanks @trullock, I'll try it out. I see your fork consolidates a fair amount of work from multiple PRs.. just out of curiosity, are you actively using it in one of your own projects as well?

I still think it would be nice to try get these changes into this root project, if possible.. this way the fixes and improvements could reach more users more easily, including your contributions.

so I'll leave this PR open for a little bit longer, just to see where this all goes. 👍🏼

@trullock
Copy link
Contributor

trullock commented Oct 1, 2024

Absolutely, you should leave this open.

I dont want to be maintaining a fork 🙈

@ZaLiTHkA
Copy link
Contributor Author

ZaLiTHkA commented Oct 1, 2024

I dont want to be maintaining a fork 🙈

yeah, completely understand that. it's definitely not a job for just anybody.. I only wish I had the time to even consider making commitments like that.

@trullock
Copy link
Contributor

trullock commented Oct 1, 2024

My fork works for me, thats all I'm guaranteeing! 😂

@kurkle kurkle closed this Nov 14, 2024
@kurkle kurkle reopened this Nov 14, 2024
@kurkle kurkle merged commit 0b79870 into chartjs:master Nov 14, 2024
12 checks passed
@ZaLiTHkA
Copy link
Contributor Author

happy days! thanks @kurkle, looking forward to switching my package.json back to the official package. 💯

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.

4 participants