From 5acaa423c97c536d1ea5ea38e8c6c53d305d1325 Mon Sep 17 00:00:00 2001 From: Andrew Harvey Date: Tue, 19 Jun 2018 23:19:27 +1000 Subject: [PATCH 01/16] open popup on a marker from keyboard --- src/ui/marker.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/ui/marker.js b/src/ui/marker.js index 3204e4eb0c4..ca83e639041 100644 --- a/src/ui/marker.js +++ b/src/ui/marker.js @@ -312,6 +312,14 @@ export default class Marker extends Evented { } this._popup = popup; if (this._lngLat) this._popup.setLngLat(this._lngLat); + + this._element.addEventListener('keypress', ((e) => { + const code = e.charCode || e.keyCode; + + if ((code === 32) || (code === 13)) { // space or enter + this.togglePopup(); + } + }).bind(this)); } return this; From 2f2fc994f047aa82e432cc2006dc78931303cd33 Mon Sep 17 00:00:00 2001 From: Andrew Harvey Date: Wed, 20 Jun 2018 21:45:52 +1000 Subject: [PATCH 02/16] arrow functions don't need to bind this --- src/ui/marker.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ui/marker.js b/src/ui/marker.js index ca83e639041..f3c4ff2638a 100644 --- a/src/ui/marker.js +++ b/src/ui/marker.js @@ -313,13 +313,13 @@ export default class Marker extends Evented { this._popup = popup; if (this._lngLat) this._popup.setLngLat(this._lngLat); - this._element.addEventListener('keypress', ((e) => { + this._element.addEventListener('keypress', (e) => { const code = e.charCode || e.keyCode; if ((code === 32) || (code === 13)) { // space or enter this.togglePopup(); } - }).bind(this)); + }); } return this; From a3907a666cf51eee43aeaa9347346910c635c8a4 Mon Sep 17 00:00:00 2001 From: Andrew Harvey Date: Wed, 20 Jun 2018 22:12:44 +1000 Subject: [PATCH 03/16] set tabindex on marker elements with popups and remove keypress listener --- src/ui/marker.js | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/ui/marker.js b/src/ui/marker.js index f3c4ff2638a..88d59f3c5b1 100644 --- a/src/ui/marker.js +++ b/src/ui/marker.js @@ -289,9 +289,19 @@ export default class Marker extends Evented { * @returns {Marker} `this` */ setPopup(popup: ?Popup) { + const keypressListener = (e) => { + const code = e.charCode || e.keyCode; + + if ((code === 32) || (code === 13)) { // space or enter + this.togglePopup(); + } + }; + if (this._popup) { this._popup.remove(); this._popup = null; + this._element.removeEventListener('keypress', keypressListener); + this._element.removeAttribute('tabindex'); } if (popup) { @@ -313,13 +323,8 @@ export default class Marker extends Evented { this._popup = popup; if (this._lngLat) this._popup.setLngLat(this._lngLat); - this._element.addEventListener('keypress', (e) => { - const code = e.charCode || e.keyCode; - - if ((code === 32) || (code === 13)) { // space or enter - this.togglePopup(); - } - }); + this._element.setAttribute('tabindex', 0); + this._element.addEventListener('keypress', keypressListener); } return this; From 66826bc1a9309ed24463947368c1fcb7cde41448 Mon Sep 17 00:00:00 2001 From: Andrew Harvey Date: Wed, 20 Jun 2018 22:34:12 +1000 Subject: [PATCH 04/16] aria-label on default marker --- src/ui/marker.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ui/marker.js b/src/ui/marker.js index 88d59f3c5b1..f1f3994af2b 100644 --- a/src/ui/marker.js +++ b/src/ui/marker.js @@ -88,6 +88,7 @@ export default class Marker extends Evented { if (!options || !options.element) { this._defaultMarker = true; this._element = DOM.create('div'); + this._element.setAttribute('aria-label', 'Map marker'); // create default map marker SVG const svg = DOM.createNS('http://www.w3.org/2000/svg', 'svg'); From 96fb8711f4c360731e7179aea9bcd792edc063ad Mon Sep 17 00:00:00 2001 From: Andrew Harvey Date: Wed, 20 Jun 2018 22:35:39 +1000 Subject: [PATCH 05/16] tabindex value should be string, thanks flow --- src/ui/marker.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/marker.js b/src/ui/marker.js index f1f3994af2b..16df95c1f8b 100644 --- a/src/ui/marker.js +++ b/src/ui/marker.js @@ -324,7 +324,7 @@ export default class Marker extends Evented { this._popup = popup; if (this._lngLat) this._popup.setLngLat(this._lngLat); - this._element.setAttribute('tabindex', 0); + this._element.setAttribute('tabindex', '0'); this._element.addEventListener('keypress', keypressListener); } From cc3120c4d7bc0c65b8fb0e9f574665895b09ebd1 Mon Sep 17 00:00:00 2001 From: Andrew Harvey Date: Wed, 20 Jun 2018 22:37:36 +1000 Subject: [PATCH 06/16] flow --- src/ui/marker.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/marker.js b/src/ui/marker.js index 16df95c1f8b..8f0b1575d98 100644 --- a/src/ui/marker.js +++ b/src/ui/marker.js @@ -290,7 +290,7 @@ export default class Marker extends Evented { * @returns {Marker} `this` */ setPopup(popup: ?Popup) { - const keypressListener = (e) => { + const keypressListener = (e: Event) => { const code = e.charCode || e.keyCode; if ((code === 32) || (code === 13)) { // space or enter From 7b845c96bc78790ff104d46803eb919586f215cf Mon Sep 17 00:00:00 2001 From: Andrew Harvey Date: Wed, 20 Jun 2018 22:48:28 +1000 Subject: [PATCH 07/16] flow --- src/ui/marker.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/marker.js b/src/ui/marker.js index 8f0b1575d98..a61ad7f22dc 100644 --- a/src/ui/marker.js +++ b/src/ui/marker.js @@ -290,7 +290,7 @@ export default class Marker extends Evented { * @returns {Marker} `this` */ setPopup(popup: ?Popup) { - const keypressListener = (e: Event) => { + const keypressListener = (e: KeyboardEvent) => { const code = e.charCode || e.keyCode; if ((code === 32) || (code === 13)) { // space or enter From 52793b965e90d25975461a4951d43ce664711fd1 Mon Sep 17 00:00:00 2001 From: Andrew Harvey Date: Sun, 21 Jul 2019 13:05:47 +1000 Subject: [PATCH 08/16] KeyboardEvent.charCode and keyCode are deprecated, support standard KeyboardEvent.code --- src/ui/marker.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/ui/marker.js b/src/ui/marker.js index a61ad7f22dc..928cbb7f334 100644 --- a/src/ui/marker.js +++ b/src/ui/marker.js @@ -291,9 +291,13 @@ export default class Marker extends Evented { */ setPopup(popup: ?Popup) { const keypressListener = (e: KeyboardEvent) => { - const code = e.charCode || e.keyCode; + const code = e.code; + const legacyCode = e.charCode || e.keyCode; - if ((code === 32) || (code === 13)) { // space or enter + if ( + (code === 'Space') || (code === 'Enter') || + (legacyCode === 32) || (legacyCode === 13) // space or enter + ) { this.togglePopup(); } }; From 88f6ae1e17fe50644dbd525a75d6dab293883aed Mon Sep 17 00:00:00 2001 From: Andrew Harvey Date: Sun, 21 Jul 2019 13:06:11 +1000 Subject: [PATCH 09/16] add unit test for opening popup on Enter --- test/unit/ui/marker.test.js | 19 +++++++++++++++++++ test/util/simulate_interaction.js | 6 ++++++ 2 files changed, 25 insertions(+) diff --git a/test/unit/ui/marker.test.js b/test/unit/ui/marker.test.js index 620411e7f34..51c2675ff37 100644 --- a/test/unit/ui/marker.test.js +++ b/test/unit/ui/marker.test.js @@ -120,6 +120,25 @@ test('Marker#togglePopup closes a popup that was open', (t) => { t.end(); }); +test('Enter key on Marker opens a popup that was closed', (t) => { + const map = createMap(t); + const marker = new Marker() + .setLngLat([0, 0]) + .addTo(map) + .setPopup(new Popup()) + + // popup not initially open + t.notOk(marker.getPopup().isOpen()); + + simulate.keypress(marker.getElement(), { code: 'Enter' }); + + // popup open after Enter keypress + t.ok(marker.getPopup().isOpen()); + + map.remove(); + t.end(); +}); + test('Marker anchor defaults to center', (t) => { const map = createMap(t); const marker = new Marker() diff --git a/test/util/simulate_interaction.js b/test/util/simulate_interaction.js index b61602403df..7889621c0e1 100644 --- a/test/util/simulate_interaction.js +++ b/test/util/simulate_interaction.js @@ -40,6 +40,12 @@ events.dblclick = function (target, options) { target.dispatchEvent(new MouseEvent('dblclick', options)); }; +events.keypress = function (target, options) { + options = Object.assign({bubbles: true}, options); + const KeyboardEvent = window(target).KeyboardEvent; + target.dispatchEvent(new KeyboardEvent('keypress', options)); +}; + [ 'mouseup', 'mousedown', 'mouseover', 'mousemove', 'mouseout' ].forEach((event) => { events[event] = function (target, options) { options = Object.assign({bubbles: true}, options); From 2b842743a3476cbb205d1667dab40999f4760358 Mon Sep 17 00:00:00 2001 From: Andrew Harvey Date: Sun, 21 Jul 2019 13:07:11 +1000 Subject: [PATCH 10/16] add unit test for opening popup on Space --- test/unit/ui/marker.test.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/test/unit/ui/marker.test.js b/test/unit/ui/marker.test.js index 51c2675ff37..7962448df08 100644 --- a/test/unit/ui/marker.test.js +++ b/test/unit/ui/marker.test.js @@ -139,6 +139,25 @@ test('Enter key on Marker opens a popup that was closed', (t) => { t.end(); }); +test('Space key on Marker opens a popup that was closed', (t) => { + const map = createMap(t); + const marker = new Marker() + .setLngLat([0, 0]) + .addTo(map) + .setPopup(new Popup()) + + // popup not initially open + t.notOk(marker.getPopup().isOpen()); + + simulate.keypress(marker.getElement(), { code: 'Space' }); + + // popup open after Enter keypress + t.ok(marker.getPopup().isOpen()); + + map.remove(); + t.end(); +}); + test('Marker anchor defaults to center', (t) => { const map = createMap(t); const marker = new Marker() From 5aafd6ec8db731a118c49c4710b1d394fcf287ff Mon Sep 17 00:00:00 2001 From: Andrew Harvey Date: Sun, 21 Jul 2019 13:25:01 +1000 Subject: [PATCH 11/16] retain existing tabindex --- src/ui/marker.js | 10 ++++++++-- test/unit/ui/marker.test.js | 27 +++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/src/ui/marker.js b/src/ui/marker.js index 928cbb7f334..8733ac3990f 100644 --- a/src/ui/marker.js +++ b/src/ui/marker.js @@ -306,7 +306,10 @@ export default class Marker extends Evented { this._popup.remove(); this._popup = null; this._element.removeEventListener('keypress', keypressListener); - this._element.removeAttribute('tabindex'); + + if (!this._originalTabIndex) { + this._element.removeAttribute('tabindex'); + } } if (popup) { @@ -328,7 +331,10 @@ export default class Marker extends Evented { this._popup = popup; if (this._lngLat) this._popup.setLngLat(this._lngLat); - this._element.setAttribute('tabindex', '0'); + this._originalTabIndex = this._element.getAttribute('tabindex'); + if (!this._originalTabIndex) { + this._element.setAttribute('tabindex', '0'); + } this._element.addEventListener('keypress', keypressListener); } diff --git a/test/unit/ui/marker.test.js b/test/unit/ui/marker.test.js index 7962448df08..500725f74dc 100644 --- a/test/unit/ui/marker.test.js +++ b/test/unit/ui/marker.test.js @@ -158,6 +158,33 @@ test('Space key on Marker opens a popup that was closed', (t) => { t.end(); }); +test('Marker#setPopup sets a tabindex', (t) => { + const popup = new Popup(); + const marker = new Marker() + .setPopup(popup); + t.equal(marker.getElement().getAttribute('tabindex'), "0"); + t.end(); +}); + +test('Marker#setPopup removes tabindex when unset', (t) => { + const popup = new Popup(); + const marker = new Marker() + .setPopup(popup) + .setPopup(); + t.notOk(marker.getElement().getAttribute('tabindex')); + t.end(); +}); + +test('Marker#setPopup does not replace existing tabindex', (t) => { + const element = window.document.createElement('div'); + element.setAttribute('tabindex', '5'); + const popup = new Popup(); + const marker = new Marker({element}) + .setPopup(popup); + t.equal(marker.getElement().getAttribute('tabindex'), "5"); + t.end(); +}); + test('Marker anchor defaults to center', (t) => { const map = createMap(t); const marker = new Marker() From 4502ffc1c2fba50d0ebaa8ac3c5ab194085789d7 Mon Sep 17 00:00:00 2001 From: Andrew Harvey Date: Sun, 21 Jul 2019 13:28:28 +1000 Subject: [PATCH 12/16] fix lint --- test/unit/ui/marker.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/ui/marker.test.js b/test/unit/ui/marker.test.js index 500725f74dc..ee44824945a 100644 --- a/test/unit/ui/marker.test.js +++ b/test/unit/ui/marker.test.js @@ -125,7 +125,7 @@ test('Enter key on Marker opens a popup that was closed', (t) => { const marker = new Marker() .setLngLat([0, 0]) .addTo(map) - .setPopup(new Popup()) + .setPopup(new Popup()); // popup not initially open t.notOk(marker.getPopup().isOpen()); @@ -144,7 +144,7 @@ test('Space key on Marker opens a popup that was closed', (t) => { const marker = new Marker() .setLngLat([0, 0]) .addTo(map) - .setPopup(new Popup()) + .setPopup(new Popup()); // popup not initially open t.notOk(marker.getPopup().isOpen()); From 32ead63fd14701f14722ac9b73ba2580617c99f9 Mon Sep 17 00:00:00 2001 From: Andrew Harvey Date: Sun, 21 Jul 2019 13:31:06 +1000 Subject: [PATCH 13/16] fix flow --- src/ui/marker.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ui/marker.js b/src/ui/marker.js index 8733ac3990f..b39029cdb7e 100644 --- a/src/ui/marker.js +++ b/src/ui/marker.js @@ -60,6 +60,7 @@ export default class Marker extends Evented { _rotation: number; _pitchAlignment: string; _rotationAlignment: string; + _originalTabIndex: ?string; // original tabindex of _element constructor(options?: Options, legacyOptions?: Options) { super(); From 000f5a2f960a96864d16f5058b749540140451b6 Mon Sep 17 00:00:00 2001 From: Andrew Harvey Date: Fri, 16 Aug 2019 18:22:28 +1000 Subject: [PATCH 14/16] _onKeyPress method --- src/ui/marker.js | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/src/ui/marker.js b/src/ui/marker.js index b39029cdb7e..5bba98f4423 100644 --- a/src/ui/marker.js +++ b/src/ui/marker.js @@ -75,7 +75,8 @@ export default class Marker extends Evented { '_onMove', '_onUp', '_addDragHandler', - '_onMapClick' + '_onMapClick', + '_onKeyPress' ], this); this._anchor = options && options.anchor || 'center'; @@ -291,22 +292,10 @@ export default class Marker extends Evented { * @returns {Marker} `this` */ setPopup(popup: ?Popup) { - const keypressListener = (e: KeyboardEvent) => { - const code = e.code; - const legacyCode = e.charCode || e.keyCode; - - if ( - (code === 'Space') || (code === 'Enter') || - (legacyCode === 32) || (legacyCode === 13) // space or enter - ) { - this.togglePopup(); - } - }; - if (this._popup) { this._popup.remove(); this._popup = null; - this._element.removeEventListener('keypress', keypressListener); + this._element.removeEventListener('keypress', this._onKeyPress); if (!this._originalTabIndex) { this._element.removeAttribute('tabindex'); @@ -336,12 +325,24 @@ export default class Marker extends Evented { if (!this._originalTabIndex) { this._element.setAttribute('tabindex', '0'); } - this._element.addEventListener('keypress', keypressListener); + this._element.addEventListener('keypress', this._onKeyPress); } return this; } + _onKeyPress(e: KeyboardEvent) { + const code = e.code; + const legacyCode = e.charCode || e.keyCode; + + if ( + (code === 'Space') || (code === 'Enter') || + (legacyCode === 32) || (legacyCode === 13) // space or enter + ) { + this.togglePopup(); + } + } + _onMapClick(e: MapMouseEvent) { const targetElement = e.originalEvent.target; const element = this._element; From dce3b0470f0ef87436e8003c682436b5deaaa86d Mon Sep 17 00:00:00 2001 From: Vladimir Agafonkin Date: Wed, 6 Nov 2019 11:58:52 +0200 Subject: [PATCH 15/16] fix scrolling issue when focusing markers --- src/ui/marker.js | 6 ++++++ test/unit/ui/marker.test.js | 4 ++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/ui/marker.js b/src/ui/marker.js index 5bba98f4423..2cc66efe3be 100644 --- a/src/ui/marker.js +++ b/src/ui/marker.js @@ -200,6 +200,12 @@ export default class Marker extends Evented { this._element.addEventListener('dragstart', (e: DragEvent) => { e.preventDefault(); }); + this._element.addEventListener('focus', () => { + // revert the default scrolling action of the container + const el = this._map.getContainer(); + el.scrollTop = 0; + el.scrollLeft = 0; + }); applyAnchorClass(this._element, this._anchor, 'marker'); this._popup = null; diff --git a/test/unit/ui/marker.test.js b/test/unit/ui/marker.test.js index ee44824945a..3fed1cf3bfd 100644 --- a/test/unit/ui/marker.test.js +++ b/test/unit/ui/marker.test.js @@ -130,7 +130,7 @@ test('Enter key on Marker opens a popup that was closed', (t) => { // popup not initially open t.notOk(marker.getPopup().isOpen()); - simulate.keypress(marker.getElement(), { code: 'Enter' }); + simulate.keypress(marker.getElement(), {code: 'Enter'}); // popup open after Enter keypress t.ok(marker.getPopup().isOpen()); @@ -149,7 +149,7 @@ test('Space key on Marker opens a popup that was closed', (t) => { // popup not initially open t.notOk(marker.getPopup().isOpen()); - simulate.keypress(marker.getElement(), { code: 'Space' }); + simulate.keypress(marker.getElement(), {code: 'Space'}); // popup open after Enter keypress t.ok(marker.getPopup().isOpen()); From 0be8a5b97f25795dcc6d502818b60df9d30694ee Mon Sep 17 00:00:00 2001 From: Vladimir Agafonkin Date: Wed, 6 Nov 2019 12:06:48 +0200 Subject: [PATCH 16/16] prevent marker focus on click --- src/ui/marker.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ui/marker.js b/src/ui/marker.js index 2cc66efe3be..b35be447895 100644 --- a/src/ui/marker.js +++ b/src/ui/marker.js @@ -200,6 +200,10 @@ export default class Marker extends Evented { this._element.addEventListener('dragstart', (e: DragEvent) => { e.preventDefault(); }); + this._element.addEventListener('mousedown', (e: MouseEvent) => { + // prevent focusing on click + e.preventDefault(); + }); this._element.addEventListener('focus', () => { // revert the default scrolling action of the container const el = this._map.getContainer();