feat(a11y): comprehensive accessibility overhaul for WCAG compliance - #295
Conversation
|
@vivek0369 is attempting to deploy a commit to the itzzavdhesh's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThis PR adds keyboard navigation, focus management, ARIA roles and labels, live regions, reduced-motion styles, skip-link support, accessible page controls, and favorite-history sanitization across the client application. ChangesAccessibility Overhaul
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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 |
✍️ DCO Sign-off NeededHey @vivek0028! 👋 One or more commits in this PR are missing a Warning
How to fix: For the latest commit: git commit --amend --signoff
git push --force-with-leaseFor multiple commits, replace git rebase --signoff HEAD~N
git push --force-with-leaseThis comment will update automatically after you push. 🤖 VoiceForge Automation · Updates automatically on edits |
🛠️ PR Needs UpdatesHey @vivek0028! 👋 A few things need fixing before a mentor can review this PR. Warning
How to fix:
Once fixed, the workflow re-runs automatically and pings the right mentor. 🤖 VoiceForge Automation · Updates automatically on edits |
There was a problem hiding this comment.
3 issues found across 17 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="client/src/components/VoiceRecorder.jsx">
<violation number="1" location="client/src/components/VoiceRecorder.jsx:190">
P2: Timer element overrides the implicit `aria-live="off"` of `role="timer"` with explicit `aria-live="polite"` and `aria-atomic="true"`. Because `duration` updates every second via `setInterval`, this causes screen readers to announce the value every second, creating noisy/interruptive output. Per ARIA semantics, `role="timer"` implies `aria-live="off"` for exactly this reason. Remove the explicit live-region attributes so the timer remains silent by default; announce only at key checkpoints (e.g., start/stop/time-limit reached) if needed.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
|
This accessibility overhaul required changes across 17 files because accessibility concerns span multiple pages and shared components. The changes include keyboard navigation, focus management, ARIA labels, screen reader announcements, reduced-motion support, and skip navigation links. No unrelated files are included in this PR. |
sabeenaviklar
left a comment
There was a problem hiding this comment.
hi @vivek0369
Some of the changes needs to be addressed before we can merge this PR:
Missing Compose Tabpanel: In App.jsx, the wrapper for role="tabpanel" is omitted when activeTab === "compose". This breaks the WAI-ARIA association since the "compose" tab's aria-controls attribute points to an ID that doesn't exist. Please wrap <VoiceForge /> in a matching role="tabpanel" container with id="tabpanel-compose".
Merge Conflicts: The branch has conflicts with main. Please rebase or merge the latest main to resolve them.
Signed-off-by: Vivek Arya <vivekaryaa369@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/src/pages/Onboarding.jsx (1)
338-380: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDuplicate progress indicators rendered simultaneously.
The pre-existing "STEP PROGRESS INDICATORS COMPONENT GRID" (Line 339-362,
aria-label="Onboarding progress indicators") and the newrole="progressbar"block (Line 363-380,aria-label="Onboarding progress") both render side-by-side. The line-range summary describes the new block as a replacement, but the old block was not removed — resulting in two visually and semantically overlapping progress indicators, which is confusing visually and redundant for screen reader users (two separate labeled regions announcing step progress).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/pages/Onboarding.jsx` around lines 338 - 380, The onboarding progress UI is rendering two separate progress indicator sections at the same time, creating duplicate visuals and duplicate accessibility regions. In Onboarding.jsx, remove the obsolete “STEP PROGRESS INDICATORS COMPONENT GRID” block or replace it with the new `role="progressbar"` implementation so only one progress indicator set remains. Keep the single progress UI consistent with the existing `activeStep`, `stepContent`, and onboarding state logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/src/pages/Onboarding.jsx`:
- Around line 211-218: Resolve the merge conflict in Onboarding by removing the
literal conflict markers and keeping a valid single React state declaration for
serverStatus. The conflicting branches define different shapes, but hasKey later
reads serverStatus.space, so preserve the space field (or update hasKey and all
dependent reads consistently if you choose the hasServerKey shape). Locate the
fix around the serverStatus useState initialization in Onboarding and ensure the
final JSX/JS compiles without any conflict artifacts.
---
Outside diff comments:
In `@client/src/pages/Onboarding.jsx`:
- Around line 338-380: The onboarding progress UI is rendering two separate
progress indicator sections at the same time, creating duplicate visuals and
duplicate accessibility regions. In Onboarding.jsx, remove the obsolete “STEP
PROGRESS INDICATORS COMPONENT GRID” block or replace it with the new
`role="progressbar"` implementation so only one progress indicator set remains.
Keep the single progress UI consistent with the existing `activeStep`,
`stepContent`, and onboarding state logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e51a38a-ceb8-4863-a8e4-7d93a5975076
📒 Files selected for processing (9)
client/src/App.jsxclient/src/components/QuickReplies.jsxclient/src/components/TextToSpeech.jsxclient/src/components/VideoPreview.jsxclient/src/components/VoiceRecorder.jsxclient/src/pages/Call.jsxclient/src/pages/Onboarding.jsxclient/src/pages/Settings.jsxclient/src/styles.css
✅ Files skipped from review due to trivial changes (2)
- client/src/components/TextToSpeech.jsx
- client/src/App.jsx
🚧 Files skipped from review as they are similar to previous changes (4)
- client/src/styles.css
- client/src/components/QuickReplies.jsx
- client/src/pages/Call.jsx
- client/src/components/VideoPreview.jsx
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
client/src/pages/Onboarding.jsx (1)
338-380: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDuplicate progress indicators rendered simultaneously.
The pre-existing "STEP PROGRESS INDICATORS COMPONENT GRID" (Line 339-362,
aria-label="Onboarding progress indicators") and the newrole="progressbar"block (Line 363-380,aria-label="Onboarding progress") both render side-by-side. The line-range summary describes the new block as a replacement, but the old block was not removed — resulting in two visually and semantically overlapping progress indicators, which is confusing visually and redundant for screen reader users (two separate labeled regions announcing step progress).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/pages/Onboarding.jsx` around lines 338 - 380, The onboarding progress UI is rendering two separate progress indicator sections at the same time, creating duplicate visuals and duplicate accessibility regions. In Onboarding.jsx, remove the obsolete “STEP PROGRESS INDICATORS COMPONENT GRID” block or replace it with the new `role="progressbar"` implementation so only one progress indicator set remains. Keep the single progress UI consistent with the existing `activeStep`, `stepContent`, and onboarding state logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@client/src/pages/Onboarding.jsx`:
- Around line 211-218: Resolve the merge conflict in Onboarding by removing the
literal conflict markers and keeping a valid single React state declaration for
serverStatus. The conflicting branches define different shapes, but hasKey later
reads serverStatus.space, so preserve the space field (or update hasKey and all
dependent reads consistently if you choose the hasServerKey shape). Locate the
fix around the serverStatus useState initialization in Onboarding and ensure the
final JSX/JS compiles without any conflict artifacts.
---
Outside diff comments:
In `@client/src/pages/Onboarding.jsx`:
- Around line 338-380: The onboarding progress UI is rendering two separate
progress indicator sections at the same time, creating duplicate visuals and
duplicate accessibility regions. In Onboarding.jsx, remove the obsolete “STEP
PROGRESS INDICATORS COMPONENT GRID” block or replace it with the new
`role="progressbar"` implementation so only one progress indicator set remains.
Keep the single progress UI consistent with the existing `activeStep`,
`stepContent`, and onboarding state logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e51a38a-ceb8-4863-a8e4-7d93a5975076
📒 Files selected for processing (9)
client/src/App.jsxclient/src/components/QuickReplies.jsxclient/src/components/TextToSpeech.jsxclient/src/components/VideoPreview.jsxclient/src/components/VoiceRecorder.jsxclient/src/pages/Call.jsxclient/src/pages/Onboarding.jsxclient/src/pages/Settings.jsxclient/src/styles.css
✅ Files skipped from review due to trivial changes (2)
- client/src/components/TextToSpeech.jsx
- client/src/App.jsx
🚧 Files skipped from review as they are similar to previous changes (4)
- client/src/styles.css
- client/src/components/QuickReplies.jsx
- client/src/pages/Call.jsx
- client/src/components/VideoPreview.jsx
🛑 Comments failed to post (1)
client/src/pages/Onboarding.jsx (1)
211-218: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
Unresolved git merge conflict markers left in source.
Lines 211-218 contain literal
<<<<<<< HEAD/=======/>>>>>>> 7eeb8da (...)conflict markers. This is invalid JavaScript/JSX and will break the build — confirmed by the Biome parser errors at these exact lines. The two conflicting branches also declare differentserverStatusshapes ({ isMock, space }vs{ isMock, hasServerKey }), andhasKey(line 229) readsserverStatus.space, so the conflict must be resolved keeping thespacefield, orhasKeyneeds updating accordingly.🐛 Suggested resolution (keeping the field consistent with `hasKey`)
-<<<<<<< HEAD - const [serverStatus, setServerStatus] = React.useState({ isMock: false, space: "" }); -======= - const [serverStatus, setServerStatus] = React.useState({ - isMock: false, - hasServerKey: false, - }); ->>>>>>> 7eeb8da (refactor: reuse voice name length constants) + const [serverStatus, setServerStatus] = React.useState({ isMock: false, space: "" });📝 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.const [serverStatus, setServerStatus] = React.useState({ isMock: false, space: "" });🧰 Tools
🪛 Biome (2.5.1)
[error] 211-211: Expected a statement but instead found '<<<<<<< HEAD'.
(parse)
[error] 213-213: Expected a statement but instead found '======='.
(parse)
[error] 218-218: numbers cannot be followed by identifiers directly after
(parse)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/pages/Onboarding.jsx` around lines 211 - 218, Resolve the merge conflict in Onboarding by removing the literal conflict markers and keeping a valid single React state declaration for serverStatus. The conflicting branches define different shapes, but hasKey later reads serverStatus.space, so preserve the space field (or update hasKey and all dependent reads consistently if you choose the hasServerKey shape). Locate the fix around the serverStatus useState initialization in Onboarding and ensure the final JSX/JS compiles without any conflict artifacts.Source: Linters/SAST tools
sabeenaviklar
left a comment
There was a problem hiding this comment.
In styles.css skip-link block is missing its closing }, breaking scrollbar styles
… in useSpeechHistory.js
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…nPlay in VideoPreview.jsx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (19)
client/src/pages/Onboarding.jsx (4)
331-348: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winExpose one accurate onboarding progress indicator.
This block renders in addition to the existing grid at Lines 307-330, so the page shows duplicate progress UI. Each progressbar reports a fixed value of
1,2, or3; it does not reportactiveStep. Inactive steps can therefore be announced as completed progress. Remove the duplicate grid and expose one progressbar witharia-valuenow={activeStep}, or use a step list witharia-current="step".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/pages/Onboarding.jsx` around lines 331 - 348, Remove the duplicate progress UI around the existing onboarding progress block, and retain a single progressbar that represents the overall onboarding state with aria-valuenow bound to activeStep rather than each fixed step number. Update the surrounding progress markup in Onboarding so inactive steps are not announced as completed.
284-286: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle the rejected
cloneVoicepromise.
useVoiceClone.cloneVoiceupdatesapiErrorand then rethrows.handleClonedoes not catch the rejection, so failed cloning creates an unhandled promise rejection. Catch the error here or change the hook contract to return a handled failure result.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/pages/Onboarding.jsx` around lines 284 - 286, Update handleClone around the cloneVoice call to catch rejected promises, preventing failed voice cloning from becoming an unhandled rejection while preserving the existing success path that sets successProfile. Reuse the hook’s apiError handling or otherwise handle the failure locally without changing successful cloning behavior.
284-286: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPersist the unlocked step after cloning succeeds.
setSuccessProfile(profile)updates only in-memory state.maxUnlockedStepremains1. The Continue button storesactiveStep = 2, but initialization later clamps it withMath.min(parsedStep, parsedMax). After reload, the user returns to Step 1. SetmaxUnlockedStepto at least2after a successful clone.Suggested fix
const profile = await cloneVoice(recording.blob, voiceName); setSuccessProfile(profile); + setMaxUnlockedStep((step) => Math.max(step, 2));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/pages/Onboarding.jsx` around lines 284 - 286, After cloneVoice succeeds in the onboarding flow, update maxUnlockedStep to at least 2 alongside setSuccessProfile(profile), ensuring the unlocked-step value is persisted before navigation or reload.
198-206: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winComplete the recording-state refactor before rendering
Onboarding.The new state stores
{ blob, duration, isValid }, buthandleRecordingReadyis not declared in this component. The reference at Line 366 causes aReferenceErrorduring render. The clone button also reads undeclaredrecordingDurationat Line 393. Readrecording.durationinstead and disable cloning whenrecording.isValidis false.Suggested wiring
const [recording, setRecording] = React.useState(null); + function handleRecordingReady(nextRecording) { + setRecording(nextRecording); + } - disabled={isCloning || !hasKey || !recording || recordingDuration < 10 || Boolean(nameError)} + disabled={ + isCloning || + !hasKey || + !recording || + !recording.isValid || + recording.duration < 10 || + Boolean(nameError) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/pages/Onboarding.jsx` around lines 198 - 206, Complete the recording-state refactor in the Onboarding component by defining handleRecordingReady and wiring it to the recording callback, then update the clone-button logic to read recording.duration instead of recordingDuration. Ensure cloning is disabled when recording is absent or recording.isValid is false, while preserving the existing valid-recording behavior.client/src/components/VoiceForge.jsx (1)
257-282: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftHide the closed mobile drawer from keyboard navigation.
The focus trap activates only when the drawer is open, but the drawer remains rendered with
-translate-x-fullwhen closed. ItsSpeechHistorycontrols remain focusable and can receive focus from Tab and assistive technology. Use responsivehidden/inertsemantics or render the mobile drawer only while open; keep the desktop sidebar available.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/components/VoiceForge.jsx` around lines 257 - 282, Update the mobile drawer rendering around the history drawer and its SpeechHistory controls so the closed state is removed from keyboard and assistive-technology navigation using responsive hidden/inert semantics or conditional rendering. Preserve the existing focus trap when historyOpen is true, and keep the desktop sidebar available regardless of drawer state.client/src/components/TextToSpeech.jsx (1)
103-128: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winSubmit speech once and block concurrent keyboard submissions.
submitcallsonSpeaktwice. The second call sends the unmodified text without the emotion settings. Pressing Enter whilestatus === "speaking"also bypasses the button's disabled state.Proposed fix
async function submit() { - if (!trimmedText || disabled) return; + if (!trimmedText || disabled || status === "speaking" || characterCount > MAX_CHARS) return; ... await onSpeak(finalText, voice_settings_override); - if (!trimmedText || disabled || characterCount > MAX_CHARS) return; - await onSpeak(trimmedText); setText(""); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/components/TextToSpeech.jsx` around lines 103 - 128, Update submit to invoke onSpeak only once using finalText and voice_settings_override, then clear the text after a successful submission. Guard submit against disabled or active speaking state, and update handleKeyDown to prevent Enter-triggered submissions while status is "speaking", matching the button’s disabled behavior.client/src/components/QuickReplies.jsx (2)
241-265: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep every ARIA relationship resolvable.
The code points
aria-controlsoraria-describedbyat elements that are absent in some states. This leaves assistive technology with broken relationships.
client/src/components/QuickReplies.jsx#L241-L265: render panels for all tabs or use a stable panel ID witharia-labelledbyfor the selected tab.client/src/components/TextToSpeech.jsx#L197-L204: keeptts-char-hintmounted or setaria-describedbyonly when the hint exists.client/src/pages/Call.jsx#L283-L303: keepcalibration-panelmounted with hidden semantics or conditionally exposearia-controls.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/components/QuickReplies.jsx` around lines 241 - 265, Keep all ARIA references resolvable: in client/src/components/QuickReplies.jsx lines 241-265, render panels for every tab or use a stable panel ID with aria-labelledby; in client/src/components/TextToSpeech.jsx lines 197-204, keep tts-char-hint mounted or only set aria-describedby when the hint exists; in client/src/pages/Call.jsx lines 283-303, keep calibration-panel mounted with hidden semantics or conditionally expose aria-controls.
59-81: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winValidate synchronized quick replies before updating state.
handleSyncaccepts every array fromlocalStorage, unlike the initializer at Lines 21-34. A malformed entry can make rendering orr.phrase.toLowerCase()throw. If the key is removed, the handler also leaves stale replies in state. Reuse one loader and normalizer for both paths.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/components/QuickReplies.jsx` around lines 59 - 81, Update handleSync and the initial quick-replies loading path to reuse a single loader/normalizer that validates each reply using the same rules as the existing initializer before calling setReplies. When STORAGE_KEY is absent or invalid, normalize to the appropriate empty/default replies so stale state is cleared, while preserving the existing storage and custom event listeners.client/src/hooks/useSpeechHistory.js (1)
396-413: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winApply the favorite invariants during backup import.
Line 398 can evict an imported favorite solely by timestamp. Lines 400-413 also persist more than
MAX_FAVORITESfavorites because this path does not callclampFavorites.Create the capped favorite set before trimming. Use
trimHistoryPreservingFavoritesfor the merged history. Reconcile the final favorite set with the retained history.Proposed fix
- const finalHistory = mergedList.slice(0, MAX_HISTORY); - - const nextFavorites = new Set(favorites); - favIdsToAdd.forEach(id => nextFavorites.add(id)); - - // Clean up favorites: only keep favorites whose IDs are in the finalHistory - const finalHistoryIds = new Set(finalHistory.map(m => m.id)); - const cleanedFavorites = new Set(); - nextFavorites.forEach(id => { - if (finalHistoryIds.has(id)) { - cleanedFavorites.add(id); - } - }); + const candidateFavorites = clampFavorites( + reconcileFavoritesWithHistory( + new Set([...favorites, ...favIdsToAdd]), + mergedList + ), + MAX_FAVORITES + ); + const finalHistory = trimHistoryPreservingFavorites( + mergedList, + candidateFavorites, + MAX_HISTORY + ); + const cleanedFavorites = reconcileFavoritesWithHistory( + candidateFavorites, + finalHistory + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/hooks/useSpeechHistory.js` around lines 396 - 413, Update the backup-import merge flow around mergedList, nextFavorites, and finalHistory to cap favorites with the existing clampFavorites logic before trimming history. Use trimHistoryPreservingFavorites so imported favorites are retained when possible, then reconcile the capped favorite set against the IDs in the retained history before calling setHistory and setFavorites.client/src/pages/Settings.jsx (4)
317-353: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRemove the duplicate
Voice Presetcontrol.Lines 336-353 repeat Lines 317-334. Both controls use
id="voice-preset"and the same label. The page renders two selectors, and the duplicate ID makes label association and programmatic lookup ambiguous. Keep one selector.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/pages/Settings.jsx` around lines 317 - 353, Remove the duplicated Voice Preset control by deleting one of the repeated label/select blocks using id="voice-preset"; retain a single selector with its existing currentPresetKey value, handlePresetChange behavior, and VOICE_PRESETS options.
639-647: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winHandle failures while refreshing received profiles.
ReceiveProfileModalcallsonSuccess()without awaiting it. IfgetSavedProfiles()rejects, this callback returns an unhandled rejection and never setsisReceivingtofalse. Catch the refresh error, show feedback, and close the modal in afinallyblock.Suggested handling
onSuccess={async () => { - const loaded = await getSavedProfiles(); - setProfiles(loaded); - setIsReceiving(false); - showToast("Profile received successfully!", "success"); + try { + const loaded = await getSavedProfiles(); + setProfiles(loaded); + showToast("Profile received successfully!", "success"); + } catch (err) { + setDbError(err?.message || String(err)); + showToast("Failed to refresh profiles", "error"); + } finally { + setIsReceiving(false); + } }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/pages/Settings.jsx` around lines 639 - 647, Update the ReceiveProfileModal onSuccess callback to catch failures from getSavedProfiles, show an error toast, and always call setIsReceiving(false) in a finally block; keep profile state updates and the success toast only on successful refresh.
80-81: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winUse the browser storage event for cross-tab voice settings updates.
window.dispatchEventonly notifies listeners in the current document. SinceVoiceQuickSettingslistens towindow.addEventListener("storage", ...)and the user-facing copy says changes are shared across tabs, centralize the same-tab notify path and dispatchwindow.dispatchEvent(new Event("storage"))from the Settings import path or handle the import overwrite inVoiceQuickSettingsfor that tab.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/pages/Settings.jsx` around lines 80 - 81, Update the Settings import path around the existing voiceforge:settingsChanged dispatch so imported voice settings notify the same tab through the browser storage-event path expected by VoiceQuickSettings. Dispatch a storage event after the import overwrite, or centralize the notification to preserve cross-tab storage behavior while ensuring the importing tab also refreshes.
207-215: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not silently drop audio during backup import.
When
audioDataUrlis present but invalid, or whenfetchfails, the code only logs the error. The profile is still saved withaudioBlob: null, and the import reports success. Throw before the commit, or report a partial import explicitly.Suggested handling
} else { - console.warn("Skipped invalid or non-audio DataURL in voice profile backup:", p.name); + throw new Error("Backup contains an invalid audio DataURL."); } } catch (e) { - console.error("Failed to parse audio DataURL:", e); + throw new Error("Failed to restore profile audio.", { cause: e }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/pages/Settings.jsx` around lines 207 - 215, Update the audio handling in the profile backup import flow so invalid or non-audio audioDataUrl values and fetch failures do not continue with audioBlob set to null while reporting success. In the try/catch around the audioDataUrl processing, propagate an error or otherwise mark the import as partial before the profile commit, while preserving successful imports for valid audio data.client/src/components/VideoPreview.jsx (1)
427-436: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExpose the blur toggle state to assistive technology.
This button changes between “Blur ON” and “Blur OFF”, but it does not expose a pressed state. Add
aria-pressed={blurEnabled}.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/components/VideoPreview.jsx` around lines 427 - 436, Add aria-pressed={blurEnabled} to the blur toggle button in VideoPreview.jsx, preserving its existing click behavior, label, and styling.client/src/components/VoiceRecorder.jsx (3)
490-495: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRender one short-recording warning.
Both this block and the existing block at Lines 483-487 render when
duration < MIN_DURATION. Users see duplicate warnings, and screen readers receive duplicate messages. Keep one warning.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/components/VoiceRecorder.jsx` around lines 490 - 495, Remove the duplicate short-recording warning block near the existing warning in VoiceRecorder, keeping only one rendered message when duration is below MIN_DURATION. Preserve the remaining warning’s condition and accessibility behavior.
374-387: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winExpose recording progress semantics.
The new progress bar is visual only. Add
role="progressbar", an accessible label, andaria-valuemin,aria-valuemax, andaria-valuenow, or use a native<progress>element.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/components/VoiceRecorder.jsx` around lines 374 - 387, Update the progress bar element in the recording progress section of VoiceRecorder to expose progress semantics: add role="progressbar", an accessible label, and aria-valuemin, aria-valuemax, and aria-valuenow values based on duration and MIN_DURATION. Preserve the existing visual styling and capped percentage width.
180-191: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winStore the final recorded blob in
rawAudioBlob.The normal
recorder.onstoppath creates a blob but never callssetRawAudioBlob(blob).AudioTrimmerrenders only whenrawAudioBlobis set, so trimming works for uploads but not for recordings.Proposed fix
const blob = new Blob(chunksRef.current, { type: recorder.mimeType || "audio/webm" }); + setRawAudioBlob(blob); const url = URL.createObjectURL(blob);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/components/VoiceRecorder.jsx` around lines 180 - 191, Update the recorder.onstop handler in VoiceRecorder to call setRawAudioBlob with the newly created blob before triggering the recording-ready flow, ensuring recorded audio renders through AudioTrimmer while preserving the existing upload behavior.client/src/pages/Call.jsx (2)
188-209: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winClear stale camera state when reopening fails.
When a device switch or retry fails, cleanup stops the previous stream but does not clear
webcamStreamorlocalVideoRef.current.srcObject.VideoPreviewcan therefore receive a stopped stream while the camera error is displayed. Clear the old stream before opening a new one or in the failure path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/pages/Call.jsx` around lines 188 - 209, Update the camera reopen flow around openCamera and its webcamError catch to clear the previous webcamStream and localVideoRef.current.srcObject before attempting the new stream or when opening fails, while preserving existing track cleanup and error reporting so VideoPreview cannot receive a stopped stream.
415-428: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGive the subtitles checkbox a feature-specific accessible name.
The wrapping label exposes only “Enabled” as the checkbox name. It does not identify the setting as the subtitles overlay. Use a label such as “Enable subtitles overlay”.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/pages/Call.jsx` around lines 415 - 428, Update the subtitles checkbox label in the JSX around checked state subtitlesEnabled so its accessible name identifies the feature, replacing the generic “Enabled” text with wording such as “Enable subtitles overlay”; preserve the existing toggle behavior and styling.
🧹 Nitpick comments (1)
client/src/components/VoiceForge.jsx (1)
354-358: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winLink the mobile toggle to its controlled dialog.
Add matching
aria-controls="history-drawer"andid="history-drawer"attributes so assistive technology can identify the controlled region.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@client/src/components/VoiceForge.jsx` around lines 354 - 358, Add matching aria-controls="history-drawer" and id="history-drawer" attributes to the mobile history toggle element identified by historyToggleRef and its historyOpen state, linking the toggle to the controlled history drawer.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@client/src/components/QuickReplies.jsx`:
- Around line 241-265: Keep all ARIA references resolvable: in
client/src/components/QuickReplies.jsx lines 241-265, render panels for every
tab or use a stable panel ID with aria-labelledby; in
client/src/components/TextToSpeech.jsx lines 197-204, keep tts-char-hint mounted
or only set aria-describedby when the hint exists; in client/src/pages/Call.jsx
lines 283-303, keep calibration-panel mounted with hidden semantics or
conditionally expose aria-controls.
- Around line 59-81: Update handleSync and the initial quick-replies loading
path to reuse a single loader/normalizer that validates each reply using the
same rules as the existing initializer before calling setReplies. When
STORAGE_KEY is absent or invalid, normalize to the appropriate empty/default
replies so stale state is cleared, while preserving the existing storage and
custom event listeners.
In `@client/src/components/TextToSpeech.jsx`:
- Around line 103-128: Update submit to invoke onSpeak only once using finalText
and voice_settings_override, then clear the text after a successful submission.
Guard submit against disabled or active speaking state, and update handleKeyDown
to prevent Enter-triggered submissions while status is "speaking", matching the
button’s disabled behavior.
In `@client/src/components/VideoPreview.jsx`:
- Around line 427-436: Add aria-pressed={blurEnabled} to the blur toggle button
in VideoPreview.jsx, preserving its existing click behavior, label, and styling.
In `@client/src/components/VoiceForge.jsx`:
- Around line 257-282: Update the mobile drawer rendering around the history
drawer and its SpeechHistory controls so the closed state is removed from
keyboard and assistive-technology navigation using responsive hidden/inert
semantics or conditional rendering. Preserve the existing focus trap when
historyOpen is true, and keep the desktop sidebar available regardless of drawer
state.
In `@client/src/components/VoiceRecorder.jsx`:
- Around line 490-495: Remove the duplicate short-recording warning block near
the existing warning in VoiceRecorder, keeping only one rendered message when
duration is below MIN_DURATION. Preserve the remaining warning’s condition and
accessibility behavior.
- Around line 374-387: Update the progress bar element in the recording progress
section of VoiceRecorder to expose progress semantics: add role="progressbar",
an accessible label, and aria-valuemin, aria-valuemax, and aria-valuenow values
based on duration and MIN_DURATION. Preserve the existing visual styling and
capped percentage width.
- Around line 180-191: Update the recorder.onstop handler in VoiceRecorder to
call setRawAudioBlob with the newly created blob before triggering the
recording-ready flow, ensuring recorded audio renders through AudioTrimmer while
preserving the existing upload behavior.
In `@client/src/hooks/useSpeechHistory.js`:
- Around line 396-413: Update the backup-import merge flow around mergedList,
nextFavorites, and finalHistory to cap favorites with the existing
clampFavorites logic before trimming history. Use trimHistoryPreservingFavorites
so imported favorites are retained when possible, then reconcile the capped
favorite set against the IDs in the retained history before calling setHistory
and setFavorites.
In `@client/src/pages/Call.jsx`:
- Around line 188-209: Update the camera reopen flow around openCamera and its
webcamError catch to clear the previous webcamStream and
localVideoRef.current.srcObject before attempting the new stream or when opening
fails, while preserving existing track cleanup and error reporting so
VideoPreview cannot receive a stopped stream.
- Around line 415-428: Update the subtitles checkbox label in the JSX around
checked state subtitlesEnabled so its accessible name identifies the feature,
replacing the generic “Enabled” text with wording such as “Enable subtitles
overlay”; preserve the existing toggle behavior and styling.
In `@client/src/pages/Onboarding.jsx`:
- Around line 331-348: Remove the duplicate progress UI around the existing
onboarding progress block, and retain a single progressbar that represents the
overall onboarding state with aria-valuenow bound to activeStep rather than each
fixed step number. Update the surrounding progress markup in Onboarding so
inactive steps are not announced as completed.
- Around line 284-286: Update handleClone around the cloneVoice call to catch
rejected promises, preventing failed voice cloning from becoming an unhandled
rejection while preserving the existing success path that sets successProfile.
Reuse the hook’s apiError handling or otherwise handle the failure locally
without changing successful cloning behavior.
- Around line 284-286: After cloneVoice succeeds in the onboarding flow, update
maxUnlockedStep to at least 2 alongside setSuccessProfile(profile), ensuring the
unlocked-step value is persisted before navigation or reload.
- Around line 198-206: Complete the recording-state refactor in the Onboarding
component by defining handleRecordingReady and wiring it to the recording
callback, then update the clone-button logic to read recording.duration instead
of recordingDuration. Ensure cloning is disabled when recording is absent or
recording.isValid is false, while preserving the existing valid-recording
behavior.
In `@client/src/pages/Settings.jsx`:
- Around line 317-353: Remove the duplicated Voice Preset control by deleting
one of the repeated label/select blocks using id="voice-preset"; retain a single
selector with its existing currentPresetKey value, handlePresetChange behavior,
and VOICE_PRESETS options.
- Around line 639-647: Update the ReceiveProfileModal onSuccess callback to
catch failures from getSavedProfiles, show an error toast, and always call
setIsReceiving(false) in a finally block; keep profile state updates and the
success toast only on successful refresh.
- Around line 80-81: Update the Settings import path around the existing
voiceforge:settingsChanged dispatch so imported voice settings notify the same
tab through the browser storage-event path expected by VoiceQuickSettings.
Dispatch a storage event after the import overwrite, or centralize the
notification to preserve cross-tab storage behavior while ensuring the importing
tab also refreshes.
- Around line 207-215: Update the audio handling in the profile backup import
flow so invalid or non-audio audioDataUrl values and fetch failures do not
continue with audioBlob set to null while reporting success. In the try/catch
around the audioDataUrl processing, propagate an error or otherwise mark the
import as partial before the profile commit, while preserving successful imports
for valid audio data.
---
Nitpick comments:
In `@client/src/components/VoiceForge.jsx`:
- Around line 354-358: Add matching aria-controls="history-drawer" and
id="history-drawer" attributes to the mobile history toggle element identified
by historyToggleRef and its historyOpen state, linking the toggle to the
controlled history drawer.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9b8067fd-6a49-450a-9bee-7274bb500a62
📒 Files selected for processing (13)
client/src/App.jsxclient/src/components/Footer.jsxclient/src/components/LanguageSelector.jsxclient/src/components/QuickReplies.jsxclient/src/components/TextToSpeech.jsxclient/src/components/VideoPreview.jsxclient/src/components/VoiceForge.jsxclient/src/components/VoiceRecorder.jsxclient/src/hooks/useSpeechHistory.jsclient/src/pages/Call.jsxclient/src/pages/Onboarding.jsxclient/src/pages/Settings.jsxclient/src/styles.css
🚧 Files skipped from review as they are similar to previous changes (4)
- client/src/components/Footer.jsx
- client/src/components/LanguageSelector.jsx
- client/src/styles.css
- client/src/App.jsx
🎊 PR Merged SuccessfullyHey @vivek0028! 👋 Congratulations and thank you for your contribution to VoiceForge! Note 🔗 Linked issue(s): #279 · ✅ Marked as merged and complete Maintainers may still handle final cleanup, release notes, or follow-up tracking after the merge. 🤖 VoiceForge Automation · Updates automatically on edits |
🚀 Program
GSSoC
📝 Description
This PR introduces a comprehensive accessibility (a11y) overhaul across VoiceForge to improve usability for users relying on assistive technologies and to align the application more closely with WCAG 2.1 AA accessibility standards.
As VoiceForge is an assistive communication platform, accessibility is a core requirement rather than an optional enhancement. This update focuses on keyboard navigation, screen reader support, focus management, motion sensitivity, and semantic improvements throughout the application.
🔗 Related Issue
Closes #279
🔄 Type of Change
♿ Accessibility Improvements
⌨️ Enhanced Keyboard Navigation
Navigation Tabs
Implemented WAI-ARIA Tabs pattern in the main navigation.
Added support for:
ArrowLeftArrowRightEnterSpaceQuick Replies
Focus Management
Added focus trapping within mobile history drawer.
Added proper focus restoration after drawer/modal close.
Added
Escapekey support for:📢 Screen Reader Improvements
Live Status Announcements
Added
role="alert"for critical error messages.Added
aria-live="polite"for dynamic status updates.Added announcements for:
Semantic Labels
Added missing
aria-labelattributes across:Canvas Accessibility
Added:
role="img"aria-labelThis ensures screen readers can identify and describe visual content.
Footer Cleanup
href="#"links that generated confusing screen reader output.🛑 Reduced Motion Support
Implemented global support for:
When users enable reduced motion at the operating system level:
⏭️ Skip Navigation
Added a "Skip to Main Content" link as the first focusable element on every page.
Benefits:
🧪 How to Test
Keyboard Navigation
Navigate the application using only:
Verify:
Screen Reader Testing
Test with:
Verify:
Reduced Motion Testing
📸 Screenshots
✅ Checklist
Summary by cubic
Accessibility overhaul for WCAG 2.1 AA. Improves keyboard navigation, screen reader support, focus handling, reduced motion, and adds a skip-to-content link with labeled tabpanels across the app.
New Features
@mediapipe/selfie_segmentation.aria-livestatus messages and role="alert" errors; labeled audio/video/canvas; accessible calibration sliders; focus trap and focus restore for the history drawer and Language Selector; onboarding progress indicator; and a TTS character counter.Bug Fixes
useSpeechHistory.js.onPlayhandler in VideoPreview.Written for commit 3a36b3b. Summary will update on new commits.
Summary by CodeRabbit
Accessibility
Improvements