Dragndrop/matching aria/screenreader updates - #1315
Conversation
There was a problem hiding this comment.
Pull request overview
This PR follows up on prior keyboard-accessibility work for Runestone’s DragNDrop and Matching interactives, adding NVDA-friendly click handling, arrow-key navigation, and stricter focus/tab behavior to reduce invalid interactions.
Changes:
- Matching: adds keyboard navigation helpers (Tab trapping, arrow navigation), click-to-activate support, and disables MathJax/tabbable nested math inside boxes.
- DragNDrop: adds click-based selection/placement, improves arrow navigation across “columns”, and disables MathJax/tabbable nested math inside premises.
- Removes the CSS pointer-events hack for MathJax in drop targets, relying instead on event handling using
currentTarget.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| bases/rsptx/interactives/runestone/matching/test/matching.test.js | Adds keyboard/mouse interaction tests for Matching (tab trapping, arrows, click behavior, math tab stops). |
| bases/rsptx/interactives/runestone/matching/js/matching.js | Implements Matching keyboard navigation, click activation, and MathJax tab-stop disabling. |
| bases/rsptx/interactives/runestone/dragndrop/test/dragndrop.test.js | Expands DragNDrop coverage for arrow navigation, click activation, and math tab stops. |
| bases/rsptx/interactives/runestone/dragndrop/js/dragndrop.js | Adds click interactions, improves focus movement logic, and hardens drop handling via currentTarget. |
| bases/rsptx/interactives/runestone/dragndrop/css/dragndrop.css | Removes pointer-events CSS workaround for MathJax inside drop zones. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (e.key === "Enter") { | ||
| e.preventDefault(); | ||
| if (!this.selectedBox) { | ||
| this.selectedBox = box; | ||
| box.classList.add("selected"); | ||
| } else { | ||
| if (box !== this.selectedBox) | ||
| this.createPermanentLine(this.selectedBox, box); | ||
| this.selectedBox.classList.remove("selected"); | ||
| this.selectedBox = null; | ||
| const currentIndex = this.allBoxes.indexOf(box); | ||
| const next = this.allBoxes[currentIndex + 1]; | ||
| if (next) next.focus(); | ||
| else this.allBoxes[0].focus(); | ||
| } | ||
| this.activateBox(box); | ||
| } else if (this.selectedBox && e.key === "Tab") { |
| if (!this.selectedPremise || ev.target.closest(".premise")) { | ||
| return; | ||
| } | ||
| ev.preventDefault(); | ||
| this.placeSelectedPremise(dpSpan); |
|
Now I can't select anything in dragndrop. It is still tabbing into the math instead of stopping the top level. |
ec5be45 to
b144415
Compare
|
Just did a fresh build of the JS. I can't reproduce your failure to reproduce. :) Just force pushed with a change to also make response Mathjax untabbable. Now the only way to reach them is when placing a premise. Looks like the two new Copilot suggestions are existing code that is working. Will let you decide whether to adopt those two suggestions. If you still can't reproduce I'll loop back in a week. |
| activateBox(box) { | ||
| if (!this.selectedBox) { | ||
| this.setSelectedBox(box); | ||
| const firstOppositeBox = this.getTabbableBoxes()[0]; | ||
| firstOppositeBox?.focus(); | ||
| return; | ||
| } | ||
|
|
||
| if (box !== this.selectedBox) { | ||
| this.createPermanentLine(this.selectedBox, box); | ||
| } | ||
| this.setSelectedBox(null); | ||
| box.focus(); | ||
| } |
| dgSpan.addEventListener("click", (ev) => { | ||
| ev.preventDefault(); | ||
| if (this.selectedPremise === dgSpan) { | ||
| this.deselectPremise(); | ||
| } else { | ||
| this.selectPremise(dgSpan); | ||
| } | ||
| }); |
|
Found a quirk when NVDA is engaged. Marking as draft until I'm back from Colorado. |
b144415 to
2527b6c
Compare
ascholerChemeketa
changed the title
| var data = ev.dataTransfer.getData("draggableID"); | ||
| var draggedSpan = document.getElementById(data); | ||
| if ( | ||
| ev.target.classList.contains("draggable-drop") && | ||
| !this.strangerDanger(draggedSpan) && | ||
| !this.premiseArray.includes(ev.target) // don't drop on another premise! | ||
| dropTarget.classList.contains("draggable-drop") && | ||
| !this.strangerDanger(draggedSpan) | ||
| ) { |
There was a problem hiding this comment.
Neither click nor keyboard will hit the mathjax - the button always gets the focus.
| const fromLabel = this.getBoxLabel(conn.fromBox); | ||
| const toLabel = this.getBoxLabel(conn.toBox); | ||
| const line = document.createElement("div"); | ||
| line.className = "conn-entry"; | ||
| line.textContent = `${fromLabel} → ${toLabel}`; | ||
| line.innerHTML = `${fromLabel} <span aria-hidden="true">→</span><span class="visuallyhidden">connected to</span> ${toLabel}`; | ||
| this.connList.appendChild(line); |
There was a problem hiding this comment.
Not too worried about injection here. We and or author control everything that might be inserted.
|
This looks good, just one kind of odd unexpected behavior in the card sort: If I simply click on one of the premises it automatically moves it to the first response (not first blank response) and then it is highlighted in dashed green. From there you can move it with the arrow keys or drag it with the mouse. Maybe this is helpful for people who don't know how to interact with these?? Or maybe this is accidental? |
|
Yes, that is an artifact of the new focus/navigation logic. Once you select a premise, the assumption is you need to place it into a response. So if it was keyboard activation, it goes into the first premise and then can be navigated. Mouse click activates the item. If you fail to drag it, it works like you activated with keyboard. We could catch that situation and prevent it. But I think it does help in the situation someone does not understand what to do and clicks on a premise - makes it clear they get moved over there. So I think it is a happy accident. |
Follow up to address issues in #1308
Also fixes another issue: when NVDA is active it consumes Enter/Space and generates click events, so we need to allow for those as if they were enter
Also implements changes to matching questions: