Category pan #33
Category pan #33
Conversation
| @@ -0,0 +1 @@ | |||
| start cmd /k "cd c:/xampp/htdocs/Chart.Zoom" | |||
There was a problem hiding this comment.
should probably not include this file in the main repo
| @@ -132,7 +132,7 @@ | |||
| mode: 'xy' | |||
| }, | |||
| zoom: { | |||
| enabled: true, | |||
| enabled: false, | |||
There was a problem hiding this comment.
we shouldn't disable zoom in the zoom sample
| }, | ||
| zoom: { | ||
| enabled: true, | ||
| enabled: false, |
There was a problem hiding this comment.
This file is zoom-bar.html. I think it would be best to create a new pan only bar sample pan-bar.html
| var offsetAmt = Math.max((scale.ticks.length - ((scale.options.gridLines.offsetGridLines) ? 0 : 1)), 1); | ||
| var panSpeed = options.pan.speed; | ||
|
|
||
| console.log('panSpeed: ' + panSpeed); |
| @@ -145,10 +158,10 @@ function panNumericalScale(scale, delta) { | |||
| } | |||
| } | |||
|
|
|||
| function panScale(scale, delta) { | |||
| function panScale(scale, delta, panSpeed) { | |||
| @@ -159,7 +172,7 @@ function doPan(chartInstance, deltaX, deltaY) { | |||
|
|
|||
| helpers.each(chartInstance.scales, function(scale, id) { | |||
| if (scale.isHorizontal() && directionEnabled(panMode, 'x') && deltaX !== 0) { | |||
| panScale(scale, deltaX); | |||
| panScale(scale, deltaX, helpers.getValueOrDefault(chartInstance.options, defaultOptions)); | |||
There was a problem hiding this comment.
I think its best to either pass chartInstance.options or panOptions here depending on if we want the pan method to have access to all the options or just the pan options. It might make sense to add a check further up that ensures the options are present so that we don't have to have all kinds of these checks everywhere (i'm fine with doing that in a separate pull request)
There was a problem hiding this comment.
You had already done it!
function doPan(chartInstance, deltaX, deltaY) {
var panOptions = chartInstance.options.pan;
if (panOptions && helpers.getValueOrDefault(panOptions.enabled, defaultOptions.pan.enabled)) {
var panMode = helpers.getValueOrDefault(chartInstance.options.pan.mode, defaultOptions.pan.mode);
There was a problem hiding this comment.
Noticed that this wasn't picking up the default pan speed if there wasn't one in the user options.
doPan() now sets panOptions.speed to either the user or default value before passing panOptions to panScale
| @@ -343,6 +357,7 @@ var zoomPlugin = { | |||
| mc.on('panend', function(e) { | |||
| currentDeltaX = null; | |||
| currentDeltaY = null; | |||
| zoomNS.panCumulativeDelta = 0; | |||
There was a problem hiding this comment.
looks like the spacing is different here according to GH
There was a problem hiding this comment.
Not sure why. My copy looks lined up. Have deleted all indents and reset it with tabs
|
Hi, I've done all the changes requested above. You had already got the panOptions variable check in doPan. |
|
The indents in panIndexScale() don't match up with the rest. |
|
Look good! |
Hi,
We recently messaged on slack.
Please consider these changes for the implementation of the category panning.
And please excuse the huge number of commits and the somewhat colourful messages.
Like I said in slack I am new to git, but now realize why you should think about your commit statements!