Skip to content

Mouse and touch drag can be used to scroll the chart. #83

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

Mouse and touch drag can be used to scroll the chart. #83

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
690844c
Mouse and touch drag can be used to scroll the chart.
dlbeswick Jul 31, 2013
9f51941
Mouse wheel events are handled via standardised browser 'wheel' event…
dlbeswick Jul 31, 2013
767d4a0
Fixed Javascript error when releasing mouse over Gantt item.
dlbeswick Jul 31, 2013
c64dc7f
Further fixes due to error when releasing mouse over Gantt item.
dlbeswick Jul 31, 2013
2085862
Improved smootheness of scrolling. Incorporated change requests from …
dlbeswick Aug 14, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 56 additions & 9 deletions js/jquery.fn.gantt.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,9 @@
onItemClick: function (data) { return; },
onAddClick: function (data) { return; },
onRender: function() { return; },
scrollToToday: true
scrollToToday: true,
scrollOnWheel: true,
scrollOnDrag: false
};

// custom selector `:findday` used to match on specified day in ms.
Expand Down Expand Up @@ -223,6 +225,7 @@
element.dateStart = tools.getMinDate(element);
element.dateEnd = tools.getMaxDate(element);

element.dataPanelIgnoreNextClick = false;

/* core.render(element); */
core.waitToggle(element, true, function () { core.render(element); });
Expand Down Expand Up @@ -338,19 +341,38 @@
dataPanel: function (element, width) {
var dataPanel = $('<div class="dataPanel" style="width: ' + width + 'px;"/>');

// 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) {
Copy link
Collaborator

@usmonster usmonster Aug 2, 2013

Choose a reason for hiding this comment

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

No need for == true.

// Handle mousewheel events for scrolling the data panel
var mousewheelevt = (/Firefox/i.test(navigator.userAgent)) ? "DOMMouseScroll" : "mousewheel";
Copy link
Owner

@taitems taitems Jul 31, 2013

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

@dlbeswick dlbeswick Jul 31, 2013

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

@dlbeswick dlbeswick Jul 31, 2013

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

@dlbeswick dlbeswick Jul 31, 2013

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

@taitems taitems Jul 31, 2013

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

@taitems taitems Jul 31, 2013

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.

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.scrollOnDrag == true) {
Copy link
Collaborator

@usmonster usmonster Aug 1, 2013

Choose a reason for hiding this comment

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

No need for == true.

dataPanel.on('touchstart', function (e) { core.dragDataPanelStart(element, e.originalEvent.changedTouches[0].screenX, e); });
dataPanel.on('touchmove', function (e) { core.dragDataPanelMove(element, e.originalEvent.changedTouches[0].screenX, e); });
dataPanel.on('touchend', function (e) { core.dragDataPanelStop(element); });

dataPanel.mousedown(function (e) { core.dragDataPanelStart(element, e.screenX, e); });
dataPanel.mousemove(function (e) { core.dragDataPanelMove(element, e.screenX, e); });
Copy link
Collaborator

@usmonster usmonster Aug 1, 2013

Choose a reason for hiding this comment

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

mousemove listeners are expensive. The event fires all the dang time. Maybe only bind the handler after (or inside) a dragPanelStart and then unbind it again it after (or inside) a dragPanelStop.

dataPanel.mouseout(function (e) { core.dragDataPanelStop(element); });
dataPanel.mouseup(function (e) { core.dragDataPanelStop(element); });
}

Copy link
Collaborator

@usmonster usmonster Aug 1, 2013

Choose a reason for hiding this comment

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

Maybe a matter of code style, but I would rewrite these to look slightly cleaner, e.g.:

dataPanel.on('mousedown touchstart', function (e) {
    var screenX = e.type.indexOf('mouse') === 0 ? e.screenX : e.originalEvent.changedTouches[0].screenX;
    core.dragDataPanelStart(element, screenX, e);
});
dataPanel.on('mouseup mouseout touchend', function (e) { core.dragDataPanelStop(element); });

// Handle click events and dispatch to registered `onAddClick`
// function
dataPanel.click(function (e) {

e.stopPropagation();

if (element.dataPanelIgnoreNextClick == true) {
Copy link
Collaborator

@usmonster usmonster Aug 1, 2013

Choose a reason for hiding this comment

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

No need for the == true part.

element.dataPanelIgnoreNextClick = false;
return dataPanel;
Copy link
Collaborator

@usmonster usmonster Aug 1, 2013

Choose a reason for hiding this comment

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

You might also want an e.PreventDefault() before you return.

Copy link
Collaborator

@usmonster usmonster Aug 1, 2013

Choose a reason for hiding this comment

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

Or, actually, just change the line to return false; since you shouldn't be returning the dataPanel in this event handler anyway.

}

var corrX, corrY;
var leftpanel = $(element).find(".fn-gantt .leftPanel");
var datapanel = $(element).find(".fn-gantt .dataPanel");
Expand Down Expand Up @@ -960,7 +982,12 @@
}
bar.click(function (e) {
e.stopPropagation();
settings.onItemClick($(this).data("dataObj"));

if (element.dataPanelIgnoreNextClick == true) {
Copy link
Collaborator

@usmonster usmonster Aug 1, 2013

Choose a reason for hiding this comment

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

Again, no need for == true.

element.dataPanelIgnoreNextClick = false;
} else {
settings.onItemClick($(this).data("dataObj"));
}
});
return bar;
},
Expand Down Expand Up @@ -1431,6 +1458,25 @@
}
element.loader = null;
}
},

