Skip to content

Matching: fix timing issue for rendering dragables with mathjax content - #1417

Merged
bnmnetp merged 3 commits into
RunestoneInteractive:mainfrom
ascholerChemeketa:matching-more-access
Aug 25, 2026
Merged

Matching: fix timing issue for rendering dragables with mathjax content#1417
bnmnetp merged 3 commits into
RunestoneInteractive:mainfrom
ascholerChemeketa:matching-more-access

Conversation

@ascholerChemeketa

Copy link
Copy Markdown
Contributor

Found a timing issue with grabbing mathjax content in matching labels.

Copilot AI lite review requested due to automatic review settings August 24, 2026 23:38

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

This PR addresses an accessibility timing issue where Matching box labels containing MathJax content may be rendered before MathJax’s speech text is available, leaving stale/incorrect aria-label values for draggable/droppable boxes.

Changes:

  • Add a MutationObserver in MatchingProblem to detect when MathJax speech attributes become available and refresh affected box labels.
  • Add a unit test to verify box labels update after MathJax speech content appears.

Reviewed changes

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

File Description
bases/rsptx/interactives/runestone/matching/js/matching.js Adds a MathJax speech attribute observer to refresh box aria-labels after MathJax updates.
bases/rsptx/interactives/runestone/matching/test/matching.test.js Adds a test ensuring aria-label updates when MathJax speech becomes available asynchronously.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +484 to +491
this.mathJaxSpeechObserver = new MutationObserver((mutations) => {
for (const mutation of mutations) {
const box = mutation.target.closest?.(".box");
if (box && this.allBoxes.includes(box)) {
this.updateBoxAriaLabel(box);
}
}
});

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 a new commit

@ascholerChemeketa

Copy link
Copy Markdown
Contributor Author

https://github.com/RunestoneInteractive/interactives updated with extensive math sample

@ascholerChemeketa
ascholerChemeketa marked this pull request as draft August 25, 2026 14:47
@ascholerChemeketa

Copy link
Copy Markdown
Contributor Author

Checking on that issue...

Copilot AI review requested due to automatic review settings August 25, 2026 15:37
@ascholerChemeketa
ascholerChemeketa marked this pull request as ready for review August 25, 2026 15:37

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 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

bases/rsptx/interactives/runestone/matching/js/matching.js:510

  • MutationObserver is configured to watch only data-semantic-speech-none changes. However, getAccessibleElementText() (via getMathJaxSpeechText) also uses MathJax-provided aria-label as speech text, so updates delivered via aria-label won’t trigger refreshes of box/connection aria-labels or the connection list. Consider also observing aria-label (and/or broadening the observer to catch MathJax speech becoming available via node insertion).
        this.mathJaxSpeechObserver.observe(this.containerDiv, {
            subtree: true,
            attributes: true,
            attributeFilter: ["data-semantic-speech-none"],
        });

@bnmnetp
bnmnetp merged commit a2cfc05 into RunestoneInteractive:main Aug 25, 2026
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.

3 participants