From c1fdb762def8527bca083cc5646983f2d2ff2859 Mon Sep 17 00:00:00 2001 From: Kevin Kim Date: Tue, 31 Jan 2023 12:08:00 -0500 Subject: [PATCH] Popup keyboard navigation bug when dismissing popup (#726) * change from 1.7.1 to 1.9.3 * fix focusing error when pop-up close button is pressed * fix tooltip error when esc button is pressed * styling fixes and added comments * add tests * fix testing detail * package.json styling * update to testing * Add comment on what keys are being handled during focus handler * fix testing * remove unneeded timeout from test --------- Co-authored-by: prushfor Co-authored-by: Aliyan Haq <55751566+AliyanH@users.noreply.github.com> --- src/mapml/features/featureGroup.js | 9 ++++-- src/mapml/layers/MapLayer.js | 8 +++-- test/e2e/core/popupTabNavigation.test.js | 40 ++++++++++++++++++++++++ 3 files changed, 52 insertions(+), 5 deletions(-) diff --git a/src/mapml/features/featureGroup.js b/src/mapml/features/featureGroup.js index 18665c9cf..475378e62 100644 --- a/src/mapml/features/featureGroup.js +++ b/src/mapml/features/featureGroup.js @@ -59,6 +59,7 @@ export var FeatureGroup = L.FeatureGroup.extend({ * @private */ _handleFocus: function(e) { + // tab, shift, cr, esc, up, left, down, right, if(([9, 16, 27, 37, 38, 39, 40].includes(e.keyCode)) && e.type === "keydown"){ let index = this._map.featureIndex.currentIndex; // Down/right arrow keys replicate tabbing through the feature index @@ -94,13 +95,17 @@ export var FeatureGroup = L.FeatureGroup.extend({ obj._map.featureIndex.inBoundFeatures[0].path.setAttribute("tabindex", 0); }, 0); } + // tab, shift, cr, esc, right, left, up, down, + // 1, 2, 3, 4, 5, 6, 7 (featureIndexOverlay available index items + // [8, 9] being allocated to next, previous menu items). } else if (!([9, 16, 13, 27, 37, 38, 39, 40, 49, 50, 51, 52, 53, 54, 55].includes(e.keyCode))){ this._map.featureIndex.currentIndex = 0; this._map.featureIndex.inBoundFeatures[0].path.focus(); } - + + // 27 added so that the tooltip opens when dismissing popup with 'esc' key if(e.target.tagName.toUpperCase() !== "G") return; - if(([9, 13, 16, 37, 38, 39, 40, 49, 50, 51, 52, 53, 54, 55].includes(e.keyCode)) && e.type === "keyup") { + if(([9, 13, 16, 37, 38, 39, 40, 49, 50, 51, 52, 53, 54, 55, 27].includes(e.keyCode)) && e.type === "keyup") { this.openTooltip(); } else if (e.keyCode === 13 || e.keyCode === 32){ this.closeTooltip(); diff --git a/src/mapml/layers/MapLayer.js b/src/mapml/layers/MapLayer.js index f833592de..d00e79c5b 100644 --- a/src/mapml/layers/MapLayer.js +++ b/src/mapml/layers/MapLayer.js @@ -1330,17 +1330,19 @@ export var MapMLLayer = L.Layer.extend({ let path = focusEvent.originalEvent.path || focusEvent.originalEvent.composedPath(); let isTab = focusEvent.originalEvent.keyCode === 9, shiftPressed = focusEvent.originalEvent.shiftKey; - if((path[0].classList.contains("leaflet-popup-close-button") && isTab && !shiftPressed) || focusEvent.originalEvent.keyCode === 27){ + if ((path[0].classList.contains("leaflet-popup-close-button") && isTab && !shiftPressed) || + focusEvent.originalEvent.keyCode === 27 || + path[0].classList.contains("leaflet-popup-close-button") && focusEvent.originalEvent.keyCode === 13){ setTimeout(() => { - L.DomEvent.stop(focusEvent); map.closePopup(popup); group.focus(); + L.DomEvent.stop(focusEvent); }, 0); } else if ((path[0].title==="Focus Map" || 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); group.focus(); + L.DomEvent.stop(focusEvent); }, 0); } } diff --git a/test/e2e/core/popupTabNavigation.test.js b/test/e2e/core/popupTabNavigation.test.js index 4c987c494..4f0cfe0d1 100644 --- a/test/e2e/core/popupTabNavigation.test.js +++ b/test/e2e/core/popupTabNavigation.test.js @@ -127,6 +127,46 @@ test.describe("Playwright Keyboard Navigation + Query Layer Tests" , () => { expect(f).toEqual("M330 83L586 83L586 339L330 339z"); }); + test("Tooltip appears after pressing esc key", async () => { + await page.keyboard.press("Enter"); + await page.waitForTimeout(500); + await page.keyboard.down("Escape"); // focus back on feature + await page.keyboard.up("Escape"); + await page.waitForTimeout(500); + + 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.querySelector(".leaflet-interactive"), nh); + const f = await (await page.evaluateHandle(elem => elem.getAttribute("d"), rh)).jsonValue(); + + let tooltipCount = await page.$eval("mapml-viewer .leaflet-tooltip-pane", div => div.childElementCount); + expect(tooltipCount).toEqual(1); + expect(f).toEqual("M330 83L586 83L586 339L330 339z"); + }); + + test("Tooltip appears after pressing enter on close button", async () => { + await page.keyboard.press("Enter"); // focus back into popup + await page.keyboard.press("Tab"); + await page.keyboard.press("Tab"); + await page.keyboard.press("Tab"); + await page.keyboard.press("Tab"); + await page.keyboard.press("Tab"); + await page.keyboard.press("Tab"); // focus on x button + await page.keyboard.down("Enter"); // press x button + await page.keyboard.up("Enter"); + await page.waitForTimeout(500); + + 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.querySelector(".leaflet-interactive"), nh); + const f = await (await page.evaluateHandle(elem => elem.getAttribute("d"), rh)).jsonValue(); + + let tooltipCount = await page.$eval("mapml-viewer .leaflet-tooltip-pane", div => div.childElementCount); + expect(tooltipCount).toEqual(1); + expect(f).toEqual("M330 83L586 83L586 339L330 339z"); + + }); + test("Next feature button focuses next feature", async () => { await page.keyboard.press("Enter"); // popup with link await page.waitForTimeout(500);