fix: improve dark mode text contrast on settings and call pages - #138
fix: improve dark mode text contrast on settings and call pages#138aditiraj-dev wants to merge 1 commit into
Conversation
|
@aditiraj-dev is attempting to deploy a commit to the itzzavdhesh's projects Team on Vercel. A member of the Team first needs to authorize it. |
github-actions
Bot
added
dco-missing
accessibility
assignee-match
bug
DCO VerifiedHi @aditiraj-dev, all commits currently include a valid Your PR can continue through normal review. VoiceForge automation |
|
Wondering what really moved? Review this PR in Change Stack to inspect semantic changes, definitions, and references. 📝 WalkthroughWalkthroughDark mode text visibility fixes applied across two pages: Call and Settings. Added Tailwind dark mode utility classes to warning banners and helper text elements to improve contrast and readability against dark backgrounds without changing any functional logic. ChangesDark Mode Contrast Improvements
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
PR Ready For Mentor ReviewHi @aditiraj-dev, your PR passed automated validation and moved into the GSSoC review queue.
@sabeenaviklar @1754riya @Anushreebasics @itsdakshjain, this GSSoC PR is ready for review. Mentor next steps: confirm scope, review behavior and tests, then approve, comment, or request changes. This does not mean the PR is approved yet. Please wait for mentor feedback before expecting merge. If changes are requested, update the same branch and keep the PR focused on the linked issue. VoiceForge automation |
Signed-off-by: Aditi Raj <aditiraj.novelist@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (2)
client/src/pages/Call.jsx (1)
152-157:⚠️ Potential issue | 🟠 Major | ⚡ Quick winApply the same dark mode text fix to the database error banner.
The
dbErrorbanner at lines 152–157 has identical styling to the warning banner at line 160, but it is missing thedark:text-neutral-200class. In dark mode, this error message will remain difficult to read, which defeats the accessibility improvement goal of this PR.This pull request fixes a dark mode contrast issue on the Settings and Call pages, and both error banners convey critical information that must be readable.
🎨 Proposed fix to add dark mode text color
- <div className="flex items-center gap-2 rounded-md border border-coral/40 bg-coral/10 p-4 text-sm font-semibold text-ink"> + <div className="flex items-center gap-2 rounded-md border border-coral/40 bg-coral/10 p-4 text-sm font-semibold text-ink dark:text-neutral-200"> <CircleAlert size={18} aria-hidden="true" /> <span>Database Error: {dbError}. Please ensure IndexedDB is enabled and not blocked.</span> </div>🤖 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 152 - 157, The dbError banner rendering in Call.jsx uses a className missing the dark mode text color, so add the same dark:text-neutral-200 class used for the other warning banner to the error banner's className on the element that renders when dbError is truthy; update the div that contains CircleAlert and the <span> to include dark:text-neutral-200 so the error text is readable in dark mode.client/src/pages/Settings.jsx (1)
101-106:⚠️ Potential issue | 🟠 Major | ⚡ Quick winApply dark mode text styling to the database error banner.
The
dbErrorbanner at lines 101–106 usestext-inkwithout a dark mode variant. In dark mode, this critical error message will have poor contrast and be difficult to read, which is inconsistent with the dark mode improvements applied elsewhere in this file.🎨 Proposed fix to add dark mode text color
- <div className="flex items-center gap-2 rounded-md border border-coral/40 bg-coral/10 p-4 text-sm font-semibold text-ink"> + <div className="flex items-center gap-2 rounded-md border border-coral/40 bg-coral/10 p-4 text-sm font-semibold text-ink dark:text-neutral-200"> <CircleAlert size={18} aria-hidden="true" /> <span>Database error: {dbError}</span> </div>🤖 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 101 - 106, The dbError banner (the conditional block referencing dbError and using CircleAlert) currently uses only text-ink and lacks a dark mode variant; update the div's className to include a dark mode text color (for example add dark:text-rose-200 or dark:text-white with appropriate contrast) so the error message retains readable contrast in dark mode—modify the className on the element rendering the database error to include the dark: text utility.
🤖 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/pages/Call.jsx`:
- Around line 152-157: The dbError banner rendering in Call.jsx uses a className
missing the dark mode text color, so add the same dark:text-neutral-200 class
used for the other warning banner to the error banner's className on the element
that renders when dbError is truthy; update the div that contains CircleAlert
and the <span> to include dark:text-neutral-200 so the error text is readable in
dark mode.
In `@client/src/pages/Settings.jsx`:
- Around line 101-106: The dbError banner (the conditional block referencing
dbError and using CircleAlert) currently uses only text-ink and lacks a dark
mode variant; update the div's className to include a dark mode text color (for
example add dark:text-rose-200 or dark:text-white with appropriate contrast) so
the error message retains readable contrast in dark mode—modify the className on
the element rendering the database error to include the dark: text utility.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2d7d44ca-4242-45ae-9a99-80ad647b9d15
📒 Files selected for processing (2)
client/src/pages/Call.jsxclient/src/pages/Settings.jsx
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
be408e9 to
a0b50c4
Compare
| @@ -157,7 +157,7 @@ export default function Call() { | |||
| )} | |||
|
|
|||
There was a problem hiding this comment.
Duplicate wrapper div retained (original non-dark version still present). Remove the extra earlier line to avoid duplicated DOM and extra padding/semantics.
There was a problem hiding this comment.
Sorry, I'd like to request a bit more clarification on this, if you don't mind. I'm not understanding what you mean by duplicates. I thought Line 155 and Line 160 are different errors for different conditionals, and so they have their own styling...I'm not sure what you are asking me to do?
| @@ -143,13 +143,13 @@ export default function Settings() { | |||
|
|
|||
There was a problem hiding this comment.
Many label/help text lines are duplicated (one for light, one for dark) — this duplicates DOM and increases bundle size. Replace pairs like:
text-ink/65 ... and dark:text-muted duplicated spans with a single span using both classes: className="text-ink/65 dark:text-muted
same for p elements: className="text-xs text-ink/50 mt-1 dark:text-muted
There was a problem hiding this comment.
I'm don't think I'm seeing the duplicates from my side; as far as I know, I just appended dark:text-muted to existing elements, and did not create separate elements for dark mode. If you don't mind, could you please point out the lines of code you are referring to?
itsdakshjain
left a comment
itsdakshjain
left a comment
There was a problem hiding this comment.
@aditiraj-dev! Thanks for working for VoiceForge.
I completely agree with the review left by @Anushreebasics above. Instead of duplicating eelements (div, span, p) for light and dark modes, we want to combine those Tailwind classes into a single element (e.g., className="text-ink/65 dark:text-muted"). This keeps our dom footprint clean and bundle size small.
|
Hi, I've reviewed my code again and am having trouble understanding the requested changes because from what I see the current implementation already uses a single element with both light and dark mode classes (for example, When I was making the changes, I did not create duplicate elements, just appended 'dark:text-muted' wherever required, so I haven't been able to identify the duplicated I'd be happy to make the requested changes once I understand the issue. |
4 participants
🚀 Program
GSSoC
📝 Description
This PR fixes a dark mode contrast issue on the Settings and Call pages, by updating the styling of relevant text elements.
🔗 Related Issue
Closes #135
🔄 Type of Change
🧪 How to Test
📸 Screenshots (if applicable)
Call Page Before:
Call Page After:
Settings Page Before:
Settings Page After:
✅ Checklist
feat: add voice preview)Summary by cubic
Improves dark mode text contrast on Settings (Voice Synthesis) and Call pages so helper text and alerts are readable without changing light mode.
dark:text-mutedto section description, value labels, and helper text.dark:text-neutral-200to the profile alert banner text.Written for commit a0b50c4. Summary will update on new commits.
Summary by CodeRabbit