🎨 Palette: [UX improvement] Add ARIA labels to icon-only buttons#31
🎨 Palette: [UX improvement] Add ARIA labels to icon-only buttons#31bobdivx wants to merge 1 commit into
Conversation
Co-authored-by: bobdivx <6737167+bobdivx@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
👋 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.
Code Review
This pull request enhances accessibility and focus visibility for buttons in the DiscussionHeader and OllamaTab components by adding ARIA attributes and focus-visible styles. Review feedback suggests further improving button robustness and consistency by explicitly setting type="button", ensuring title attributes are as descriptive as aria-label, and using rounded-full classes. Additionally, it is recommended to move hardcoded hex colors to the Tailwind configuration and verify that menu containers have the appropriate ARIA roles to match the aria-haspopup attribute.
| {inst.enabled ? 'Désactiver' : 'Activer'} | ||
| </button> | ||
| <button onClick={() => startEdit(inst)} class="p-2 text-gray-400 hover:text-gray-600 transition-colors"> | ||
| <button onClick={() => startEdit(inst)} class="p-2 text-gray-400 hover:text-gray-600 transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-blue-500 rounded" aria-label={`Modifier l'instance ${inst.name}`} title="Modifier l'instance"> |
There was a problem hiding this comment.
The title attribute should be as descriptive as the aria-label to provide the same context to sighted users on hover. Additionally, it's a best practice to include type="button" to prevent any potential form submission issues, and using rounded-full would maintain visual consistency with other icon buttons in the application.
| <button onClick={() => startEdit(inst)} class="p-2 text-gray-400 hover:text-gray-600 transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-blue-500 rounded" aria-label={`Modifier l'instance ${inst.name}`} title="Modifier l'instance"> | |
| <button type="button" onClick={() => startEdit(inst)} class="p-2 text-gray-400 hover:text-gray-600 transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-blue-500 rounded-full" aria-label={`Modifier l'instance ${inst.name}`} title={`Modifier l'instance ${inst.name}`}> |
| <svg class="w-4 h-4" fill="none" viewBox="0 0 24 24" stroke="currentColor"><path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M11 5H6a2 2 0 00-2 2v11a2 2 0 002 2h11a2 2 0 002-2v-5m-1.414-9.414a2 2 0 112.828 2.828L11.828 15H9v-2.828l8.586-8.586z" /></svg> | ||
| </button> | ||
| <button onClick={() => deleteInstance(inst.id)} class="p-2 text-gray-400 hover:text-red-600 transition-colors"> | ||
| <button onClick={() => deleteInstance(inst.id)} class="p-2 text-gray-400 hover:text-red-600 transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-red-500 rounded" aria-label={`Supprimer l'instance ${inst.name}`} title="Supprimer l'instance"> |
There was a problem hiding this comment.
Similar to the edit button, the delete button's title should be updated to include the instance name for better clarity. Adding type="button" and using rounded-full will also improve consistency and robustness.
| <button onClick={() => deleteInstance(inst.id)} class="p-2 text-gray-400 hover:text-red-600 transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-red-500 rounded" aria-label={`Supprimer l'instance ${inst.name}`} title="Supprimer l'instance"> | |
| <button type="button" onClick={() => deleteInstance(inst.id)} class="p-2 text-gray-400 hover:text-red-600 transition-colors focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-red-500 rounded-full" aria-label={`Supprimer l'instance ${inst.name}`} title={`Supprimer l'instance ${inst.name}`}> |
| <button | ||
| type="button" | ||
| class="flex h-11 w-11 items-center justify-center rounded-full border border-gray-200 text-gray-500 transition hover:bg-gray-50 hover:text-gray-800" | ||
| class="flex h-11 w-11 items-center justify-center rounded-full border border-gray-200 text-gray-500 transition hover:bg-gray-50 hover:text-gray-800 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#175B37]" |
| class="flex h-11 w-11 items-center justify-center rounded-full border border-gray-200 text-gray-500 transition hover:bg-gray-50 hover:text-gray-800 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-[#175B37]" | ||
| aria-label="Menu d'options" | ||
| aria-expanded={headerMenuOpen} | ||
| aria-haspopup="menu" |
There was a problem hiding this comment.
Using aria-haspopup="menu" indicates to assistive technologies that this button triggers a menu. For this to be fully accessible, the target container (at line 127) should have role="menu" and its children should have role="menuitem". If the popup is not a formal menu, aria-haspopup="true" might be more appropriate.
💡 What: Added
aria-label,aria-expanded,aria-haspopup, andtitleattributes to icon-only buttons inDiscussionHeader.tsxandOllamaTab.tsx. Also added keyboard focus states (focus-visible:ring-2) to these buttons.🎯 Why: Icon-only buttons without labels are inaccessible to screen readers. Focus states improve keyboard navigation.
📸 Before/After: Focus outline now clearly visible when navigating via keyboard. Screen readers now announce button actions.
♿ Accessibility: Improved screen reader and keyboard accessibility for critical interaction points.
PR created automatically by Jules for task 5523120068416955368 started by @bobdivx