🎨 Palette: Add accessible labels to navigation buttons in DiffPlayback - #1
🎨 Palette: Add accessible labels to navigation buttons in DiffPlayback#1Wenbobobo wants to merge 1 commit into
Conversation
Co-authored-by: Wenbobobo <[email protected]>
|
👋 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. |
There was a problem hiding this comment.
Pull request overview
This PR improves the DiffPlayback UI’s accessibility by giving the step navigation buttons accessible names (and tooltips), and includes a few small TypeScript build-cleanups for unused parameters / optional property access.
Changes:
- Add
aria-labelandtitleto the Previous/Next step buttons inDiffPlayback. - Adjust
DiffPlaybackto avoid a potentially-undefined property access (currentEntry?.file). - Silence unused-parameter TypeScript build errors by prefixing unused callback args with
_.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| ui/client/src/views/DiffPlayback.tsx | Adds accessible labels/tooltips to step nav buttons; tweaks toFile prop passing. |
| ui/client/src/utils/markdown.ts | Renames unused regex capture to _lang to satisfy TS unused-parameter checks. |
| ui/client/src/hooks/useLogStream.ts | Prefixes unused error/event parameters with _ to satisfy TS unused-parameter checks. |
| .Jules/palette.md | Adds a short note documenting the accessibility learning/action. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fromFile={compareMode === 'previous' | ||
| ? (currentIdx === 0 ? '(initial)' : timeline![currentIdx - 1]?.file) | ||
| : (compareEntry?.file || '(initial)')} | ||
| toFile={currentEntry.file} | ||
| toFile={currentEntry?.file || ""} | ||
| addedLines={displayDiff.addedLines} |
There was a problem hiding this comment.
toFile={currentEntry?.file || ""} can hide the diff header (the fromFile → toFile label only renders when both are truthy) and also makes the UI behavior depend on an empty-string fallback rather than the actual state. Since DiffView is only meaningful when currentEntry exists, consider tightening the render condition (e.g., gate on currentEntry before rendering DiffView) or otherwise ensure a real filename is passed instead of "" (or pass undefined if you truly want to omit it).
| let result = content.replace(/```(\w*)\n([\s\S]*?)```/g, (_, _lang, code) => { | ||
| codeBlocks.push(`<pre><code>${code}</code></pre>`); | ||
| return `\x00CODE${codeBlocks.length - 1}\x00`; | ||
| }); |
There was a problem hiding this comment.
markdownToHtml interpolates unescaped user-controlled text into HTML (e.g., code blocks and table cells) and is rendered via dangerouslySetInnerHTML elsewhere, which makes this an XSS vector if the markdown content can contain <script>/HTML. Please HTML-escape raw text before wrapping it in tags, or switch to a vetted markdown renderer + sanitizer (e.g., DOMPurify) so only an allowlisted set of tags/attributes can reach the DOM.
💡 What: Added
aria-labelandtitleattributes to the Previous (◀) and Next (▶) step buttons in theDiffPlaybackcomponent. Also fixed several build errors related to unused variables (_e,_ev,_lang) and potentially undefined object properties (currentEntry?.file).🎯 Why: To make the
DiffPlaybackcomponent more intuitive and accessible. Text symbols like '◀' and '▶' are not well-supported by all screen readers, leaving visually impaired users without context. Addingaria-labels ensures correct parsing by assistive technologies, andtitleattributes provide helpful tooltips for sighted users.📸 Before/After: Sighted users will now see a tooltip displaying "Previous step" or "Next step" when hovering over the navigation buttons. Screen reader users will hear "Previous step button" instead of an unhelpful symbol description.
♿ Accessibility: Ensures that icon-only/symbol-only interactive elements possess accessible names, complying with WCAG 2.1 SC 4.1.2 (Name, Role, Value).
PR created automatically by Jules for task 15530259999417076477 started by @Wenbobobo