Skip to content

Add keyboard controls for dragndrop#1308

Open
ascholerChemeketa wants to merge 4 commits into
RunestoneInteractive:mainfrom
ascholerChemeketa:dragndrop-keyboard
Open

Add keyboard controls for dragndrop#1308
ascholerChemeketa wants to merge 4 commits into
RunestoneInteractive:mainfrom
ascholerChemeketa:dragndrop-keyboard

Conversation

@ascholerChemeketa

Copy link
Copy Markdown
Contributor

Implements keyboard controls allowing someone to complete a dragndrop without keyboard and mouse.

Mostly vibecoded. Interactions and basic aria attribute use verified by hand.

Copilot AI review requested due to automatic review settings July 15, 2026 10:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds keyboard-only interaction support to the Runestone DragNDrop interactive so learners can pick up a premise, navigate responses, and place/cancel via common keys, with accompanying visual focus/selection styling and Jest coverage.

Changes:

  • Introduces selectedPremise state plus key handlers for selecting premises, navigating between premises/responses, placing, and canceling selection.
  • Updates tab order behavior so responses are only tabbable while a premise is selected.
  • Adds CSS outlines for keyboard focus vs “picked up” state and adds a comprehensive keyboard-controls test suite.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
bases/rsptx/interactives/runestone/dragndrop/js/dragndrop.js Implements keyboard interaction model (selection, focus movement, placement) and tab-order toggling.
bases/rsptx/interactives/runestone/dragndrop/test/dragndrop.test.js Adds Jest tests covering the new keyboard behaviors and focus/tabindex expectations.
bases/rsptx/interactives/runestone/dragndrop/css/dragndrop.css Adds visual styling for focus vs selected/picked-up premise state.
Comments suppressed due to low confidence (1)

bases/rsptx/interactives/runestone/dragndrop/js/dragndrop.js:520

  • setDropListeners references self but no longer defines it (the previous implicit global self = this was removed). This will throw a ReferenceError on dragover/dragleave/drop for responses, breaking mouse/touch dragging.
    setDropListeners(dpSpan) {
        dpSpan.addEventListener(
            "dragover",
            function (ev) {
                self.isAnswered = true;
                ev.preventDefault();

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread bases/rsptx/interactives/runestone/dragndrop/js/dragndrop.js
Copilot AI review requested due to automatic review settings July 15, 2026 10:45

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment thread bases/rsptx/interactives/runestone/dragndrop/js/dragndrop.js Outdated
Comment on lines 522 to 523
dpSpan.addEventListener(
"dragover",
Copilot AI review requested due to automatic review settings July 15, 2026 10:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

bases/rsptx/interactives/runestone/dragndrop/js/dragndrop.js:527

  • setDropListeners uses self.isAnswered = true inside a handler that is already .bind(this). Here self resolves to the global window.self, so the component’s isAnswered flag won’t be updated correctly during pointer dragover.
            "dragover",
            function (ev) {
                self.isAnswered = true;
                ev.preventDefault();
                if (ev.target.classList.contains("possibleDrop")) {

Comment thread bases/rsptx/interactives/runestone/dragndrop/js/dragndrop.js Outdated
Comment on lines +668 to +672
this.isAnswered = true;
this.logBookEvent({
event: "dragNdrop-drop",
div_id: this.divid,
act: `${premise.id} -> ${destinationName}`,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resolved in followup commit.

Comment thread bases/rsptx/interactives/runestone/dragndrop/js/dragndrop.js
Copilot AI review requested due to automatic review settings July 15, 2026 11:11

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment on lines 535 to 539
dpSpan.addEventListener("dragleave", function (ev) {
self.isAnswered = true;
this.isAnswered = true;
ev.preventDefault();
if (!ev.target.classList.contains("possibleDrop")) {
return;
Comment on lines +57 to +64
/* Keep keyboard focus and the picked-up state visually distinct. */
.draggable-drag:focus {
outline: 3px solid #3498db;
}

.draggable-drag.selected {
outline: 3px dashed #27ae60;
}
Copilot AI review requested due to automatic review settings July 15, 2026 14:07

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

Comment on lines 536 to 540
dpSpan.addEventListener("dragleave", function (ev) {
self.isAnswered = true;
this.isAnswered = true;
ev.preventDefault();
if (!ev.target.classList.contains("possibleDrop")) {
return;
Comment on lines +663 to +667
moveSelectedPremise(destination) {
const premise = this.selectedPremise;
if (!premise || premise.parentElement === destination) {
return;
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants