Skip to content

Commit

Permalink
Popup keyboard navigation bug when dismissing popup (#726)
Browse files Browse the repository at this point in the history
* 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 <prushfor@L-BSC-A146390.nrn.nrcan.gc.ca>
Co-authored-by: Aliyan Haq <55751566+AliyanH@users.noreply.github.com>
  • Loading branch information
3 people committed Jan 31, 2023
1 parent 249b22b commit c1fdb76
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 5 deletions.
9 changes: 7 additions & 2 deletions src/mapml/features/featureGroup.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
Expand Down
8 changes: 5 additions & 3 deletions src/mapml/layers/MapLayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
40 changes: 40 additions & 0 deletions test/e2e/core/popupTabNavigation.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit c1fdb76

Please sign in to comment.