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 a function for getting zoom range #780

Merged
merged 4 commits into from
Nov 16, 2024
Merged

Conversation

joshkel
Copy link
Contributor

@joshkel joshkel commented Sep 4, 2023

This adds a function, getZoomedScaleBounds, to act as a counterpart to getInitialScaleBounds (getting the scale ranges after zoom rather than before) and zoomScale (reading, rather than writing, each scale's zoom).

A question for code review: Is it okay to return the internal object directly, or should I return a new object to ensure that it can't be mutated?

Update types for getInitialScaleBounds - its implementation shows that it may return undefined.
@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

@kurkle kurkle closed this Nov 14, 2024
@kurkle kurkle reopened this Nov 14, 2024
@kurkle
Copy link
Member

kurkle commented Nov 14, 2024

@joshkel would you be able to resolve the merge conflicts in this?

A question for code review: Is it okay to return the internal object directly, or should I return a new object to ensure that it can't be mutated?

I think its better to return a new object, as you did.

@kurkle kurkle merged commit dd0c8b3 into chartjs:master Nov 16, 2024
12 checks passed
@joshkel joshkel deleted the get-zoom-range branch November 20, 2024 20:00
@joshkel
Copy link
Contributor Author

joshkel commented Nov 20, 2024

Thanks for taking care of this, @kurkle. I apologize for not following up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants