feat: add High-Contrast Accessibility Mode Toggle - #1177
Conversation
|
@hrshjswniii is attempting to deploy a commit to the itzzavdhesh's projects Team on Vercel. A member of the Team first needs to authorize it. |
✍️ DCO Sign-off NeededHey @hrshjswniii! 👋 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 |
github-actions
Bot
added
accessibility
assignee-match
client
enhancement
🎉 PR Ready for Mentor ReviewHey @hrshjswniii! 👋 Your PR passed all checks and is now in the GSSoC review queue. Note 🔗 Closing: #1127 · 📐 117 lines across 4 file(s) · 📬 Already requested or no eligible reviewer found @sabeenaviklar @Anushreebasics @itsdakshjain @snehkris @Mrigakshi-Rathore @Itzzavdheshh @Nitya-003 @4f4d @lovestaco, this PR is ready for your review — please confirm scope, check behavior and tests, then approve or request changes. Important This is not an approval. Please wait for mentor feedback before expecting a merge. If changes are requested, push them to this same branch and keep the PR focused on the linked issue. 🤖 VoiceForge Automation · Updates automatically on edits |
📝 WalkthroughWalkthroughAdds a persisted high-contrast mode to the theme context, exposes it through a new Settings toggle, and applies high-contrast colors, borders, and focus styles through a document-level CSS class. ChangesHigh-contrast accessibility mode
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Settings
participant ThemeContext
participant localStorage
participant documentElement
Settings->>ThemeContext: toggleHighContrast()
ThemeContext->>localStorage: persist voiceforge:highContrast
ThemeContext->>documentElement: apply or remove high-contrast
ThemeContext-->>Settings: provide isHighContrast
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/components/ThemeContext.test.js`:
- Around line 1-15: Replace the direct localStorage test in the “ThemeContext
theme & high-contrast state module” suite with a mounted consumer inside
ThemeProvider. Use useTheme to assert the initial high-contrast state, invoke
toggleHighContrast, and verify the state, high-contrast document class, and
localStorage value stay synchronized without relying on a vacuous non-jsdom
fallback.
In `@client/src/styles.css`:
- Around line 147-148: Add a blank line immediately before the color declaration
in the affected CSS rule, preserving the existing color and background-color
declarations while satisfying the declaration-empty-line-before Stylelint rule.
- Around line 151-162: Update the .high-contrast universal styling so section
and card containers receive a 2px solid white border, not just the existing
controls and links. Preserve the current white border color and box-shadow
removal while ensuring card boundaries are thickened consistently.
- Around line 139-149: Update the .high-contrast styles so descendant palette
utilities such as bg-white, bg-amber-50/40, and text-ink/65 are overridden
through semantic variables or scoped high-contrast component tokens. Ensure
supported descendants resolve to black backgrounds, white text, and the defined
high-contrast colors rather than retaining direct utility declarations; preserve
the existing root variables and high-contrast behavior.
🪄 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: 9bcff6eb-14d1-4f10-84be-a92c0fa2938f
📒 Files selected for processing (4)
client/src/components/ThemeContext.jsxclient/src/components/ThemeContext.test.jsclient/src/pages/Settings.jsxclient/src/styles.css
| import { describe, it, expect } from "vitest"; | ||
|
|
||
| describe("ThemeContext theme & high-contrast state module", () => { | ||
| it("manages high contrast localStorage persistence safely", () => { | ||
| if (typeof localStorage !== "undefined") { | ||
| localStorage.setItem("voiceforge:highContrast", "true"); | ||
| expect(localStorage.getItem("voiceforge:highContrast")).toBe("true"); | ||
|
|
||
| localStorage.setItem("voiceforge:highContrast", "false"); | ||
| expect(localStorage.getItem("voiceforge:highContrast")).toBe("false"); | ||
| } else { | ||
| expect(true).toBe(true); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Test the ThemeProvider contract, not localStorage itself.
This test does not exercise ThemeProvider, useTheme, toggleHighContrast, or the high-contrast document class; in a non-jsdom environment, the fallback assertion can also pass vacuously. Mount a consumer inside ThemeProvider and assert initialization, toggling, class synchronization, and persistence.
🤖 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/ThemeContext.test.js` around lines 1 - 15, Replace the
direct localStorage test in the “ThemeContext theme & high-contrast state
module” suite with a mounted consumer inside ThemeProvider. Use useTheme to
assert the initial high-contrast state, invoke toggleHighContrast, and verify
the state, high-contrast document class, and localStorage value stay
synchronized without relying on a vacuous non-jsdom fallback.
| .high-contrast { | ||
| --bg-page: #000000 !important; | ||
| --bg-card: #000000 !important; | ||
| --bg-input: #000000 !important; | ||
| --text-base: #ffffff !important; | ||
| --text-muted: #ffff00 !important; | ||
| --border: #ffffff !important; | ||
| --ring: #ffff00 !important; | ||
| color: #ffffff !important; | ||
| background-color: #000000 !important; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Override descendant palette utilities, not only root variables.
Existing classes such as bg-white, bg-amber-50/40, and text-ink/65 emit direct declarations on descendants, so they override inherited root colors. Consequently, the mode cannot guarantee black/white styling or the claimed WCAG AAA contrast across the UI. Route these utilities through semantic variables or add scoped high-contrast tokens for supported components.
🧰 Tools
🪛 Stylelint (17.14.0)
[error] 147-147: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
🤖 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/styles.css` around lines 139 - 149, Update the .high-contrast
styles so descendant palette utilities such as bg-white, bg-amber-50/40, and
text-ink/65 are overridden through semantic variables or scoped high-contrast
component tokens. Ensure supported descendants resolve to black backgrounds,
white text, and the defined high-contrast colors rather than retaining direct
utility declarations; preserve the existing root variables and high-contrast
behavior.
| .high-contrast * { | ||
| border-color: #ffffff !important; | ||
| box-shadow: none !important; | ||
| } | ||
|
|
||
| .high-contrast button, | ||
| .high-contrast input, | ||
| .high-contrast select, | ||
| .high-contrast textarea, | ||
| .high-contrast a { | ||
| border: 2px solid #ffffff !important; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Increase card border widths as well as border colors.
The universal rule changes only border-color, while the 2px border applies only to controls and links. Existing section and card containers therefore remain 1px wide, missing the stated requirement for thicker card boundaries.
🤖 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/styles.css` around lines 151 - 162, Update the .high-contrast
universal styling so section and card containers receive a 2px solid white
border, not just the existing controls and links. Preserve the current white
border color and box-shadow removal while ensuring card boundaries are thickened
consistently.
itsdakshjain
left a comment
itsdakshjain
left a comment
There was a problem hiding this comment.
please include visual proof via screenshot or video
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
5 issues found across 4 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/pages/Settings.jsx">
<violation number="1" location="client/src/pages/Settings.jsx:70">
P3: `theme` and `toggleTheme` are never used in `Settings`; destructure only the two high-contrast values needed by the new section to avoid dead code and lint failures.</violation>
</file>
<file name="client/src/components/ThemeContext.jsx">
<violation number="1" location="client/src/components/ThemeContext.jsx:59">
P2: The `.high-contrast` class is applied in a `useEffect`, which runs after the browser paints. On initial load, users who have high-contrast mode persisted will see a brief flash of unstyled content before the class takes effect. Since this is an accessibility feature, consider using `useLayoutEffect` instead so the class is applied synchronously before the first paint, eliminating the flash entirely. The same improvement could also benefit the existing theme effect.</violation>
</file>
<file name="client/src/components/ThemeContext.test.js">
<violation number="1" location="client/src/components/ThemeContext.test.js:1">
P1: The test file provides essentially no coverage for the actual ThemeContext component or the high-contrast feature it claims to test. It only exercises basic localStorage setItem/getItem — no imports of ThemeProvider, useTheme, toggleHighContrast, or any of the actual module exports. The `expect(true).toBe(true)` fallback in the else branch is a placeholder assertion that always passes, and the `typeof localStorage !== "undefined"` guard is unnecessary in a Vitest/JSDOM test environment where localStorage is always available. This test would pass even if the entire high-contrast implementation were removed or broken, giving a false sense of coverage and missing the very regressions tests are meant to catch.</violation>
</file>
<file name="client/src/styles.css">
<violation number="1" location="client/src/styles.css:147">
P1: The high-contrast mode overrides CSS custom properties on the root and sets `color`/`background-color` only on `.high-contrast` itself. However, descendant elements styled with Tailwind utility classes like `bg-white`, `bg-amber-50/40`, and `text-ink/65` (visible in `Settings.jsx`) emit direct property declarations that will take precedence over inherited values. As a result, the mode cannot guarantee the claimed WCAG AAA contrast across the UI. Consider adding descendant overrides (e.g., `.high-contrast * { background-color: #000000 !important; color: #ffffff !important; }`) or routing these utilities through semantic custom properties that the high-contrast mode can control.</violation>
<violation number="2" location="client/src/styles.css:152">
P2: The `.high-contrast *` rule sets only `border-color` without increasing `border-width`, and the `2px solid` border shorthand is scoped to form controls and links only. Card and section containers (e.g., the `<section>` elements in Settings with `border border-ink/10`) will retain their default 1px width, undermining the stated goal of thicker visual boundaries for all elements. Consider adding `border-width: 2px !important;` to the universal rule or extending the 2px border selector to include container elements.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| @@ -0,0 +1,15 @@ | |||
| import { describe, it, expect } from "vitest"; | |||
There was a problem hiding this comment.
P1: The test file provides essentially no coverage for the actual ThemeContext component or the high-contrast feature it claims to test. It only exercises basic localStorage setItem/getItem — no imports of ThemeProvider, useTheme, toggleHighContrast, or any of the actual module exports. The expect(true).toBe(true) fallback in the else branch is a placeholder assertion that always passes, and the typeof localStorage !== "undefined" guard is unnecessary in a Vitest/JSDOM test environment where localStorage is always available. This test would pass even if the entire high-contrast implementation were removed or broken, giving a false sense of coverage and missing the very regressions tests are meant to catch.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/src/components/ThemeContext.test.js, line 1:
<comment>The test file provides essentially no coverage for the actual ThemeContext component or the high-contrast feature it claims to test. It only exercises basic localStorage setItem/getItem — no imports of ThemeProvider, useTheme, toggleHighContrast, or any of the actual module exports. The `expect(true).toBe(true)` fallback in the else branch is a placeholder assertion that always passes, and the `typeof localStorage !== "undefined"` guard is unnecessary in a Vitest/JSDOM test environment where localStorage is always available. This test would pass even if the entire high-contrast implementation were removed or broken, giving a false sense of coverage and missing the very regressions tests are meant to catch.</comment>
<file context>
@@ -0,0 +1,15 @@
+import { describe, it, expect } from "vitest";
+
+describe("ThemeContext theme & high-contrast state module", () => {
</file context>
| --text-muted: #ffff00 !important; | ||
| --border: #ffffff !important; | ||
| --ring: #ffff00 !important; | ||
| color: #ffffff !important; |
There was a problem hiding this comment.
P1: The high-contrast mode overrides CSS custom properties on the root and sets color/background-color only on .high-contrast itself. However, descendant elements styled with Tailwind utility classes like bg-white, bg-amber-50/40, and text-ink/65 (visible in Settings.jsx) emit direct property declarations that will take precedence over inherited values. As a result, the mode cannot guarantee the claimed WCAG AAA contrast across the UI. Consider adding descendant overrides (e.g., .high-contrast * { background-color: #000000 !important; color: #ffffff !important; }) or routing these utilities through semantic custom properties that the high-contrast mode can control.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/src/styles.css, line 147:
<comment>The high-contrast mode overrides CSS custom properties on the root and sets `color`/`background-color` only on `.high-contrast` itself. However, descendant elements styled with Tailwind utility classes like `bg-white`, `bg-amber-50/40`, and `text-ink/65` (visible in `Settings.jsx`) emit direct property declarations that will take precedence over inherited values. As a result, the mode cannot guarantee the claimed WCAG AAA contrast across the UI. Consider adding descendant overrides (e.g., `.high-contrast * { background-color: #000000 !important; color: #ffffff !important; }`) or routing these utilities through semantic custom properties that the high-contrast mode can control.</comment>
<file context>
@@ -134,3 +134,34 @@ textarea {
+ --text-muted: #ffff00 !important;
+ --border: #ffffff !important;
+ --ring: #ffff00 !important;
+ color: #ffffff !important;
+ background-color: #000000 !important;
+}
</file context>
| storeTheme(theme); | ||
| }, [theme]); | ||
|
|
||
| React.useEffect(() => { |
There was a problem hiding this comment.
P2: The .high-contrast class is applied in a useEffect, which runs after the browser paints. On initial load, users who have high-contrast mode persisted will see a brief flash of unstyled content before the class takes effect. Since this is an accessibility feature, consider using useLayoutEffect instead so the class is applied synchronously before the first paint, eliminating the flash entirely. The same improvement could also benefit the existing theme effect.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/src/components/ThemeContext.jsx, line 59:
<comment>The `.high-contrast` class is applied in a `useEffect`, which runs after the browser paints. On initial load, users who have high-contrast mode persisted will see a brief flash of unstyled content before the class takes effect. Since this is an accessibility feature, consider using `useLayoutEffect` instead so the class is applied synchronously before the first paint, eliminating the flash entirely. The same improvement could also benefit the existing theme effect.</comment>
<file context>
@@ -39,12 +56,26 @@ export function ThemeProvider({ children }) {
storeTheme(theme);
}, [theme]);
+ React.useEffect(() => {
+ const root = document.documentElement;
+ if (isHighContrast) {
</file context>
| React.useEffect(() => { | |
| React.useLayoutEffect(() => { |
| } | ||
|
|
||
| .high-contrast * { | ||
| border-color: #ffffff !important; |
There was a problem hiding this comment.
P2: The .high-contrast * rule sets only border-color without increasing border-width, and the 2px solid border shorthand is scoped to form controls and links only. Card and section containers (e.g., the <section> elements in Settings with border border-ink/10) will retain their default 1px width, undermining the stated goal of thicker visual boundaries for all elements. Consider adding border-width: 2px !important; to the universal rule or extending the 2px border selector to include container elements.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/src/styles.css, line 152:
<comment>The `.high-contrast *` rule sets only `border-color` without increasing `border-width`, and the `2px solid` border shorthand is scoped to form controls and links only. Card and section containers (e.g., the `<section>` elements in Settings with `border border-ink/10`) will retain their default 1px width, undermining the stated goal of thicker visual boundaries for all elements. Consider adding `border-width: 2px !important;` to the universal rule or extending the 2px border selector to include container elements.</comment>
<file context>
@@ -134,3 +134,34 @@ textarea {
+}
+
+.high-contrast * {
+ border-color: #ffffff !important;
+ box-shadow: none !important;
+}
</file context>
| border-color: #ffffff !important; | |
| border-color: #ffffff !important; | |
| border-width: 2px !important; |
|
|
||
|
|
||
| const defaultSettings = DEFAULT_VOICE_SETTINGS; | ||
| const { theme, toggleTheme, isHighContrast, toggleHighContrast } = useTheme(); |
There was a problem hiding this comment.
P3: theme and toggleTheme are never used in Settings; destructure only the two high-contrast values needed by the new section to avoid dead code and lint failures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At client/src/pages/Settings.jsx, line 70:
<comment>`theme` and `toggleTheme` are never used in `Settings`; destructure only the two high-contrast values needed by the new section to avoid dead code and lint failures.</comment>
<file context>
@@ -66,6 +67,7 @@ export default function Settings() {
const defaultSettings = DEFAULT_VOICE_SETTINGS;
+ const { theme, toggleTheme, isHighContrast, toggleHighContrast } = useTheme();
const [voiceSettings, setVoiceSettings] = React.useState(loadVoiceSettings);
const [language, setLanguage] = React.useState(loadLanguage);
</file context>
| const { theme, toggleTheme, isHighContrast, toggleHighContrast } = useTheme(); | |
| const { isHighContrast, toggleHighContrast } = useTheme(); |
Nitya-003
left a comment
Nitya-003
left a comment
There was a problem hiding this comment.
@hrshjswniii Resolve comments.
github-actions
Bot
added
mentor:Nitya-003
🔄 Changes RequestedHey @hrshjswniii! 👋 A mentor has reviewed your PR and requested some changes. Warning Please review the feedback above, update this same branch, and keep the PR focused on the linked issue. Once you push your updates, the review flow will continue automatically on this same PR. 🤖 VoiceForge Automation · Updates automatically on edits |
itzzavdhesh
added
gssoc:approved
🎊 PR Merged SuccessfullyHey @hrshjswniii! 👋 Congratulations and thank you for your contribution to VoiceForge! Note 🔗 Linked issue(s): #1127 · ✅ 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 |
itzzavdhesh
added
SSoC26
4 participants
🚀 Program
GSSoC
📝 Description
This PR implements a High-Contrast Accessibility Mode Toggle in
ThemeContext.jsxandSettings.jsx, enhancing visual boundaries, text contrast ratios (WCAG AAA compliant), and focus outlines for visually impaired, neurodivergent, or high-ambient light users.Key additions:
styles.css): Defined.high-contrastrules enforcing maximum WCAG AAA contrast (#000000/#ffffff), 2px solid element borders, and 3px yellow (#ffff00) focus outlines.ThemeContext.jsx): ExtendedThemeContext.jsxwithisHighContraststate,toggleHighContrast()action,voiceforge:highContrastlocalStorage persistence, and dynamic.high-contrastclass toggling ondocument.documentElement.Settings.jsx): Rendered an Appearance & Accessibility settings section with an interactive High-Contrast Accessibility Mode toggle switch.ThemeContext.test.js): Added Vitest test assertions verifying high-contrast theme state management.🔗 Related Issue
Closes #1127
🔄 Type of Change
🧪 How to Test
http://localhost:5173in a browser.npm run test --workspace clientand verify all tests pass.✅ Checklist
feat: add High-Contrast Accessibility Mode Toggle)Summary by cubic
Adds a High-Contrast Accessibility Mode with a settings toggle and persisted state to improve readability and keyboard focus visibility across the app. Applies WCAG AAA contrast with clear borders and bright focus outlines.
isHighContrastandtoggleHighContrastinThemeContext, persisted vialocalStorage(voiceforge:highContrast) and applied by toggling.high-contrastondocument.documentElement. Safe storage access with try/catch..high-contrastCSS: black/white palette, 2px borders, 3px yellow focus rings, and no box shadows.ThemeContext.test.js.Written for commit 78ea49b. Summary will update on new commits.
Summary by CodeRabbit
New Features
Tests