🎨 Palette: Replace custom interactive divs with native buttons - #604
🎨 Palette: Replace custom interactive divs with native buttons#604sheepdestroyer wants to merge 1 commit 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 GuideThe PR improves semantic and keyboard accessibility in the visualizer by replacing custom or link-based interactions with native buttons, explicitly preventing unintended form submission, and adding CSS resets so the UI remains visually unchanged. Sequence diagram for accessible prompt selectionsequenceDiagram
participant User
participant Visualizer
participant Browser
User->>Browser: Activate native list-item button
Browser->>Visualizer: onclick selectPrompt(index)
Visualizer->>Browser: Render selected prompt details
Browser-->>User: Show selected state and details
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 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments### Comment 1
<locationpath="router/static/visualizer.html"line_range="258-262" />
<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>
**issue (bug_risk):** The generated `<button>` contains `<div>` elements, which are not permitted inside a button. The HTML parser implicitly closes the button before the first `<div>`, leaving the visible list-item content outside the button, so clicking or keyboard-activating the item does not reliably call `selectPrompt(i)`.
**Triggers:** When the browser parses the dynamically assigned `listEl.innerHTML`.
**Suggested fix:** Replace the nested `<div>` elements with `<span>` elements styled as needed, or use a non-button wrapper containing a separate native button.
```suggestion return `<button type="button" class="list-item${i === selectedIdx ? ' selected' : ''}" onclick="selectPrompt(${i})" aria-label="Prompt ${i}"> <span class="id" style="display:block">#${i} · LLM: ${displayLlm}${pred ? ' · CLS: ' + displayPred : ''}</span> <span class="snippet" style="display:block">${escapeHtml(p.prompt.substring(0, 120))}</span> <span class="tags">${tags}</span> </button>`;```
</issue_to_address>Sourcery assessment
Approval pending. 1 finding to address first.
Blocking findings: router/static/visualizer.html:262
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.
issue (bug_risk): The generated <button> contains <div> elements, which are not permitted inside a button. The HTML parser implicitly closes the button before the first <div>, leaving the visible list-item content outside the button, so clicking or keyboard-activating the item does not reliably call selectPrompt(i).
Triggers: When the browser parses the dynamically assigned listEl.innerHTML.
Suggested fix: Replace the nested <div> elements with <span> elements styled as needed, or use a non-button wrapper containing a separate native button.
| 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" style="display:block">#${i} · LLM: ${displayLlm}${pred ? ' · CLS: ' + displayPred : ''}</span> | |
| <spanclass="snippet" style="display:block">${escapeHtml(p.prompt.substring(0, 120))}</span> | |
| <spanclass="tags">${tags}</span> | |
| </button>`; |
💡 What: Replaced an interactive tag (used for the clear action) and a custom
🎯 Why: Native buttons provide out-of-the-box keyboard accessibility (like Space/Enter key support) and semantic meaning for screen readers, unlike tags with href="#" which also cause unintended page jumps.
📸 Before/After: Visuals remain unchanged due to CSS resets applied to the new buttons.
♿ Accessibility: Improved keyboard navigation and screen reader semantics by using native interactive elements.
PR created automatically by Jules for task 11937495021812497139 started by @sheepdestroyer
Summary by Sourcery
Replace custom interactive controls with semantically correct native buttons and ensure all buttons explicitly use button type.
Bug Fixes:
Enhancements:
Chores: