Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 38 additions & 14 deletions client/src/components/ThemeContext.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,33 @@ function getSystemTheme() {
}
}

function getStoredHighContrast() {
try {
return localStorage.getItem("voiceforge:highContrast") === "true";
} catch {
return false;
}
}

function storeTheme(theme) {
try {
localStorage.setItem("voiceforge:theme", theme);
} catch {
// Theme still works for the current session when persistence is unavailable.
}
}

function storeHighContrast(enabled) {
try {
localStorage.setItem("voiceforge:highContrast", String(enabled));
} catch {
// Storage unavailable
}
}

export function ThemeProvider({ children }) {
const [theme, setTheme] = React.useState(() => getStoredTheme() || getSystemTheme());
const [theme, setTheme] = React.useState(getStoredTheme);
const [isHighContrast, setIsHighContrast] = React.useState(getStoredHighContrast);

React.useEffect(() => {
const root = document.documentElement;
Expand All @@ -34,19 +59,14 @@ export function ThemeProvider({ children }) {
}, [theme]);

React.useEffect(() => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
React.useEffect(() => {
React.useLayoutEffect(() => {

const mediaQuery = window.matchMedia("(prefers-color-scheme: dark)");
const handleChange = (e) => {
// Only dynamically sync with OS if user hasn't explicitly overridden the theme
if (!getStoredTheme()) {
setTheme(e.matches ? "dark" : "light");
}
};

if (mediaQuery?.addEventListener) {
mediaQuery.addEventListener("change", handleChange);
return () => mediaQuery.removeEventListener("change", handleChange);
const root = document.documentElement;
if (isHighContrast) {
root.classList.add("high-contrast");
} else {
root.classList.remove("high-contrast");
}
}, []);
storeHighContrast(isHighContrast);
}, [isHighContrast]);

function toggleTheme() {
setTheme((prev) => {
Expand All @@ -60,8 +80,12 @@ export function ThemeProvider({ children }) {
});
}

function toggleHighContrast() {
setIsHighContrast((prev) => !prev);
}

return (
<ThemeContext.Provider value={{ theme, toggleTheme }}>
<ThemeContext.Provider value={{ theme, toggleTheme, isHighContrast, toggleHighContrast }}>
{children}
</ThemeContext.Provider>
);
Expand Down
15 changes: 15 additions & 0 deletions client/src/components/ThemeContext.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { describe, it, expect } from "vitest";

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>


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);
}
});
});
Comment on lines +1 to +15

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

πŸ“ 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.

38 changes: 37 additions & 1 deletion client/src/pages/Settings.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ import {
LANGUAGE_STORAGE_KEY,
} from "../utils/languages.js";

import { Trash2, CircleAlert, Download, Upload, Globe, Webcam } from "lucide-react";
import { Trash2, CircleAlert, Download, Upload, Globe, Eye } from "lucide-react";
import { useToast, ToastContainer } from "../components/useToast.jsx";
import { LanguageSelector } from "../components/LanguageSelector.jsx";
import { useTheme } from "../components/ThemeContext.jsx";
import {
deleteVoiceProfile,
getSavedProfiles,
Expand Down Expand Up @@ -77,6 +78,7 @@ export default function Settings() {


const defaultSettings = DEFAULT_VOICE_SETTINGS;
const { theme, toggleTheme, isHighContrast, toggleHighContrast } = useTheme();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
const { theme, toggleTheme, isHighContrast, toggleHighContrast } = useTheme();
const { isHighContrast, toggleHighContrast } = useTheme();

const [voiceSettings, setVoiceSettings] = React.useState(loadVoiceSettings);
const [language, setLanguage] = React.useState(loadLanguage);
const selectedLangObj = getLanguageByCode(language);
Expand Down Expand Up @@ -628,6 +630,40 @@ export default function Settings() {
<AudioOutputSelector />
</section>

<section className="rounded-lg border border-ink/10 bg-white p-5 shadow-soft dark:border-border dark:bg-surface dark:text-neutral-100 dark:shadow-soft-dk">
<h2 className="text-xl font-bold mb-1">Appearance & Accessibility</h2>
<p className="text-sm text-ink/65 mb-5 dark:text-muted">
Customize high-contrast accessibility options, contrast ratios, and visual boundaries.
</p>

<div className="flex flex-col gap-4 sm:flex-row sm:items-center justify-between rounded-md border border-ink/10 bg-amber-50/40 p-4 dark:border-border dark:bg-black">
<div className="flex items-start gap-3">
<Eye size={20} className="mt-0.5 text-moss dark:text-glow" aria-hidden="true" />
<div>
<h3 className="font-semibold text-sm text-ink dark:text-neutral-100">
High-Contrast Accessibility Mode
</h3>
<p className="text-xs text-ink/65 dark:text-muted mt-0.5">
Enforces maximum WCAG AAA contrast ratios, thick element borders, and bright yellow focus rings.
</p>
</div>
</div>

<button
type="button"
onClick={toggleHighContrast}
aria-pressed={isHighContrast}
className={`inline-flex items-center gap-2 rounded-full px-4 py-2 text-xs font-bold transition-all ${
isHighContrast
? "bg-amber-500 text-black shadow-sm ring-2 ring-amber-400"
: "bg-ink/10 text-ink hover:bg-ink/20 dark:bg-neutral-800 dark:text-neutral-200"
}`}
>
{isHighContrast ? "High-Contrast ON" : "High-Contrast OFF"}
</button>
</div>
</section>

<section className="rounded-lg border border-ink/10 bg-white p-5 shadow-soft dark:border-border dark:bg-surface dark:text-neutral-100 dark:shadow-soft-dk">
<h2 className="text-xl font-bold">Backup & Restore</h2>
<p className="mt-1 text-sm text-ink/65 mb-5 dark:text-muted">
Expand Down
31 changes: 31 additions & 0 deletions client/src/styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -243,3 +243,34 @@ textarea {
transition: stroke-dashoffset 0.1s linear, opacity 0.1s ease;
opacity: 0;
}

/* High-Contrast Accessibility Mode */
.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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>

background-color: #000000 !important;
Comment thread
itsdakshjain marked this conversation as resolved.
}
Comment on lines +248 to +258

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
border-color: #ffffff !important;
border-color: #ffffff !important;
border-width: 2px !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;
}
Comment on lines +260 to +271

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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.


.high-contrast :focus-visible {
outline: 3px solid #ffff00 !important;
outline-offset: 3px !important;
}
Loading