dragDataPanelStart: function (element, screenX, e) {
element.scrollNavigation.dragStartX = screenX;
},

dragDataPanelMove: function (element, screenX, e) {
if (element.scrollNavigation.dragStartX != undefined) {
Copy link
Collaborator

@usmonster usmonster Aug 1, 2013

Choose a reason for hiding this comment

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

Should use !== here, or typeof element.scrollNavigation.dragStartX !== 'undefined'. (To be on the safe side in older browsers, we should probably also add an undefined parameter to the top of the IIFE.)

core.scrollPanel(element, screenX - element.scrollNavigation.dragStartX)
element.scrollNavigation.dragStartX = screenX

// Stop the click event from being generated on mouse up when the drag ends.
element.dataPanelIgnoreNextClick = true;
e.preventDefault();
}
},

dragDataPanelStop: function (element) {
element.scrollNavigation.dragStartX = undefined;
}
};

Expand Down Expand Up @@ -1728,7 +1774,8 @@
panelMargin: 0,
repositionDelay: 0,
panelMaxPos: 0,
canScroll: true
canScroll: true,
dragStartX: undefined
};

this.gantt = null;
Expand Down
118 changes: 118 additions & 0 deletions tests/test02.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
<!doctype html>
<html lang="en-au">
<head>
<title>jQuery.Gantt - Test Suite 02</title>
<meta charset="utf-8">
<meta http-equiv="X-UA-Compatible" content="IE=Edge;chrome=1" >
<link rel="stylesheet" href="../css/style.css" />
<link href="http://netdna.bootstrapcdn.com/twitter-bootstrap/2.3.2/css/bootstrap-combined.min.css" rel="stylesheet">
<link rel="stylesheet" href="http://taitems.github.com/UX-Lab/core/css/prettify.css" />
<style type="text/css">
body {
font-family: Helvetica, Arial, sans-serif;
font-size: 13px;
padding: 0 0 50px 0;
}
.contain {
width: 800px;
margin: 0 auto;
}
h1 {
margin: 40px 0 20px 0;
}
h2 {
font-size: 1.5em;
padding-bottom: 3px;
border-bottom: 1px solid #DDD;
margin-top: 50px;
margin-bottom: 25px;
}
table th:first-child {
width: 150px;
}
</style>
</head>
<body>

<div class="contain">

<h1>
jQuery.Gantt
<small>&mdash; Test Suite 02</small>
</h1>

<p>
<strong>Expected behaviour:</strong>
<ul>
<li>Chart can be scrolled by dragging empty space in data panel.</li>
<li>Touch screen clients can scroll with drag touches.</li>
<li>Releasing the mouse over a Gantt item after a drag completes doesn't trigger the onItemClick event.</li>
<li>Releasing the mouse over an empty spot on the data panel after a drag completes doesn't trigger the onAddClick event.</li>
<li>Using the scroll wheel while mouse is over the data panel doesn't scroll the data panel.</li>
</ul>
</p>

<p>
<strong>Manual validation:</strong>
<ul>
<li>Passing</li>
</ul>
</p>

<div class="gantt"></div>


</body>
<script src="../js/jquery.min.js"></script>
<script src="../js/jquery.fn.gantt.js"></script>
<script src="moment.min.js"></script>
<script src="http://netdna.bootstrapcdn.com/twitter-bootstrap/2.3.2/js/bootstrap.min.js"></script>
<script src="http://taitems.github.com/UX-Lab/core/js/prettify.js"></script>
<script>

$(function() {

"use strict";

var today = moment();
var future = moment().add("hours",200);

var today_friendly = "/Date(" + today.valueOf() + ")/";
var next_friendly = "/Date(" + future.valueOf() + ")/";
var click_date_friendly = "/Date(" + today.add("hours", -10).valueOf() + ")/";

$(".gantt").gantt({
source: [
{
name: "Testing",
desc: " ",
values: [{
from: today_friendly,
to: next_friendly,
label: "Test",
customClass: "ganttRed"
}]
},
{
name: "Test click release",
desc: " ",
values: [{
from: click_date_friendly,
to: next_friendly,
label: "Test2",
customClass: "ganttRed"
}]
}
],
scale: "hours",
minScale: "hours",
navigate: "scroll",
scrollOnDrag: true,
scrollOnWheel: false,
onItemClick: function() { alert('Gantt item clicked.'); },
onAddClick: function() { alert('Data panel clicked.'); }
});
});

</script>
</html>