Skip to content

Commit

Permalink
Convert all shortcuts to act on keydown already
Browse files Browse the repository at this point in the history
Contrary to mouse clicks, when pressing keys on a keyboard the
standard behavior is to perform the associated action immediately,
not only when releasing the key again. This should also improve the
perceived performance slightly.

Note that the 'D' shortcut had formerly been handled by Leaflet, which
we now have to do on our own.

While at it, move the character codes over to the options variable, as
found in other parts of the codebase already.

Also removing the listener from the container does not seem needed
anymore nowadays.
  • Loading branch information
rkflx committed May 26, 2020
1 parent 38586fa commit 36d8a20
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 14 deletions.
12 changes: 5 additions & 7 deletions js/plugin/POIMarkers.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,9 @@ BR.PoiMarkers = L.Control.extend({
self.draw(false);
});

var container = new L.DomUtil.create('div');
// keys not working when map container does not have focus, use document instead
L.DomEvent.removeListener(container, 'keyup', this._keyupListener);
L.DomEvent.addListener(document, 'keyup', this._keyupListener, this);
L.DomEvent.addListener(document, 'keydown', this._keydownListener, this);

var container = new L.DomUtil.create('div');
return container;
},

Expand All @@ -62,14 +60,14 @@ BR.PoiMarkers = L.Control.extend({
}
},

_keyupListener: function(e) {
_keydownListener: function(e) {
// Suppress shortcut handling when a text input field is focussed
if (document.activeElement.type == 'text' || document.activeElement.type == 'textarea') {
return;
}
if (e.keyCode === this.options.shortcut.draw.disable) {
if (e.keyCode === this.options.shortcut.draw.disable && !e.repeat) {
this.draw(false);
} else if (e.keyCode === this.options.shortcut.draw.enable) {
} else if (e.keyCode === this.options.shortcut.draw.enable && !e.repeat) {
this.draw(true);
}
},
Expand Down
26 changes: 19 additions & 7 deletions js/plugin/Routing.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,12 @@ BR.Routing = L.Routing.extend({
textFunction: function(distance) {
return distance / 1000;
}
},
shortcut: {
draw: {
enable: 68, // char code for 'd'
disable: 27 // char code for 'ESC'
}
}
},

Expand Down Expand Up @@ -163,8 +169,7 @@ BR.Routing = L.Routing.extend({
this._draw
);

// keys not working when map container does not have focus, use document instead
L.DomEvent.removeListener(this._container, 'keyup', this._keyupListener);
L.DomEvent.addListener(document, 'keydown', this._keydownListener, this);
L.DomEvent.addListener(document, 'keyup', this._keyupListener, this);

// enable drawing mode
Expand Down Expand Up @@ -336,16 +341,23 @@ BR.Routing = L.Routing.extend({
return segments;
},

_keyupListener: function(e) {
_keydownListener: function(e) {
// Suppress shortcut handling when a text input field is focussed
if (document.activeElement.type == 'text' || document.activeElement.type == 'textarea') {
return;
}
// add 'esc' to disable drawing
if (e.keyCode === 27) {
if (e.keyCode === this.options.shortcut.draw.disable && !e.repeat) {
this._draw.disable();
} else {
L.Routing.prototype._keyupListener.call(this, e);
} else if (e.keyCode === this.options.shortcut.draw.enable && !e.repeat) {
this._draw.enable();
}
},

_keyupListener: function(e) {
// Prevent Leaflet from triggering drawing a second time on keyup,
// since this is already done in _keydownListener
if (e.keyCode === this.options.shortcut.draw.enable) {
return;
}
},

Expand Down

0 comments on commit 36d8a20

Please sign in to comment.