Skip to content

Commit

Permalink
Unbind global listeners after the map is removed (patch) (#1434)
Browse files Browse the repository at this point in the history
* fix global keyboard and window listeners are not removed after the map is destroyed

* test: add a test case for keyboard mixin

* change `map.on` to `map.once` & extract keyboard event listener unbind function

Co-authored-by: Florian Bischof <[email protected]>

* fix missing dot operator

* tweak the mixins test case

Co-authored-by: Florian Bischof <[email protected]>

* Fix test naming

---------

Co-authored-by: Florian Bischof <[email protected]>
  • Loading branch information
plainheart and Falke-Design authored Feb 4, 2024
1 parent 9994568 commit 77b6545
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 5 deletions.
25 changes: 25 additions & 0 deletions cypress/integration/mixins.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
describe('KeyboardMixin', () => {
it('Should unbind event listeners that bound by the KeyboardMixin after the map is destroyed', () => {
cy.window().then((window) => {
const { map, document } = window;

map.remove();

const isWindowBlurEventUnbound = !Object.entries(
window._leaflet_events
).some(([name, handler]) => name.startsWith('blur') && handler);
expect(
isWindowBlurEventUnbound,
'window blur event listener is not unbound'
).to.eq(true);

const isKeyUpDownEventUnbound = !Object.entries(
document._leaflet_events
).some(([name, handler]) => name.startsWith('key') && handler);
expect(
isKeyUpDownEventUnbound,
'document keyboard event listener is not unbound'
).to.eq(true);
});
});
});
4 changes: 2 additions & 2 deletions src/js/L.PM.Map.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import GlobalDragMode from './Mixins/Modes/Mode.Drag';
import GlobalRemovalMode from './Mixins/Modes/Mode.Removal';
import GlobalRotateMode from './Mixins/Modes/Mode.Rotate';
import EventMixin from './Mixins/Events';
import KeyboardMixins from './Mixins/Keyboard';
import createKeyboardMixins from './Mixins/Keyboard';
import { getRenderer } from './helpers';

const Map = L.Class.extend({
Expand All @@ -20,7 +20,7 @@ const Map = L.Class.extend({
this.map = map;
this.Draw = new L.PM.Draw(map);
this.Toolbar = new L.PM.Toolbar(map);
this.Keyboard = KeyboardMixins;
this.Keyboard = createKeyboardMixins();

this.globalOptions = {
snappable: true,
Expand Down
14 changes: 11 additions & 3 deletions src/js/Mixins/Keyboard.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,17 @@
const KeyboardMixins = {
// use function to create a new mixin object for keeping isolation
// to make it work for multiple map instances
const createKeyboardMixins = () => ({
_lastEvents: { keydown: undefined, keyup: undefined, current: undefined },
_initKeyListener(map) {
this.map = map;
L.DomEvent.on(document, 'keydown keyup', this._onKeyListener, this);
L.DomEvent.on(window, 'blur', this._onBlur, this);
// clean up global listeners when current map instance is destroyed
map.once('unload', this._unbindKeyListenerEvents, this);
},
_unbindKeyListenerEvents() {
L.DomEvent.off(document, 'keydown keyup', this._onKeyListener, this);
L.DomEvent.off(window, 'blur', this._onBlur, this);
},
_onKeyListener(e) {
let focusOn = 'document';
Expand Down Expand Up @@ -44,6 +52,6 @@ const KeyboardMixins = {
getPressedKey() {
return this._lastEvents.current?.event.key;
},
};
});

export default KeyboardMixins;
export default createKeyboardMixins;

0 comments on commit 77b6545

Please sign in to comment.