plotly.js
plotly.js copied to clipboard
add support for min/max scale limits
Resolves #5191
This is my first contribution to this repository. Please review this contribution thoroughly! I'm especially not sure if I thought of every edge case of all geo-charts and their parameters.
I could use some help on how to tackle the failed tests in the ci. I tried to run the tests locally but got too many errors.
@archmoj Thanks for reviewing this!
Right now if I set minscale and maxscaleto 1, I expect the graph not to be scaled. However, usingwheel` scroll zoom the scale changes once.
To fix that you should likely constrain scales in this function as well:
https://github.com/plotly/plotly.js/blob/5d4c888407306f2719c76c67d79f8203bcdaac1a/src/plots/geo/zoom.js#L85-L97
@mojoaxel we are planning next plotly.js minor (possibly next week) to include new features like this one. Do you have time to address the remaining comments, so that we could include this feature?
Right now if I set minscale and maxscaleto 1, I expect the graph not to be scaled. However, usingwheel` scroll zoom the scale changes once.
I had a look at this a few days ago. That looks like a d3 issue to me. I could not figure it out.
To fix that you should likely constrain scales in this function as well:
I tried that code and it gets not called at all.
@mojoaxel we are planning next plotly.js minor (possibly next week) to include new features like this one. Do you have time to address the remaining comments, so that we could include this feature?
Cool, I'll try to look into the open issues today, but I'm not sure that I'll manage to fix the open issues (alone).
@archmoj Feel free to take my code, fix it and create your own pull-request. At the moment I don't have the time to dive deep into this complex codebase.
A small update on timing: our team is working hard on releasing v2.0 of Plotly.js, which we anticipate will happen in early April. This PR would be a good candidate to land in the library in v2.1 or later, so with apologies for the delay, we will likely not be able to give much feedback on this PR for the next few weeks :)
This pull request has been sitting for a while, so I would like to close it as part of our effort to tidy up our public repositories. I've assigned it to myself to keep track of it; I'll wait until 2024-06-17 for someone to say it's still relevant and they'll to take it on, and otherwise I will close it then. Thanks - @gvwilson
I'll wait until 2024-06-17 for someone to say it's still relevant and they'll to take it on, and otherwise I will close it then.
I still would love to see this merged. I rebased on the current master but I could use some help with the test.
At the moment I would consider this still work-in-progress. I'll try to finish this until 2024-06-17.
thanks very much @mojoaxel - much appreciated.