a11y: screen reader and keyboard improvements#243
Open
ways2read wants to merge 2 commits intojamiepine:mainfrom
Open
a11y: screen reader and keyboard improvements#243ways2read wants to merge 2 commits intojamiepine:mainfrom
ways2read wants to merge 2 commits intojamiepine:mainfrom
Conversation
- Audio player: aria-labels for Play/Pause, Loop, Mute, Close; labelled playback and volume sliders - Generation: aria-labels for Generate speech and Fine tune instructions buttons - Voice cards: focusable, labelled, Enter/Space to select - History rows: focusable, labelled, Enter/Space to play; transcript textarea labelled - Voices tab: focusable rows, labelled, Enter/Space to edit; Actions button labelled - Model management: focusable model rows and labelled Download/Delete buttons - Server tab: regions with aria-label and tabIndex for Connection, Status, App Updates - Stories: focusable story rows, labelled, Enter/Space to select; Actions and track editor buttons labelled - Voice profile samples: Play/Pause/Stop and mini-player slider labelled Tested with NVDA and Narrator on Windows. See docs/PR-ACCESSIBILITY.md for full description. Made-with: Cursor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds accessibility attributes and keyboard interactions across the app: ARIA labels/roles, tabIndex, aria-valuetext, and onKeyDown handlers for playback, generation, history, voices, stories, server settings, and model management. No exported APIs or core control flow were changed. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 5
🧹 Nitpick comments (1)
app/src/components/Generation/FloatingGenerateBox.tsx (1)
335-353: Consider usingaria-pressedfor toggle state.The current label approach works, but for toggle buttons,
aria-pressedis the semantic standard that screen readers specifically announce as "pressed/not pressed."♻️ Optional: Use aria-pressed for toggle semantics
<Button type="button" variant="ghost" size="icon" onClick={() => setIsInstructMode(!isInstructMode)} className={cn( 'h-10 w-10 rounded-full transition-all duration-200', isInstructMode ? 'bg-accent text-accent-foreground border border-accent hover:bg-accent/90' : 'bg-card border border-border hover:bg-background/50', )} - aria-label={ - isInstructMode - ? 'Fine tune instructions, on' - : 'Fine tune instructions' - } + aria-label="Fine tune instructions" + aria-pressed={isInstructMode} >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/src/components/Generation/FloatingGenerateBox.tsx` around lines 335 - 353, The toggle Button in FloatingGenerateBox.tsx currently uses aria-label to indicate state; update the Button element (the component rendering SlidersHorizontal and using isInstructMode and setIsInstructMode) to include the boolean aria-pressed={isInstructMode} so screen readers get proper toggle semantics, while keeping or adjusting aria-label if desired; ensure the onClick still calls setIsInstructMode(!isInstructMode) and that the aria-pressed value is derived from the same isInstructMode state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/src/components/History/HistoryTable.tsx`:
- Around line 275-282: The row-level onKeyDown handler currently only ignores
textarea children so Enter/Space pressed on action controls still triggers
handlePlay; update the guard in that onKeyDown to detect and return early when
the event target is inside any interactive child (e.g., closest('input, button,
select, textarea, a, [role="button"]') or a custom marker like
[data-ignore-row-shortcut]) so keyboard interactions on child controls don’t
bubble to the row and call handlePlay(gen.id, gen.text, gen.profile_id).
In `@app/src/components/StoriesTab/StoryList.tsx`:
- Around line 197-211: The row is keyboard-selectable but lacks programmatic
state for assistive tech; update the element in StoryList.tsx (the container
using role="button", selectedStoryId and setSelectedStoryId) to expose the
active story—add an appropriate ARIA attribute such as
aria-pressed={selectedStoryId === story.id} or aria-current={selectedStoryId ===
story.id} (or convert to listbox/option roles if you prefer that pattern), and
ensure the attribute updates when setSelectedStoryId is called so screen readers
can announce which story is selected.
In `@app/src/components/VoiceProfiles/ProfileCard.tsx`:
- Around line 64-69: The onKeyDown handler in ProfileCard (handleKeyDown) is
firing for nested action controls (Export/Edit/Delete) because the event
bubbles; update handleKeyDown to ignore events originating from interactive
descendants by checking the event target (e.g., if (e.target as
HTMLElement).closest('button, a, [role="button"], input, textarea, select,
[tabindex]') return;) before calling e.preventDefault() and handleSelect();
apply the same guard to the similar handler used around lines 82-87.
In `@app/src/components/VoicesTab/VoicesTab.tsx`:
- Around line 198-205: Remove the ARIA role on the table row: do not set
role="button" on the TableRow (which renders a <tr>) and instead place an
interactive element inside a cell; keep the existing onClick/onKeyDown logic but
move the clickable target into a focusable control (e.g., a <button> or <a>
inside the relevant TableCell) that calls onEdit and uses handleKeyDown, and use
rowLabel for the inner control's aria-label so table semantics remain intact
while preserving keyboard accessibility.
In `@docs/PR-ACCESSIBILITY.md`:
- Line 21: Update the two bullets to use the requested wording: change the
phrase "Fine tune instructions (sliders)" to "Fine-tune instructions (sliders)"
and change the other bullet's phrasing to include "focus on the text area"
(replace the existing wording on that line with a variant that says e.g. "when
opened, focus on the text area"). Locate the lines containing the current
bullets (the one mentioning "Generate speech (submit) and Fine tune instructions
(sliders) – Icon buttons now have `aria-label`..." and the other bullet around
the sentence that should become "focus on the text area") and replace the
phrasing accordingly.
---
Nitpick comments:
In `@app/src/components/Generation/FloatingGenerateBox.tsx`:
- Around line 335-353: The toggle Button in FloatingGenerateBox.tsx currently
uses aria-label to indicate state; update the Button element (the component
rendering SlidersHorizontal and using isInstructMode and setIsInstructMode) to
include the boolean aria-pressed={isInstructMode} so screen readers get proper
toggle semantics, while keeping or adjusting aria-label if desired; ensure the
onClick still calls setIsInstructMode(!isInstructMode) and that the aria-pressed
value is derived from the same isInstructMode state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5fcc3ff3-64b9-4a3e-aed4-21ca59cfd26a
📒 Files selected for processing (16)
app/src/components/AudioPlayer/AudioPlayer.tsxapp/src/components/Generation/FloatingGenerateBox.tsxapp/src/components/History/HistoryTable.tsxapp/src/components/ServerSettings/ConnectionForm.tsxapp/src/components/ServerSettings/ModelManagement.tsxapp/src/components/ServerSettings/ServerStatus.tsxapp/src/components/ServerSettings/UpdateStatus.tsxapp/src/components/StoriesTab/StoryList.tsxapp/src/components/StoriesTab/StoryTrackEditor.tsxapp/src/components/VoiceProfiles/AudioSampleRecording.tsxapp/src/components/VoiceProfiles/AudioSampleSystem.tsxapp/src/components/VoiceProfiles/AudioSampleUpload.tsxapp/src/components/VoiceProfiles/ProfileCard.tsxapp/src/components/VoiceProfiles/SampleList.tsxapp/src/components/VoicesTab/VoicesTab.tsxdocs/PR-ACCESSIBILITY.md
Comment on lines
+275
to
+282
| onKeyDown={(e) => { | ||
| const target = e.target as HTMLElement; | ||
| if (target.closest('textarea')) return; | ||
| if (e.key === 'Enter' || e.key === ' ') { | ||
| e.preventDefault(); | ||
| handlePlay(gen.id, gen.text, gen.profile_id); | ||
| } | ||
| }} |
There was a problem hiding this comment.
Skip the row shortcut when focus is inside child controls.
onKeyDown only excludes the transcript, so Enter/Space on the Actions trigger will also play or restart the sample. The mouse path is protected with stopPropagation(), but the keyboard path still bubbles to the row handler.
💡 Minimal guard
onKeyDown={(e) => {
- const target = e.target as HTMLElement;
- if (target.closest('textarea')) return;
+ if (e.target !== e.currentTarget) return;
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
handlePlay(gen.id, gen.text, gen.profile_id);
}
}}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| onKeyDown={(e) => { | |
| const target = e.target as HTMLElement; | |
| if (target.closest('textarea')) return; | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| handlePlay(gen.id, gen.text, gen.profile_id); | |
| } | |
| }} | |
| onKeyDown={(e) => { | |
| if (e.target !== e.currentTarget) return; | |
| if (e.key === 'Enter' || e.key === ' ') { | |
| e.preventDefault(); | |
| handlePlay(gen.id, gen.text, gen.profile_id); | |
| } | |
| }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/History/HistoryTable.tsx` around lines 275 - 282, The
row-level onKeyDown handler currently only ignores textarea children so
Enter/Space pressed on action controls still triggers handlePlay; update the
guard in that onKeyDown to detect and return early when the event target is
inside any interactive child (e.g., closest('input, button, select, textarea, a,
[role="button"]') or a custom marker like [data-ignore-row-shortcut]) so
keyboard interactions on child controls don’t bubble to the row and call
handlePlay(gen.id, gen.text, gen.profile_id).
Comment on lines
+64
to
+69
| const handleKeyDown = (e: React.KeyboardEvent) => { | ||
| if (e.key === 'Enter' || e.key === ' ') { | ||
| e.preventDefault(); | ||
| handleSelect(); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Keep row-level key handling off the nested action buttons.
Because onKeyDown is attached to the whole card, pressing Enter/Space on Export/Edit/Delete will bubble up and also run handleSelect(). That makes the inner actions unexpectedly select/deselect the profile for keyboard users.
💡 Minimal guard
const handleKeyDown = (e: React.KeyboardEvent) => {
+ if (e.target !== e.currentTarget) {
+ return;
+ }
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
handleSelect();
}
};Also applies to: 82-87
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/src/components/VoiceProfiles/ProfileCard.tsx` around lines 64 - 69, The
onKeyDown handler in ProfileCard (handleKeyDown) is firing for nested action
controls (Export/Edit/Delete) because the event bubbles; update handleKeyDown to
ignore events originating from interactive descendants by checking the event
target (e.g., if (e.target as HTMLElement).closest('button, a, [role="button"],
input, textarea, select, [tabindex]') return;) before calling e.preventDefault()
and handleSelect(); apply the same guard to the similar handler used around
lines 82-87.
- HistoryTable: skip row key handler when focus is on Actions button (Enter/Space) - StoryList: expose selected story (aria-pressed, 'Selected' in label) - ProfileCard: skip card key handler when focus is on Export/Edit/Delete - VoicesTab: keep table semantics; edit button in first cell instead of role=button on row - PR-ACCESSIBILITY.md: 'Fine-tune' wording, 'focus on the text area' phrasing Made-with: Cursor
|
add hindi language |
Author
|
I wonder if you meant to post this request elsewhere. If the comment relates to screen reader or keyboard support, please explain.
…________________________________
From: Vishaldadlani321 ***@***.***>
Sent: Sunday, March 8, 2026 4:52:30 PM
To: jamiepine/voicebox ***@***.***>
Cc: Richard Orme ***@***.***>; Author ***@***.***>
Subject: Re: [jamiepine/voicebox] a11y: screen reader and keyboard improvements (PR #243)
[https://avatars.githubusercontent.com/u/204477525?s=20&v=4]Vishaldadlani321 left a comment (jamiepine/voicebox#243)<#243 (comment)>
add hindi language
—
Reply to this email directly, view it on GitHub<#243 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACFL56ZAZUEAVHQVY2WCUZL4PYBT5AVCNFSM6AAAAACWKRSH6WVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DAMRQGI4TIMJUGA>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
voice box mein audio generate nahi kar paa raha hai |
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
2 participants
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.
Tested with NVDA and Narrator on Windows. See docs/PR-ACCESSIBILITY.md for full description.
Summary by CodeRabbit