From 87f5a505b8ba7010fe27346795a9954cb310daa6 Mon Sep 17 00:00:00 2001 From: Ahmad Ayubi <55214462+ahmadayubi@users.noreply.github.com> Date: Tue, 16 Feb 2021 15:31:07 -0500 Subject: [PATCH] Follows updated keyboard interaction rules (#292) * Follows updated keyboard interaction rules * Test updates * Change to headless test * Open feature popup on space * New interactions * Adds tests and new keyboard interaction Co-authored-by: Peter Rushforth --- src/mapml.css | 6 +-- src/mapml/handlers/QueryHandler.js | 6 ++- src/mapml/layers/Crosshair.js | 34 ++++++------- src/mapml/layers/FeatureLayer.js | 9 ++++ src/mapml/layers/MapLayer.js | 58 ++++++++++++++++++----- test/e2e/core/keyboardInteraction.test.js | 24 ++++++---- 6 files changed, 94 insertions(+), 43 deletions(-) diff --git a/src/mapml.css b/src/mapml.css index 37fada4c1..230d591a4 100644 --- a/src/mapml.css +++ b/src/mapml.css @@ -391,9 +391,9 @@ summary { } .mapml-crosshair { - margin: -18px 0 0 -18px; - width: 36px; - height: 36px; + margin: -36px 0 0 -36px; + width: 72px; + height: 72px; left: 50%; top: 50%; content: ''; diff --git a/src/mapml/handlers/QueryHandler.js b/src/mapml/handlers/QueryHandler.js index cbf14e012..b40456115 100644 --- a/src/mapml/handlers/QueryHandler.js +++ b/src/mapml/handlers/QueryHandler.js @@ -22,13 +22,17 @@ export var QueryHandler = L.Handler.extend({ } }, _queryTopLayerAtMapCenter: function (event) { - if (event.originalEvent.key === " ") { + setTimeout(() => { + if (this._map.isFocused && !this._map._popupClosed && (event.originalEvent.key === " " || +event.originalEvent.keyCode === 13)) { this._map.fire('click', { latlng: this._map.getCenter(), layerPoint: this._map.latLngToLayerPoint(this._map.getCenter()), containerPoint: this._map.latLngToContainerPoint(this._map.getCenter()) }); + } else { + delete this._map._popupClosed; } + }, 0); }, _queryTopLayer: function(event) { var layer = this._getTopQueryableLayer(); diff --git a/src/mapml/layers/Crosshair.js b/src/mapml/layers/Crosshair.js index ad622adb1..4021ee00f 100644 --- a/src/mapml/layers/Crosshair.js +++ b/src/mapml/layers/Crosshair.js @@ -30,16 +30,22 @@ export var Crosshair = L.Layer.extend({ this._container = L.DomUtil.create("div", "mapml-crosshair", map._container); this._container.innerHTML = svgInnerHTML; - this._mapFocused = false; + map.isFocused = false; this._isQueryable = false; map.on("layerchange layeradd layerremove overlayremove", this._toggleEvents, this); - L.DomEvent.on(map._container, "keydown keyup mousedown", this._onKey, this); - + map.on("popupopen", this._isMapFocused, this); + L.DomEvent.on(map._container, "keydown keyup mousedown", this._isMapFocused, this); this._addOrRemoveCrosshair(); }, + onRemove: function (map) { + map.off("layerchange layeradd layerremove overlayremove", this._toggleEvents); + map.off("popupopen", this._isMapFocused); + L.DomEvent.off(map._container, "keydown keyup mousedown", this._isMapFocused); + }, + _toggleEvents: function () { if (this._hasQueryableLayer()) { this._map.on("viewreset move moveend", this._addOrRemoveCrosshair, this); @@ -59,7 +65,7 @@ export var Crosshair = L.Layer.extend({ _hasQueryableLayer: function () { let layers = this._map.options.mapEl.layers; - if (this._mapFocused) { + if (this._map.isFocused) { for (let layer of layers) { if (layer.checked && layer._layer.queryable) { return true; @@ -69,20 +75,14 @@ export var Crosshair = L.Layer.extend({ return false; }, - _onKey: function (e) { - //set mapFocused = true if arrow buttons are used - if (["keydown", "keyup"].includes(e.type) && e.target.classList.contains("leaflet-container") && [32, 37, 38, 39, 40, 187, 189].includes(+e.keyCode)) { - this._mapFocused = true; - //set mapFocused = true if map is focued using tab - } else if (e.type === "keyup" && e.target.classList.contains("leaflet-container") && +e.keyCode === 9) { - this._mapFocused = true; - // set mapFocused = false and close all popups if tab or escape is used - } else if((e.type === "keyup" && e.target.classList.contains("leaflet-interactive") && +e.keyCode === 9) || +e.keyCode === 27){ - this._mapFocused = false; - this._map.closePopup(); - // set mapFocused = false if any other key is pressed + _isMapFocused: function (e) { + //set this._map.isFocused = true if arrow buttons are used + if (this._map._container.parentNode.activeElement.classList.contains("leaflet-container") && ["keydown"].includes(e.type) && (e.shiftKey && e.keyCode === 9)) { + this._map.isFocused = false; + } else if (this._map._container.parentNode.activeElement.classList.contains("leaflet-container") && ["keyup", "keydown"].includes(e.type)) { + this._map.isFocused = true; } else { - this._mapFocused = false; + this._map.isFocused = false; } this._addOrRemoveCrosshair(); }, diff --git a/src/mapml/layers/FeatureLayer.js b/src/mapml/layers/FeatureLayer.js index d90321683..2c5c81ce4 100644 --- a/src/mapml/layers/FeatureLayer.js +++ b/src/mapml/layers/FeatureLayer.js @@ -281,6 +281,10 @@ export var MapMLFeatures = L.FeatureGroup.extend({ if (options.onEachFeature) { options.onEachFeature(layer.properties, layer); + layer._events.keypress.push({ + "ctx": layer, + "fn": this._onSpacePress, + }); } if(this._staticFeature){ let featureZoom = mapml.getAttribute('zoom') || nativeZoom; @@ -324,6 +328,11 @@ export var MapMLFeatures = L.FeatureGroup.extend({ for(let i = 0; i < toDelete.length;i++){ this._container.removeChild(toDelete[i]); } + }, + _onSpacePress: function(e){ + if(e.originalEvent.keyCode === 32){ + this._openPopup(e); + } }, geometryToLayer: function (mapml, pointToLayer, coordsToLatLng, vectorOptions, nativeCS, zoom) { var geometry = mapml.tagName.toUpperCase() === 'FEATURE' ? mapml.getElementsByTagName('geometry')[0] : mapml, diff --git a/src/mapml/layers/MapLayer.js b/src/mapml/layers/MapLayer.js index 283bb0c3b..404738d8e 100644 --- a/src/mapml/layers/MapLayer.js +++ b/src/mapml/layers/MapLayer.js @@ -95,7 +95,7 @@ export var MapMLLayer = L.Layer.extend({ var c = document.createElement('div'); c.classList.add("mapml-popup-content"); c.insertAdjacentHTML('afterbegin', properties.innerHTML); - geometry.bindPopup(c, {autoPan:false, closeButton: false, minWidth: 108}); + geometry.bindPopup(c, {autoPan:false, minWidth: 108}); } } }); @@ -125,7 +125,7 @@ export var MapMLLayer = L.Layer.extend({ var c = document.createElement('div'); c.classList.add("mapml-popup-content"); c.insertAdjacentHTML('afterbegin', properties.innerHTML); - geometry.bindPopup(c, {autoPan:false, closeButton: false, minWidth: 108}); + geometry.bindPopup(c, {autoPan:false, minWidth: 108}); } } }).addTo(map); @@ -1162,8 +1162,7 @@ export var MapMLLayer = L.Layer.extend({ content = popup._container.getElementsByClassName("mapml-popup-content")[0]; content.setAttribute("tabindex", "-1"); - content.focus(); - popup._count = 0; + popup._count = 0; // used for feature pagination if(popup._source._eventParents){ // check if the popup is for a feature or query layer = popup._source._eventParents[Object.keys(popup._source._eventParents)[0]]; // get first parent of feature, there should only be one @@ -1232,7 +1231,11 @@ export var MapMLLayer = L.Layer.extend({ L.DomEvent.on(controlFocusButton, 'click', L.DomEvent.stop); L.DomEvent.on(controlFocusButton, 'click', (e) => { map.closePopup(); - map._controlContainer.focus(); + if(map._controlContainer.firstElementChild.firstElementChild.firstElementChild){ + map._controlContainer.firstElementChild.firstElementChild.firstElementChild.focus(); + } else { + map._controlContainer.focus(); + } }, popup); let divider = L.DomUtil.create("hr"); @@ -1241,31 +1244,60 @@ export var MapMLLayer = L.Layer.extend({ popup._navigationBar = div; popup._content.appendChild(divider); popup._content.appendChild(div); - + + content.focus(); if(path) { // e.target = this._map // Looks for keydown, more specifically tab and shift tab map.on("keydown", focusFeature); - // if popup closes then the focusFeature handler can be removed - map.on("popupclose", removeHandlers); + } else { + map.on("keydown", focusMap); } // When popup is open, what gets focused with tab needs to be done using JS as the DOM order is not in an accessibility friendly manner function focusFeature(focusEvent){ - if(focusEvent.originalEvent.path[0].title==="Focus Controls" && +focusEvent.originalEvent.keyCode === 9){ + let isTab = focusEvent.originalEvent.keyCode === 9, + shiftPressed = focusEvent.originalEvent.shiftKey; + if((focusEvent.originalEvent.path[0].classList.contains("leaflet-popup-close-button") && isTab && !shiftPressed) || focusEvent.originalEvent.keyCode === 27){ L.DomEvent.stop(focusEvent); - path.focus(); - } else if(focusEvent.originalEvent.shiftKey && +focusEvent.originalEvent.keyCode === 9){ map.closePopup(popup); - L.DomEvent.stop(focusEvent); path.focus(); + } else if ((focusEvent.originalEvent.path[0].title==="Focus Map" || focusEvent.originalEvent.path[0].classList.contains("mapml-popup-content")) && isTab && shiftPressed){ + setTimeout(() => { //timeout needed so focus of the feature is done even after the keypressup event occurs + L.DomEvent.stop(focusEvent); + map.closePopup(popup); + path.focus(); + }, 0); + } + } + + function focusMap(focusEvent){ + let isTab = focusEvent.originalEvent.keyCode === 9, + shiftPressed = focusEvent.originalEvent.shiftKey; + + if((focusEvent.originalEvent.keyCode === 13 && focusEvent.originalEvent.path[0].classList.contains("leaflet-popup-close-button")) || focusEvent.originalEvent.keyCode === 27 ){ + L.DomEvent.stopPropagation(focusEvent); + map._container.focus(); + map.closePopup(popup); + if(focusEvent.originalEvent.keyCode !== 27)map._popupClosed = true; + } else if (isTab && focusEvent.originalEvent.path[0].classList.contains("leaflet-popup-close-button")){ + map.closePopup(popup); + } else if ((focusEvent.originalEvent.path[0].title==="Focus Map" || focusEvent.originalEvent.path[0].classList.contains("mapml-popup-content")) && isTab && shiftPressed){ + setTimeout(() => { //timeout needed so focus of the feature is done even after the keypressup event occurs + L.DomEvent.stop(focusEvent); + map.closePopup(popup); + map._container.focus(); + }, 0); } } + // if popup closes then the focusFeature handler can be removed + map.on("popupclose", removeHandlers); function removeHandlers(removeEvent){ if (removeEvent.popup === popup){ map.off("keydown", focusFeature); - map.off("popupclose", removeHandlers); + map.off("keydown", focusMap); + map.off('popupclose', removeHandlers); } } }, diff --git a/test/e2e/core/keyboardInteraction.test.js b/test/e2e/core/keyboardInteraction.test.js index 9866c958f..e85b60e9e 100644 --- a/test/e2e/core/keyboardInteraction.test.js +++ b/test/e2e/core/keyboardInteraction.test.js @@ -30,7 +30,7 @@ jest.setTimeout(50000); expect(afterTab).toEqual(""); }); - test("[" + browserType + "]" + " Crosshair remains on map move with arrow keys + space", async () => { + test("[" + browserType + "]" + " Crosshair remains on map move with arrow keys", async () => { await page.keyboard.press("ArrowUp"); await page.waitForTimeout(500); await page.keyboard.press("ArrowDown"); @@ -39,13 +39,11 @@ jest.setTimeout(50000); await page.waitForTimeout(500); await page.keyboard.press("ArrowRight"); await page.waitForTimeout(500); - await page.keyboard.press(" ") - await page.waitForTimeout(500); const afterMove = await page.$eval("div > div.mapml-crosshair", (div) => div.style.visibility); expect(afterMove).toEqual(""); }); - test("[" + browserType + "]" + " Crosshair hidden on esc + tab out", async () => { + test("[" + browserType + "]" + " Crosshair shows on esc but hidden on tab out", async () => { await page.keyboard.press("Escape"); const afterEsc = await page.$eval("div > div.mapml-crosshair", (div) => div.style.visibility); await page.click("body"); @@ -55,7 +53,7 @@ jest.setTimeout(50000); await page.keyboard.press("Tab"); const afterTab = await page.$eval("div > div.mapml-crosshair", (div) => div.style.visibility); - expect(afterEsc).toEqual("hidden"); + expect(afterEsc).toEqual(""); expect(afterTab).toEqual("hidden"); }); @@ -164,16 +162,24 @@ jest.setTimeout(50000); const rh6 = await page.evaluateHandle(root => root.activeElement, nh6); const f6 = await (await page.evaluateHandle(elem => elem.title, rh6)).jsonValue(); + await page.keyboard.press("Tab"); + const h7 = await page.evaluateHandle(() => document.querySelector("mapml-viewer")); + const nh7 = await page.evaluateHandle(doc => doc.shadowRoot, h7); + const rh7 = await page.evaluateHandle(root => root.activeElement, nh7); + const f7 = await (await page.evaluateHandle(elem => elem.className, rh7)).jsonValue(); + expect(f).toEqual("mapml-popup-content"); - expect(f2).toEqual("A"); + expect(f2.toUpperCase()).toEqual("A"); expect(f3).toEqual("Focus Map"); expect(f4).toEqual("Previous Feature"); expect(f5).toEqual("Next Feature"); expect(f6).toEqual("Focus Controls"); + expect(f7).toEqual("leaflet-popup-close-button"); }); test("[" + browserType + "]" + " Tab to next feature after tabbing out of popup", async () => { await page.keyboard.press("Tab"); + const h = await page.evaluateHandle(() => document.querySelector("mapml-viewer")); const nh = await page.evaluateHandle(doc => doc.shadowRoot, h); const rh = await page.evaluateHandle(root => root.activeElement, nh); @@ -182,19 +188,19 @@ jest.setTimeout(50000); expect(f).toEqual("M-53 451L153 508L113 146L-53 191z"); }); - test("[" + browserType + "]" + " Shift + Tab to previous feature while popup open", async () => { + test("[" + browserType + "]" + " Shift + Tab to current feature while popup open", async () => { await page.keyboard.press("Enter"); await page.keyboard.press("Shift+Tab"); + const h = await page.evaluateHandle(() => document.querySelector("mapml-viewer")); const nh = await page.evaluateHandle(doc => doc.shadowRoot, h); const rh = await page.evaluateHandle(root => root.activeElement, nh); const f = await (await page.evaluateHandle(elem => elem.getAttribute("d"), rh)).jsonValue(); - expect(f).toEqual("M330 83L553 83L553 339L330 339z"); + expect(f).toEqual("M-53 451L153 508L113 146L-53 191z"); }); test("[" + browserType + "]" + " Previous feature button focuses previous feature", async () => { - await page.keyboard.press("Tab"); await page.keyboard.press("Enter"); await page.keyboard.press("Tab"); await page.keyboard.press("Tab");