Added optional modifier keys

[BarryPSmith]

Often the chart captures scroll events in an undesired way, so this PR makes it possible to set an optional wheelModifierKey that must be pressed to activate mouse wheel zoom.

A modifierKey can also be set for pan. By default this is only checked when the input comes via mouse, but can be set for touch and other inputs with requireModifierNonMouse: true

zoom.onWheelModifierKeyFailed is fired when a zoom does not occur because the modifier key is not present (e.g. to allow the consumer to display "Press ctrl to zoom"). A similar pan.onModifierKeyFailed is also available.

Although not necessarily the requested fix, it will help with the following issues:
#377, #370, #335, #308 (Actually I see the collaborators offer to accept PRs on this issue there). Not looking any further...

Relationship to other pull requests:
Might be incompatible with #380.
A more flexible version of #368

[BarryPSmith]

@jledentu I see on issue #308 you suggest the name modifierKey for both the pan and zoom modifiers. I have used 'wheelModifierKey' for the zoom key in this PR because it only affects wheel zoom, but it would be an easy change to have them both 'modifierKey'.

[benmccann]

I think a better way to handle this might be a generic function that let's you filter whether the zoom occurs as proposed in #329. That would allow you to handle more use cases

Or maybe you can actually handle this case already today with an external listener that enables the plugin when the modifier key is pressed and disables the plugin when the modifier key is released and all we need is an example of doing that

[NicolasGoeddel]

Okay, why the heck do you delete comments?

[benmccann]

The comments are meant for code review. If you want to express interest you can simply leave a thumbs up on the PR

[kurkle]

@BarryPSmith would you be up to rebasing this?

[BarryPSmith]

@benmccann I think that this and #329 are intended to solve different problems. I do not see any parameters in that PR related to keys or the type of input that is causing the zoom or pan event.

Or maybe you can actually handle this case already today with an external listener that enables the plugin when the modifier key is pressed and disables the plugin when the modifier key is released and all we need is an example of doing that

Perhaps I overestimate the complexity of the situation. I would be interested to see the robust example.

@kurkle Is there interest in merging this then? What else needs to be done for this to be merged?

[kurkle]

@BarryPSmith yes, I think something like this is needed. I'm not sure there is a use case for the non-mouse modifier, that we meght want to leave out - unless there is a use case. Maybe some nits, I did not do a full review yet.

[BarryPSmith]

It looks like enough has changed that it'll be easier to rewrite the PR. That's the impression I get looking at the conflict files that rebase has generated. At least it's a fairly simple set of changes to the functional code.

I'll throw away the zoom tests, given the modified test/utils.js has migrated into a different repo.

At least the weather is bad here so I have time.

[kurkle]

some lint issues, npm run lint should reveal them locally.

[BarryPSmith]

Test coverage decrease: In order to test the new code one must be able to send mouse events with keys attached. That is not possible with the current test framework - util.js of chartjs-test-utils must be updated. See test/util.js of d5cb18f

[kurkle]

Test coverage decrease: In order to test the new code one must be able to send mouse events with keys attached. That is not possible with the current test framework - util.js of chartjs-test-utils must be updated. See test/util.js of d5cb18f

Ok, I'll take a look at the tests later. I think it needs to be a WheelEvent for the delta stuff.

[kurkle]

Overall looks good. I'm not sure about the event names, is there some reasoning that I missed?

[kurkle]

Thanks @BarryPSmith