🎨 Palette: Improve interactive element accessibility - #599
🎨 Palette: Improve interactive element accessibility#599sheepdestroyer wants to merge 4 commits into
Conversation
Co-authored-by: sheepdestroyer <1377479+sheepdestroyer@users.noreply.github.com>
👋 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. |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideImproves Palette accessibility by replacing custom role-based list items and JavaScript action links with semantic native buttons, adding CSS resets to preserve the existing appearance while gaining native keyboard and screen-reader behavior. Flow diagram for semantic Palette interactionsflowchart LR
User[User] --> PromptButton[Native prompt button]
PromptButton --> SelectPrompt[selectPrompt]
SelectPrompt --> Visualizer[Updated visualizer]
User --> ClearButton[Native clear button]
ClearButton --> ClearAnnotation[clearAnnotation]
ClearAnnotation --> ReviewState[Updated review state]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 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 |
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments### Comment 1
<locationpath="router/static/visualizer.html"line_range="255-259" />
<code_context>
const displayPred = pred ? escapeHtml(pred.replace('agent-','')) : '';
- return `<divclass="list-item${i === selectedIdx ? ' selected' : ''}"onclick="selectPrompt(${i})"tabindex="0"role="button"aria-label="Prompt ${i}"onkeydown="if(event.key==='Enter'||event.key===''){event.preventDefault();selectPrompt(${i})}">
+ return `<buttontype="button"class="list-item${i === selectedIdx ? ' selected' : ''}"onclick="selectPrompt(${i})"aria-label="Prompt ${i}">
<divclass="id">#${i} · LLM: ${displayLlm}${pred ? ' · CLS: ' + displayPred : ''}</div>
<divclass="snippet">${escapeHtml(p.prompt.substring(0, 120))}</div>
<divclass="tags">${tags}</div>
- </div>`;
+ </button>`;
}).join('');
</code_context>
<issue_to_address>
**nitpick (bug_risk):** Each new `<button>` contains `<div>` children, but a button's content model is phrasing content and `<div>` is flow content. The generated markup is non-conforming HTML and is reported as invalid by HTML validators, undermining the semantic-HTML goal of the change.
**Suggested fix:** Use phrasing elements such as `<span>` for the button's contents, or keep the existing block structure outside the button while styling the button itself.
```suggestion return `<button type="button" class="list-item${i === selectedIdx ? ' selected' : ''}" onclick="selectPrompt(${i})" aria-label="Prompt ${i}"> <span class="id">#${i} · LLM: ${displayLlm}${pred ? ' · CLS: ' + displayPred : ''}</span> <span class="snippet">${escapeHtml(p.prompt.substring(0, 120))}</span> <span class="tags">${tags}</span> </button>`;```
</issue_to_address>
### Comment 2
<locationpath=".Jules/palette.md"line_range="25-27" />
<code_context>
+## 2024-08-26 - Interactive element accessibility in custom lists
</code_context>
<issue_to_address>
**nitpick:** The new 2024-08-26 palette entry is appended after a 2026-08-16 entry, so the learning log is no longer in chronological order and presents its history misleadingly.
**Suggested fix:** Insert the 2024-08-26 entry before the 2026 entries, or update the date if 2024-08-26 is a typo.
</issue_to_address>Sourcery assessment
Approved.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| return `<button type="button" class="list-item${i === selectedIdx ? ' selected' : ''}" onclick="selectPrompt(${i})" aria-label="Prompt ${i}"> | ||
| <div class="id">#${i} · LLM: ${displayLlm}${pred ? ' · CLS: ' + displayPred : ''}</div> | ||
| <div class="snippet">${escapeHtml(p.prompt.substring(0, 120))}</div> | ||
| <div class="tags">${tags}</div> | ||
| </div>`; | ||
| </button>`; |
There was a problem hiding this comment.
nitpick (bug_risk): Each new <button> contains <div> children, but a button's content model is phrasing content and <div> is flow content. The generated markup is non-conforming HTML and is reported as invalid by HTML validators, undermining the semantic-HTML goal of the change.
Suggested fix: Use phrasing elements such as <span> for the button's contents, or keep the existing block structure outside the button while styling the button itself.
| return `<buttontype="button" class="list-item${i === selectedIdx ? ' selected' : ''}" onclick="selectPrompt(${i})" aria-label="Prompt ${i}"> | |
| <divclass="id">#${i} · LLM: ${displayLlm}${pred ? ' · CLS: ' + displayPred : ''}</div> | |
| <divclass="snippet">${escapeHtml(p.prompt.substring(0, 120))}</div> | |
| <divclass="tags">${tags}</div> | |
| </div>`; | |
| </button>`; | |
| return `<buttontype="button" class="list-item${i === selectedIdx ? ' selected' : ''}" onclick="selectPrompt(${i})" aria-label="Prompt ${i}"> | |
| <spanclass="id">#${i} · LLM: ${displayLlm}${pred ? ' · CLS: ' + displayPred : ''}</span> | |
| <spanclass="snippet">${escapeHtml(p.prompt.substring(0, 120))}</span> | |
| <spanclass="tags">${tags}</span> | |
| </button>`; |
| ## 2024-08-26 - Interactive element accessibility in custom lists | ||
| **Learning:** Custom interactive elements that act as buttons (e.g. `div` with `role="button"` and `tabindex="0"`) lack semantic meaning and require manual Javascript event handlers for keyboard events (like `Enter` and `Space`). Additionally, using `<a href="#">` with `onclick` for Javascript actions causes unintended page jumps. | ||
| **Action:** Replace interactive `div` elements with native `<button type="button">` elements and apply CSS resets (`display: block`, `width: 100%`, `text-align: left`, `background: transparent`, `border: none`, `color: inherit`, `font-family: inherit`) to retain visual styling while gaining native accessibility features. Replace Javascript action links (`<a href="#">`) with inline `<button type="button">`. |
There was a problem hiding this comment.
nitpick: The new 2024-08-26 palette entry is appended after a 2026-08-16 entry, so the learning log is no longer in chronological order and presents its history misleadingly.
Suggested fix: Insert the 2024-08-26 entry before the 2026 entries, or update the date if 2024-08-26 is a typo.
Co-authored-by: sheepdestroyer <1377479+sheepdestroyer@users.noreply.github.com>
Co-authored-by: sheepdestroyer <1377479+sheepdestroyer@users.noreply.github.com>
Co-authored-by: sheepdestroyer <1377479+sheepdestroyer@users.noreply.github.com>
💡 What: Replaced custom interactive
divlist items andatags with semantic<button type="button">elements.🎯 Why: Custom
divs withrole="button"andtabindex="0"require manual keydown handlers and don't provide native button behavior. Using<a href="#">for actions causes unexpected scrolling. Using native buttons improves screen reader compatibility and semantic HTML.📸 Before/After: Visually identical, but functionally superior.
♿ Accessibility: Improved keyboard interaction parity and screen reader semantics by using native interactive elements.
PR created automatically by Jules for task 13223841212063358673 started by @sheepdestroyer
Summary by Sourcery
Use native buttons for visualizer interactions to provide consistent keyboard and screen-reader behavior.
Bug Fixes:
Enhancements:
Chores: