[UEPR-516] Debug modal accessibility#450
[UEPR-516] Debug modal accessibility#450kbangelov wants to merge 3 commits intoscratchfoundation:release/UEPR-297-accessibility-improvementsfrom
Conversation
kbangelov
left a comment
There was a problem hiding this comment.
Also, the close button's focus doesn't look the prettiest. Its behavior was actually not changed at all in this PR, but maybe it's still worth fixing in terms of styling?
packages/scratch-gui/src/components/debug-modal/debug-modal.jsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR enhances keyboard accessibility for the debug modal component by adding arrow key navigation between slides and making navigation buttons keyboard-focusable. The changes support Scratch's accessibility initiative to ensure users can navigate the debugging modal using only the keyboard.
Changes:
- Added keyboard navigation with arrow keys to cycle through modal slides
- Made previous and next navigation buttons focusable and keyboard-activatable
- Added refs to slide items for programmatic focus management
Comments suppressed due to low confidence (7)
packages/scratch-gui/src/components/debug-modal/debug-modal.jsx:158
- The slide items with role="button" are missing keyboard event handlers. While arrow key navigation is handled globally, these elements should also respond to Enter and Space keys for activation when focused. Add onKeyDown handler to trigger handleTopicSelect when Enter or Space is pressed.
tabIndex={-1}
role="button"
ref={slideRefs[index]}
packages/scratch-gui/src/components/debug-modal/debug-modal.jsx:200
- Navigation buttons with role="button" should have aria-label attributes to provide accessible names for screen reader users. The alt attribute on img elements is not sufficient when the img is a child of an interactive element with role="button". Add aria-label="Previous" to the previous button.
tabIndex={0}
role="button"
onKeyDown={handleKeyDownPrevious}
/>
packages/scratch-gui/src/components/debug-modal/debug-modal.jsx:94
- The global keydown event listener on window will intercept arrow key events from all elements, potentially interfering with other interactive elements within the modal (e.g., text inputs, scrollable areas). Consider checking event.target to ensure the event is not from an input field or other interactive element, or scope the event listener to a specific element within the modal.
useEffect(() => {
if (!isOpen) return;
window.addEventListener('keydown', handleKeyDownSlides);
return () => window.removeEventListener('keydown', handleKeyDownSlides);
}, [isOpen, handleKeyDownSlides]);
packages/scratch-gui/src/components/debug-modal/debug-modal.jsx:100
- Elements with role="button" should respond to both Enter and Space keys for full keyboard accessibility. Currently, only Enter key is handled. Add handling for KEY.SPACE as well to match standard button behavior.
const handleKeyDownPrevious = useCallback(e => {
if (e.key === KEY.ENTER) {
handlePrevious();
}
}, [handlePrevious]);
packages/scratch-gui/src/components/debug-modal/debug-modal.jsx:106
- Elements with role="button" should respond to both Enter and Space keys for full keyboard accessibility. Currently, only Enter key is handled. Add handling for KEY.SPACE as well to match standard button behavior.
const handleKeyDownNext = useCallback(e => {
if (e.key === KEY.ENTER) {
handleNext();
}
}, [handleNext]);
packages/scratch-gui/src/components/debug-modal/debug-modal.jsx:211
- Navigation buttons with role="button" should have aria-label attributes to provide accessible names for screen reader users. The alt attribute on img elements is not sufficient when the img is a child of an interactive element with role="button". Add aria-label="Next" to the next button.
tabIndex={0}
role="button"
onKeyDown={handleKeyDownNext}
/>
packages/scratch-gui/src/components/debug-modal/debug-modal.jsx:156
- Setting tabIndex={-1} on elements with role="button" removes them from the keyboard tab order, which defeats the purpose of adding keyboard accessibility. The slide items should be focusable via keyboard navigation. Consider using tabIndex={0} to make them keyboard accessible, or manage focus programmatically if using a roving tabindex pattern.
tabIndex={-1}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Resolves
https://scratchfoundation.atlassian.net/browse/UEPR-516
Proposed Changes
Reason for Changes
Part of Accessibility initiative for Scratch.