jQuery.Gantt icon indicating copy to clipboard operation
jQuery.Gantt copied to clipboard

Default scale should be adjusted if beyond given min/max scale (was: Zoom out not working)

Open cookiejest opened this issue 10 years ago • 5 comments

The zoom out button seems to have stopped working, even in the demo site.

cookiejest avatar Apr 29 '14 09:04 cookiejest

Whoa, you're absolutely correct--good catch, thanks! Should be an easy fix, though might need to wait a week or so before I can take a look. Feel free to submit a patch before then if you're so inclined!

usmonster avatar Apr 29 '14 12:04 usmonster

Can do.. Can you point me towards the solution? I'm not that familiar with the code!

Adam

Sent from my iPhone

On 29 Apr 2014, at 13:36, usmonster [email protected] wrote:

Whoa, you're absolutely correct--good catch, thanks! Should be an easy fix, though might need to wait a week or so before I can take a look. Feel free to submit a patch before then if you're so inclined!

— Reply to this email directly or view it on GitHub.

cookiejest avatar Apr 29 '14 13:04 cookiejest

@cookiejest I would point you to the zoomInOut function, but I would first suggest that you try to reproduce the issue using the most recent version of the plugin from the master branch. The version on the demo site is often behind.

usmonster avatar Apr 29 '14 13:04 usmonster

I have checked that function you suggest against an older version I have where zoom out is working and it looks identical.

Also checked the function against the latest version and doesn't look like it's changed so it must be something else?

Not sure I'm advanced enough to work this out. :S

cookiejest avatar Apr 29 '14 17:04 cookiejest

Hi @cookiejest! I just had a chance to look, and it seems that there are two issues at play here:

The first issue is that there's currently no sanity check to make sure the initial/default scale option doesn't violate the max/min scales, which is currently the case on the demo page (introduced in 5a52561). A minimal fix for this would be to adjust the default scale to be the closest of either maxScale or minScale (but only if the user doesn't already supply the scale option). Feel free to put this into a pull request, though it might be just as well to wait until #84 lands, as that will surely touch the same code.

The other issue is that the semantics of the maxScale/minScale options can seem kind of counterintuitive (I might actually say "wrong").. In this plugin, minScale denotes in fact the minimum time unit that you want the chart to display; i.e., how "close" you want to be able to "zoom in" to the timeline, or in other words, the scale with the maximum(!) zoom factor. maxScale does the opposite -- basically it's the limit for how far you can zoom out / the largest displayed time unit, i.e., the scale that has the minimum zoom factor. Yeah, that doesn't really make a lot of sense...

We could maybe deprecate those options and introduce new options with clearer names (minScale => zoomMax, maxScale => zoomMin?), but a more immediate minimal patch would be to just document this behavior. This, paired with the recommended fix to the default scale issue, would be a nice way to get your hands dirty and start contributing if you like! Otherwise there should be patches in the coming weeks.

Thanks again for reporting this!

usmonster avatar May 26 '14 16:05 usmonster