🎨 Palette: Improve accessibility and focus states of AgentCard icon buttons#80
🎨 Palette: Improve accessibility and focus states of AgentCard icon buttons#80bobdivx wants to merge 1 commit into
Conversation
Co-authored-by: bobdivx <6737167+bobdivx@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. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![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 formats the AgentCard.tsx component to use double quotes and multi-line formatting, while also introducing accessibility enhancements such as focus rings and ARIA attributes on several interactive elements. The review feedback highlights further opportunities to improve accessibility and correctness, specifically by adding visible focus states to the main card link and the model selector dropdown, providing an accessible label for the model selector, and explicitly setting type="button" on the action and configuration buttons to prevent accidental form submissions.
| <a | ||
| href={swarmHref} | ||
| class="block flex-1 p-4 pb-3 text-inherit no-underline" | ||
| > |
There was a problem hiding this comment.
Accessibility: Missing Focus State on Main Card Link
While focus states have been added to the icon buttons, the main card link (<a>) still lacks a visible focus indicator. When a keyboard user navigates through the cards, there is no visual feedback showing which card is currently focused.
Adding standard focus-visible styles to the link will greatly improve keyboard navigation.
| <a | |
| href={swarmHref} | |
| class="block flex-1 p-4 pb-3 text-inherit no-underline" | |
| > | |
| <a | |
| href={swarmHref} | |
| class="block flex-1 p-4 pb-3 text-inherit no-underline focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#175B37] focus-visible:rounded-2xl" | |
| > |
| <button | ||
| key={cmd} | ||
| onClick={() => onSwarmCommand?.(agent.id, cmd)} | ||
| disabled={commandBusy} | ||
| class={`rounded-xl border p-2 transition-all disabled:opacity-50 ${cmd === 'start_work' ? 'bg-gray-900 border-gray-900 text-white hover:bg-black' : 'bg-white border-gray-200 text-gray-400 hover:text-rose-500 hover:border-rose-200'}`} | ||
| title={cmd === 'start_work' ? 'Lancer une mission' : 'Arrêter'} | ||
| class={`rounded-xl border p-2 transition-all disabled:opacity-50 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#175B37] ${cmd === "start_work" ? "bg-gray-900 border-gray-900 text-white hover:bg-black" : "bg-white border-gray-200 text-gray-400 hover:text-rose-500 hover:border-rose-200"}`} | ||
| title={cmd === "start_work" ? "Lancer une mission" : "Arrêter"} | ||
| aria-label={ | ||
| cmd === "start_work" ? "Lancer une mission" : "Arrêter" | ||
| } | ||
| > |
There was a problem hiding this comment.
Correctness: Missing type="button" on Swarm Command Buttons
In HTML, the default type for a <button> element is "submit". If this card component is ever rendered inside a <form>, clicking these buttons will trigger an unintended form submission.
Explicitly setting type="button" prevents this behavior and is a recommended best practice.
| <button | |
| key={cmd} | |
| onClick={() => onSwarmCommand?.(agent.id, cmd)} | |
| disabled={commandBusy} | |
| class={`rounded-xl border p-2 transition-all disabled:opacity-50 ${cmd === 'start_work' ? 'bg-gray-900 border-gray-900 text-white hover:bg-black' : 'bg-white border-gray-200 text-gray-400 hover:text-rose-500 hover:border-rose-200'}`} | |
| title={cmd === 'start_work' ? 'Lancer une mission' : 'Arrêter'} | |
| class={`rounded-xl border p-2 transition-all disabled:opacity-50 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#175B37] ${cmd === "start_work" ? "bg-gray-900 border-gray-900 text-white hover:bg-black" : "bg-white border-gray-200 text-gray-400 hover:text-rose-500 hover:border-rose-200"}`} | |
| title={cmd === "start_work" ? "Lancer une mission" : "Arrêter"} | |
| aria-label={ | |
| cmd === "start_work" ? "Lancer une mission" : "Arrêter" | |
| } | |
| > | |
| <button | |
| key={cmd} | |
| type="button" | |
| onClick={() => onSwarmCommand?.(agent.id, cmd)} | |
| disabled={commandBusy} | |
| class={`rounded-xl border p-2 transition-all disabled:opacity-50 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#175B37] ${cmd === "start_work" ? "bg-gray-900 border-gray-900 text-white hover:bg-black" : "bg-white border-gray-200 text-gray-400 hover:text-rose-500 hover:border-rose-200"}`} | |
| title={cmd === "start_work" ? "Lancer une mission" : "Arrêter"} | |
| aria-label={ | |
| cmd === "start_work" ? "Lancer une mission" : "Arrêter" | |
| } | |
| > |
| setConfigOpen(true); | ||
| }} | ||
| class="rounded-xl border border-gray-200 bg-white p-2 text-gray-400 transition-all hover:border-[#175B37]/30 hover:text-[#175B37]" | ||
| class="rounded-xl border border-gray-200 bg-white p-2 text-gray-400 transition-all hover:border-[#175B37]/30 hover:text-[#175B37] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#175B37]" |
There was a problem hiding this comment.
Correctness: Missing type="button" on Configuration Button
Similar to the swarm command buttons, the configuration button lacks an explicit type="button", which defaults its behavior to "submit" and can cause unexpected form submissions if placed inside a <form>.
| class="rounded-xl border border-gray-200 bg-white p-2 text-gray-400 transition-all hover:border-[#175B37]/30 hover:text-[#175B37] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#175B37]" | |
| type="button" | |
| class="rounded-xl border border-gray-200 bg-white p-2 text-gray-400 transition-all hover:border-[#175B37]/30 hover:text-[#175B37] focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#175B37]" |
| <div class="mt-0.5 max-w-[145px]" onClick={(e) => e.preventDefault()}> | ||
| <select | ||
| class="max-w-full cursor-pointer appearance-none truncate border-none bg-transparent pr-2 text-right font-mono text-[9px] text-gray-500 outline-none hover:text-gray-900" | ||
| value={agent.model || "Auto"} | ||
| onChange={(e) => onModelChange?.(agent.id, (e.currentTarget as HTMLSelectElement).value)} | ||
| onChange={(e) => | ||
| onModelChange?.( | ||
| agent.id, | ||
| (e.currentTarget as HTMLSelectElement).value, | ||
| ) | ||
| } | ||
| title="Modifier le modèle" | ||
| > | ||
| <option value={agent.model}>{agent.model || "Modèle"}</option> |
There was a problem hiding this comment.
Accessibility: Missing Focus State and Accessible Label on Model Selector
The model selection <select> element has outline-none but lacks any focus-visible styles, making it invisible to keyboard navigators when focused. Additionally, it lacks an aria-label to provide programmatic context to screen readers.
Adding a focus ring and aria-label will ensure full accessibility compliance for this interactive control.
| <div class="mt-0.5 max-w-[145px]" onClick={(e) => e.preventDefault()}> | |
| <select | |
| class="max-w-full cursor-pointer appearance-none truncate border-none bg-transparent pr-2 text-right font-mono text-[9px] text-gray-500 outline-none hover:text-gray-900" | |
| value={agent.model || "Auto"} | |
| onChange={(e) => onModelChange?.(agent.id, (e.currentTarget as HTMLSelectElement).value)} | |
| onChange={(e) => | |
| onModelChange?.( | |
| agent.id, | |
| (e.currentTarget as HTMLSelectElement).value, | |
| ) | |
| } | |
| title="Modifier le modèle" | |
| > | |
| <option value={agent.model}>{agent.model || "Modèle"}</option> | |
| <div class="mt-0.5 max-w-[145px]" onClick={(e) => e.preventDefault()}> | |
| <select | |
| class="max-w-full cursor-pointer appearance-none truncate border-none bg-transparent pr-2 text-right font-mono text-[9px] text-gray-500 outline-none hover:text-gray-900 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#175B37] focus-visible:rounded" | |
| value={agent.model || "Auto"} | |
| onChange={(e) => | |
| onModelChange?.( | |
| agent.id, | |
| (e.currentTarget as HTMLSelectElement).value, | |
| ) | |
| } | |
| title="Modifier le modèle" | |
| aria-label="Modifier le modèle" | |
| > | |
| <option value={agent.model}>{agent.model || "Modèle"}</option> |
1 participant
💡 What: Added
aria-labelattributes to the icon-only buttons inAgentCard.tsx(Start, Stop, Configure), along witharia-hidden='true'on their purely decorative inner SVGs. Addedfocus-visibleclasses to render the standard green focus ring for keyboard navigation.🎯 Why: To improve accessibility for screen reader users and keyboard navigators. Previously, these icon-only buttons lacked programmatic names, meaning screen readers would provide no context, and they lacked distinct focus indicators, making keyboard navigation difficult.
📸 Before/After: The visual layout is unchanged for pointer users, but keyboard focus now triggers a highly visible green ring (
#175B37). Screen readers now correctly announce 'Lancer une mission', 'Arrêter', or 'Configurer cet agent'.♿ Accessibility: Solves a major accessibility compliance issue regarding icon-only interactive controls and keyboard focus visibility.
PR created automatically by Jules for task 17503076025238926114 started by @bobdivx