From 7852bb0cad37f4fe62a9fdad874621f32ee45a69 Mon Sep 17 00:00:00 2001 From: plainheart Date: Tue, 2 Jan 2024 23:43:50 +0800 Subject: [PATCH 1/6] fix global keyboard and window listeners are not removed after the map is destroyed --- src/js/L.PM.Map.js | 4 ++-- src/js/Mixins/Keyboard.js | 13 ++++++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/js/L.PM.Map.js b/src/js/L.PM.Map.js index 4566f2d9..fe76cbcb 100644 --- a/src/js/L.PM.Map.js +++ b/src/js/L.PM.Map.js @@ -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({ @@ -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, diff --git a/src/js/Mixins/Keyboard.js b/src/js/Mixins/Keyboard.js index 2908dfea..47099936 100644 --- a/src/js/Mixins/Keyboard.js +++ b/src/js/Mixins/Keyboard.js @@ -1,9 +1,16 @@ -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.on('unload', () => { + L.DomEvent.off(document, 'keydown keyup', this._onKeyListener, this); + L.DomEvent.off(window, 'blur', this._onBlur, this); + }); }, _onKeyListener(e) { let focusOn = 'document'; @@ -44,6 +51,6 @@ const KeyboardMixins = { getPressedKey() { return this._lastEvents.current?.event.key; }, -}; +}); -export default KeyboardMixins; +export default createKeyboardMixins; From f725b3916bd13565e719d95e8f787001a299d5b1 Mon Sep 17 00:00:00 2001 From: plainheart Date: Sun, 4 Feb 2024 18:56:20 +0800 Subject: [PATCH 2/6] test: add a test case for keyboard mixin --- cypress/integration/mixins.spec.js | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) create mode 100644 cypress/integration/mixins.spec.js diff --git a/cypress/integration/mixins.spec.js b/cypress/integration/mixins.spec.js new file mode 100644 index 00000000..81f5e5e9 --- /dev/null +++ b/cypress/integration/mixins.spec.js @@ -0,0 +1,25 @@ +describe('Should unbind event listeners that bound by the Mixins after the map is destroyed', () => { + it('KeyboardMixin', () => { + 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); + }); + }); +}); From 7ac3dd575bd67b5c1a4ce26e158576691c11f4a9 Mon Sep 17 00:00:00 2001 From: Zhongxiang Wang Date: Sun, 4 Feb 2024 23:43:25 +0800 Subject: [PATCH 3/6] change `map.on` to `map.once` & extract keyboard event listener unbind function Co-authored-by: Florian Bischof --- src/js/Mixins/Keyboard.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/js/Mixins/Keyboard.js b/src/js/Mixins/Keyboard.js index 47099936..e04c525d 100644 --- a/src/js/Mixins/Keyboard.js +++ b/src/js/Mixins/Keyboard.js @@ -7,10 +7,11 @@ const createKeyboardMixins = () => ({ 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.on('unload', () => { + 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'; From c3c1c84d73d1ecee3f95b64f073fe6acf7f816aa Mon Sep 17 00:00:00 2001 From: Zhongxiang Wang Date: Sun, 4 Feb 2024 23:56:22 +0800 Subject: [PATCH 4/6] fix missing dot operator --- src/js/Mixins/Keyboard.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/js/Mixins/Keyboard.js b/src/js/Mixins/Keyboard.js index e04c525d..ff2c5f16 100644 --- a/src/js/Mixins/Keyboard.js +++ b/src/js/Mixins/Keyboard.js @@ -7,7 +7,7 @@ const createKeyboardMixins = () => ({ 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); + map.once('unload', this._unbindKeyListenerEvents, this); }, _unbindKeyListenerEvents() { L.DomEvent.off(document, 'keydown keyup', this._onKeyListener, this); From 3134ab84b176ae616603864e805be55fd8dc7e81 Mon Sep 17 00:00:00 2001 From: Zhongxiang Wang Date: Sun, 4 Feb 2024 23:59:27 +0800 Subject: [PATCH 5/6] tweak the mixins test case Co-authored-by: Florian Bischof --- cypress/integration/mixins.spec.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cypress/integration/mixins.spec.js b/cypress/integration/mixins.spec.js index 81f5e5e9..41a7b5ff 100644 --- a/cypress/integration/mixins.spec.js +++ b/cypress/integration/mixins.spec.js @@ -1,5 +1,5 @@ -describe('Should unbind event listeners that bound by the Mixins after the map is destroyed', () => { - it('KeyboardMixin', () => { +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; From 598e2f94751702198da84f292a1589277ae53e84 Mon Sep 17 00:00:00 2001 From: Florian Bischof Date: Sun, 4 Feb 2024 17:02:46 +0100 Subject: [PATCH 6/6] Fix test naming --- cypress/integration/mixins.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cypress/integration/mixins.spec.js b/cypress/integration/mixins.spec.js index 41a7b5ff..e2a49564 100644 --- a/cypress/integration/mixins.spec.js +++ b/cypress/integration/mixins.spec.js @@ -1,4 +1,4 @@ -describe('', (KeyboardMixin) => { +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;