🎨 Palette: Make scrollable output regions keyboard accessible#3521
🎨 Palette: Make scrollable output regions keyboard accessible#3521EffortlessSteven wants to merge 1 commit intomainfrom
Conversation
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request improves the accessibility of scrollable output regions in the browser examples by replacing invalid tags with styled
| <label>Streaming Output:</label> | ||
| <div id="streaming-output" class="streaming-output">Streaming output will appear here...</div> | ||
| <div style="font-weight: 500; margin-bottom: 5px;" id="streaming-output-label">Streaming Output:</div> | ||
| <div id="streaming-output" class="streaming-output" tabindex="0" role="region" aria-labelledby="streaming-output-label">Streaming output will appear here...</div> |
There was a problem hiding this comment.
The .streaming-output class (defined on line 167) lacks a max-height property, unlike the .output class. Without a height constraint, this container will grow vertically to fit its content rather than scrolling internally. As a result, tabindex="0" becomes a redundant tab stop for keyboard users because there is no overflowing content to scroll within the element itself. To fix this, you should add a max-height (e.g., 300px) to the .streaming-output CSS class or as an inline style to ensure the region is actually scrollable.
| <div id="streaming-output" class="streaming-output" tabindex="0" role="region" aria-labelledby="streaming-output-label">Streaming output will appear here...</div> | |
| <div id="streaming-output" class="streaming-output" tabindex="0" role="region" aria-labelledby="streaming-output-label" style="max-height: 300px;">Streaming output will appear here...</div> |
| <label>Streaming Output:</label> | ||
| <div id="streaming-output" class="streaming-output">Streaming output will appear here...</div> | ||
| <div style="font-weight: 500; margin-bottom: 5px;" id="streaming-output-label">Streaming Output:</div> | ||
| <div id="streaming-output" class="streaming-output" tabindex="0" role="region" aria-labelledby="streaming-output-label">Streaming output will appear here...</div> |
There was a problem hiding this comment.
The .streaming-output class (defined on line 157) lacks a max-height property. Without a height constraint, this container will expand to fit its content, meaning overflow-y: auto will not trigger an internal scrollbar. This makes the tabindex="0" attribute unnecessary and potentially confusing for keyboard users as the region won't actually be scrollable. Consider adding a max-height to the CSS class or as an inline style to ensure the region functions as an accessible scrollable area.
| <div id="streaming-output" class="streaming-output" tabindex="0" role="region" aria-labelledby="streaming-output-label">Streaming output will appear here...</div> | |
| <div id="streaming-output" class="streaming-output" tabindex="0" role="region" aria-labelledby="streaming-output-label" style="max-height: 300px;">Streaming output will appear here...</div> |
1 participant
💡 What
Converted invalid
<label>tags wrapping non-interactive output regions into styleddivs withids, and addedtabindex="0",role="region", andaria-labelledbyto the scrollable output containers across browser WASM examples.🎯 Why
Using
<label>for non-form controls is invalid HTML. More importantly, the outputdivs useoverflow-y: auto, meaning their content can scroll. Withouttabindex="0", keyboard-only users cannot focus the region to scroll it using arrow keys, making long outputs inaccessible.📸 Before/After
Before: Output regions were unfocusable by keyboard, and used invalid
<label>tags.After: Output regions can receive keyboard focus to allow scrolling, and use valid ARIA labelling semantics for screen readers.
♿ Accessibility
<label>for non-form elements).tabindex="0"to allow keyboard navigation and scrolling of overflowing content.role="region"andaria-labelledbyto properly announce the region's purpose to screen reader users.PR created automatically by Jules for task 11126914773992388177 started by @EffortlessSteven