feat: improve accessibility with aria-labels on icon buttons#53
Merged
APPLEPIE6969 merged 1 commit intomainfrom Mar 14, 2026
Merged
Conversation
Added `aria-label` attributes to various icon-only buttons across components: - `AuthForm`: Password visibility toggle - `EmptyState`: Action back arrow - `VoiceInput`: Microphone toggle - `Select`: Dropdown toggle and option selections - `AnimatedDropdown`: Dropdown toggle This ensures that screen readers can properly announce the purpose of these interactive elements. Co-authored-by: APPLEPIE6969 <242827480+APPLEPIE6969@users.noreply.github.com>
Contributor
|
👋 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. |
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~4 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan for PR comments
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
components/ui/Select.tsx (1)
54-59:⚠️ Potential issue | 🟠 MajorThis still needs select/listbox semantics, not just labels.
The trigger and options are still exposed as plain buttons, so assistive tech won’t get expanded state or which option is selected. Please add the missing semantics here (
aria-expanded/aria-haspopup="listbox"on the trigger,role="listbox"on the popup, androle="option"+aria-selectedon each option), or switch to a native/headless select primitive.Example of the minimal ARIA shape
<button type="button" onClick={() => setIsOpen(!isOpen)} + aria-haspopup="listbox" + aria-expanded={isOpen} aria-label={`Select option, current value: ${selectedOption?.label || placeholder}`} >- <div className="max-h-64 overflow-y-auto py-1"> + <div role="listbox" className="max-h-64 overflow-y-auto py-1"><button key={option.value} type="button" + role="option" + aria-selected={value === option.value} onClick={() => { onChange(option.value) setIsOpen(false) }} aria-label={`Select ${option.label}`} >Also applies to: 73-98
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/ui/Select.tsx` around lines 54 - 59, The trigger button (onClick toggling setIsOpen, using isOpen, selectedOption, placeholder) lacks ARIA listbox semantics; add aria-expanded={isOpen} and aria-haspopup="listbox" to that button and ensure its aria-label remains, then give the popup container role="listbox" (and manage aria-activedescendant if using roving focus), and each rendered option element should use role="option" plus aria-selected={option.value === selectedOption?.value}; update focus/keyboard handling accordingly or replace with a headless/native select primitive to provide these semantics automatically (ensure references to setIsOpen, isOpen, selectedOption, and placeholder are preserved while adding the attributes).components/VoiceInput.tsx (1)
207-219:⚠️ Potential issue | 🟡 MinorInclude the processing state in the spoken label.
When
isProcessingis true, the button is disabled and shows a spinner, but thearia-labelstill resolves to the idle action. Derive a single state-aware label for bothtitleandaria-labelso processing is announced too.Suggested shape
+ const micLabel = isProcessing + ? t("tutor.processing") || "Processing audio" + : isRecording + ? t("tutor.stop") || "Stop recording" + : lastError + ? "Microphone Access Blocked" + : t("tutor.voice_mode") || "Voice mode" <button onClick={handleMicClick} disabled={disabled || isProcessing} - title={isRecording ? t("tutor.stop") : lastError ? "Microphone Access Blocked" : t("tutor.voice_mode")} - aria-label={isRecording ? t("tutor.stop") || "Stop recording" : lastError ? "Microphone Access Blocked" : t("tutor.voice_mode") || "Voice mode"} + title={micLabel} + aria-label={micLabel} + aria-busy={isProcessing} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/VoiceInput.tsx` around lines 207 - 219, Create a single state-aware label in the VoiceInput component (e.g., compute a const like buttonLabel) that reflects isProcessing, isRecording, lastError, and diagInfo.permissionState; use that one label for both the button's title and aria-label so the “processing” state is announced (e.g., when isProcessing true return a localized "Processing" or "Recording stopped — processing" string via t(), otherwise return the existing "Stop recording", "Microphone Access Blocked", or "Voice mode" variants). Update the JSX button to reference this computed label instead of building title and aria-label separately; ensure the logic covers isProcessing first, then isRecording, then error/permission cases, and falls back to the normal voice mode text.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/AnimatedDropdown.tsx`:
- Around line 40-45: The trigger button in AnimatedDropdown uses
selectedOption?.label directly, causing empty visible text and an incomplete
aria-label when no option matches; update the button to use a non-empty fallback
(e.g., "Select option" or "No selection") for both the visible label and the
aria-label—replace selectedOption?.label with a fallbackLabel variable or inline
fallback in the JSX (refer to AnimatedDropdown, setIsOpen, and selectedOption)
so the button always renders readable text and a complete aria-label like
`Toggle dropdown, current selection: ${fallbackLabel}`.
In `@components/AuthForm.tsx`:
- Around line 87-89: In the AuthForm component, wire up a showPassword boolean
state (e.g., useState in AuthForm) and toggle it from the password-visibility
button: add an onClick handler on the button that flips showPassword, set the
password input (id="password") type to "text" when showPassword is true else
"password", update the button icon text to "visibility" vs "visibility_off"
accordingly, and expose the pressed state via aria-pressed (and update
aria-label to reflect current action like "Show password" / "Hide password") so
the control truly functions as a toggle.
---
Outside diff comments:
In `@components/ui/Select.tsx`:
- Around line 54-59: The trigger button (onClick toggling setIsOpen, using
isOpen, selectedOption, placeholder) lacks ARIA listbox semantics; add
aria-expanded={isOpen} and aria-haspopup="listbox" to that button and ensure its
aria-label remains, then give the popup container role="listbox" (and manage
aria-activedescendant if using roving focus), and each rendered option element
should use role="option" plus aria-selected={option.value ===
selectedOption?.value}; update focus/keyboard handling accordingly or replace
with a headless/native select primitive to provide these semantics automatically
(ensure references to setIsOpen, isOpen, selectedOption, and placeholder are
preserved while adding the attributes).
In `@components/VoiceInput.tsx`:
- Around line 207-219: Create a single state-aware label in the VoiceInput
component (e.g., compute a const like buttonLabel) that reflects isProcessing,
isRecording, lastError, and diagInfo.permissionState; use that one label for
both the button's title and aria-label so the “processing” state is announced
(e.g., when isProcessing true return a localized "Processing" or "Recording
stopped — processing" string via t(), otherwise return the existing "Stop
recording", "Microphone Access Blocked", or "Voice mode" variants). Update the
JSX button to reference this computed label instead of building title and
aria-label separately; ensure the logic covers isProcessing first, then
isRecording, then error/permission cases, and falls back to the normal voice
mode text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 28a8664f-7feb-459d-999e-3459117a4067
📒 Files selected for processing (5)
components/AnimatedDropdown.tsxcomponents/AuthForm.tsxcomponents/EmptyState.tsxcomponents/VoiceInput.tsxcomponents/ui/Select.tsx
Comment on lines
40
to
45
| <button | ||
| onClick={() => setIsOpen(!isOpen)} | ||
| className="flex items-center gap-2 rounded-lg bg-slate-100 px-3 py-2 text-xs font-medium text-slate-600 transition-all hover:bg-slate-200 focus:outline-none focus:ring-2 focus:ring-primary/50 dark:bg-[#1a1622] dark:text-[#a69db9] dark:hover:bg-[#2e2839]" | ||
| aria-label={`Toggle dropdown, current selection: ${selectedOption?.label || ''}`} | ||
| > | ||
| {selectedOption?.label} |
There was a problem hiding this comment.
Use a non-empty fallback when value no longer matches an option.
If selectedOption is undefined, this becomes Toggle dropdown, current selection: and the trigger renders as icon-only. Please give both the visible label and the aria-label a real fallback such as “Select option” or “No selection”.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/AnimatedDropdown.tsx` around lines 40 - 45, The trigger button in
AnimatedDropdown uses selectedOption?.label directly, causing empty visible text
and an incomplete aria-label when no option matches; update the button to use a
non-empty fallback (e.g., "Select option" or "No selection") for both the
visible label and the aria-label—replace selectedOption?.label with a
fallbackLabel variable or inline fallback in the JSX (refer to AnimatedDropdown,
setIsOpen, and selectedOption) so the button always renders readable text and a
complete aria-label like `Toggle dropdown, current selection: ${fallbackLabel}`.
Comment on lines
87
to
89
| <input className="w-full h-12 pl-11 pr-4 bg-gray-50 dark:bg-[#1f1c27] border border-gray-200 dark:border-[#433b54] rounded-xl text-slate-900 dark:text-white text-sm focus:outline-none focus:ring-2 focus:ring-primary/50 focus:border-primary transition-all placeholder:text-gray-400" id="password" placeholder="••••••••" type="password" /> | ||
| <button className="absolute inset-y-0 right-0 pr-3.5 flex items-center text-gray-400 hover:text-white transition-colors" type="button"> | ||
| <button aria-label="Toggle password visibility" className="absolute inset-y-0 right-0 pr-3.5 flex items-center text-gray-400 hover:text-white transition-colors" type="button"> | ||
| <span className="material-symbols-outlined text-[20px]">visibility_off</span> |
There was a problem hiding this comment.
Don’t label this as a toggle until it actually toggles.
Right now the button is focusable and announced as a password-visibility control, but it never changes the input type or icon. Please wire a showPassword state into the input, add an onClick, and expose the pressed state.
Minimal fix
+ const [showPassword, setShowPassword] = useState(false)- <input className="w-full h-12 pl-11 pr-4 bg-gray-50 dark:bg-[`#1f1c27`] border border-gray-200 dark:border-[`#433b54`] rounded-xl text-slate-900 dark:text-white text-sm focus:outline-none focus:ring-2 focus:ring-primary/50 focus:border-primary transition-all placeholder:text-gray-400" id="password" placeholder="••••••••" type="password" />
- <button aria-label="Toggle password visibility" className="absolute inset-y-0 right-0 pr-3.5 flex items-center text-gray-400 hover:text-white transition-colors" type="button">
- <span className="material-symbols-outlined text-[20px]">visibility_off</span>
+ <input className="w-full h-12 pl-11 pr-4 bg-gray-50 dark:bg-[`#1f1c27`] border border-gray-200 dark:border-[`#433b54`] rounded-xl text-slate-900 dark:text-white text-sm focus:outline-none focus:ring-2 focus:ring-primary/50 focus:border-primary transition-all placeholder:text-gray-400" id="password" placeholder="••••••••" type={showPassword ? "text" : "password"} />
+ <button
+ aria-label={showPassword ? "Hide password" : "Show password"}
+ aria-pressed={showPassword}
+ onClick={() => setShowPassword(v => !v)}
+ className="absolute inset-y-0 right-0 pr-3.5 flex items-center text-gray-400 hover:text-white transition-colors"
+ type="button"
+ >
+ <span className="material-symbols-outlined text-[20px]">{showPassword ? "visibility" : "visibility_off"}</span>
</button>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/AuthForm.tsx` around lines 87 - 89, In the AuthForm component,
wire up a showPassword boolean state (e.g., useState in AuthForm) and toggle it
from the password-visibility button: add an onClick handler on the button that
flips showPassword, set the password input (id="password") type to "text" when
showPassword is true else "password", update the button icon text to
"visibility" vs "visibility_off" accordingly, and expose the pressed state via
aria-pressed (and update aria-label to reflect current action like "Show
password" / "Hide password") so the control truly functions as a toggle.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Improved accessibility across the application by adding
aria-labelattributes to icon-only buttons. Screen readers will now be able to correctly interpret the functionality of these buttons. Components updated: AuthForm, EmptyState, VoiceInput, Select, and AnimatedDropdown.PR created automatically by Jules for task 14217599385858934600 started by @APPLEPIE6969
Summary by CodeRabbit