Skip to content

Conversation

@dlbeswick
Copy link

Hi, I added the ability to scroll the chart by clicking and dragging, or by dragging on a touchscreen. I did this to aid usability of the Gantt chart on iPads and other touch devices. Hope you find it useful.

David

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Considering this project has a jQuery dependency can we just use $(element).on("mousewheel",function() ..., as that would handle the event fallbacks?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure thing, I'll make the change and test it.

On 31 July 2013 15:46, Tait Brown [email protected] wrote:

In js/jquery.fn.gantt.js:

@@ -338,19 +341,38 @@
dataPanel: function (element, width) {
var dataPanel = $('

');

  •            // Handle mousewheel events for scrolling the data panel
    
  •            var mousewheelevt = (/Firefox/i.test(navigator.userAgent)) ? "DOMMouseScroll" : "mousewheel";
    
  •            if (document.attachEvent) {
    
  •                element.attachEvent("on" + mousewheelevt, function (e) { core.wheelScroll(element, e); });
    
  •            } else if (document.addEventListener) {
    
  •                element.addEventListener(mousewheelevt, function (e) { core.wheelScroll(element, e); }, false);
    
  •            if (settings.scrollOnWheel == true) {
    
  •                // Handle mousewheel events for scrolling the data panel
    
  •                var mousewheelevt = (/Firefox/i.test(navigator.userAgent)) ? "DOMMouseScroll" : "mousewheel";
    

Considering this project has a jQuery dependency can we just use $(element).on("mousewheel",function()
..., as that would handle the event fallbacks?


Reply to this email directly or view it on GitHubhttps://github.com//pull/83/files#r5496245
.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made a change, but not sure that I've done it properly. Trying to
listen to the "mousewheel" event didn't work on Firefox. The "wheel" event
worked.

The "mousewheel" event is deprecated in favour of "wheel" according to this
page:
https://developer.mozilla.org/en-US/docs/Web/Reference/Events/wheel?redirectlocale=en-US&redirectslug=DOM%2FMozilla_event_reference%2Fwheel

My change may mean that mouse wheel scroll will no longer work on older
browsers. From the googling I did, there is a specific jQuery plugin to
handle cross-browser mousewheel compatibility, but I don't know if you want
to bring in that dependency.

On 31 July 2013 15:48, David Beswick [email protected] wrote:

Sure thing, I'll make the change and test it.

On 31 July 2013 15:46, Tait Brown [email protected] wrote:

In js/jquery.fn.gantt.js:

@@ -338,19 +341,38 @@
dataPanel: function (element, width) {
var dataPanel = $('

');

  •            // Handle mousewheel events for scrolling the data panel
    
  •            var mousewheelevt = (/Firefox/i.test(navigator.userAgent)) ? "DOMMouseScroll" : "mousewheel";
    
  •            if (document.attachEvent) {
    
  •                element.attachEvent("on" + mousewheelevt, function (e) { core.wheelScroll(element, e); });
    
  •            } else if (document.addEventListener) {
    
  •                element.addEventListener(mousewheelevt, function (e) { core.wheelScroll(element, e); }, false);
    
  •            if (settings.scrollOnWheel == true) {
    
  •                // Handle mousewheel events for scrolling the data panel
    
  •                var mousewheelevt = (/Firefox/i.test(navigator.userAgent)) ? "DOMMouseScroll" : "mousewheel";
    

Considering this project has a jQuery dependency can we just use $(element).on("mousewheel",function()
..., as that would handle the event fallbacks?


Reply to this email directly or view it on GitHubhttps://github.com//pull/83/files#r5496245
.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also to be clear, this wheel scrolling code was present in fn.Gantt prior
to my commit.

On 31 July 2013 16:28, David Beswick [email protected] wrote:

I've made a change, but not sure that I've done it properly. Trying to
listen to the "mousewheel" event didn't work on Firefox. The "wheel" event
worked.

The "mousewheel" event is deprecated in favour of "wheel" according to
this page:
https://developer.mozilla.org/en-US/docs/Web/Reference/Events/wheel?redirectlocale=en-US&redirectslug=DOM%2FMozilla_event_reference%2Fwheel

My change may mean that mouse wheel scroll will no longer work on older
browsers. From the googling I did, there is a specific jQuery plugin to
handle cross-browser mousewheel compatibility, but I don't know if you want
to bring in that dependency.

On 31 July 2013 15:48, David Beswick [email protected] wrote:

Sure thing, I'll make the change and test it.

On 31 July 2013 15:46, Tait Brown [email protected] wrote:

In js/jquery.fn.gantt.js:

@@ -338,19 +341,38 @@
dataPanel: function (element, width) {
var dataPanel = $('

');

  •            // Handle mousewheel events for scrolling the data panel
    
  •            var mousewheelevt = (/Firefox/i.test(navigator.userAgent)) ? "DOMMouseScroll" : "mousewheel";
    
  •            if (document.attachEvent) {
    
  •                element.attachEvent("on" + mousewheelevt, function (e) { core.wheelScroll(element, e); });
    
  •            } else if (document.addEventListener) {
    
  •                element.addEventListener(mousewheelevt, function (e) { core.wheelScroll(element, e); }, false);
    
  •            if (settings.scrollOnWheel == true) {
    
  •                // Handle mousewheel events for scrolling the data panel
    
  •                var mousewheelevt = (/Firefox/i.test(navigator.userAgent)) ? "DOMMouseScroll" : "mousewheel";
    

Considering this project has a jQuery dependency can we just use $(element).on("mousewheel",function()
..., as that would handle the event fallbacks?


Reply to this email directly or view it on GitHubhttps://github.com//pull/83/files#r5496245
.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahhh of course, I read the Git Diff wrong, but it would still be nice to clean up any code cruft (there are mountains of the stuff).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have to test during lunch/later tonight.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are still quite a few older (but still globally-popular) browsers that don't support the 'wheel' event, which is why the check was in there, even if it wasn't quite correct. I suggest something like this (adapted from MDN):

var wheel = core.wheel = 'onwheel' in element ? 'wheel' : document.onmousewheel !== undefined ? 'mousewheel' : 'DOMMouseScroll';
$(element).on(wheel, function (e) { core.wheelScroll(element, e); });

@usmonster
Copy link
Collaborator

It would be cool if dragging worked vertically as well, for paging. Even better would be a "Google Maps" mode, where the whole thing could be dragged nice and smoothly in all directions, though large charts might have to be lazy-loaded (like map tiles in GMaps).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for == true.

@dlbeswick
Copy link
Author

Thanks for taking the time to give feedback, guys. I'll try and incorporate your recommendations in the next few days.

@dlbeswick
Copy link
Author

I've incorporated usmonster's change requests. I've only tested in Firefox so far.

@dlbeswick
Copy link
Author

Touch drag changes work in Safari on iPad.

@AWilco
Copy link

AWilco commented Nov 29, 2013

I've merged this PR with the current master, and there are a few conflicts, but this PR still works. I can offer the newly merged version in another PR if you would like?

@dlbeswick
Copy link
Author

Sounds great Alex, thanks.

@usmonster usmonster added this to the n+2 milestone Feb 24, 2018
@dlbeswick dlbeswick closed this Jun 20, 2021
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.

5 participants