-
Notifications
You must be signed in to change notification settings - Fork 120
fix: improve dark mode contrast and accessibility for input placeholders #240
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: improve dark mode contrast and accessibility for input placeholders #240
Conversation
📝 WalkthroughWalkthroughThis PR adds dark mode support to Tailwind configuration and restructures global CSS into semantic layers, enhancing dark mode contrast for cards, tables, input placeholders, and scrollbar styling to improve accessibility and readability. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@landing/src/index.css`:
- Around line 23-27: The transition is currently set only on the tr:hover rule
so the un-hover state snaps; move the transition declaration from tr:hover into
the base tr selector (target the tr selector, not tr:hover) so both hover-in and
hover-out animate, and replace the hardcoded background-color value with the
design token (e.g., --dark-lighter) in tr:hover to use the themed color.
🧹 Nitpick comments (3)
landing/src/index.css (3)
7-13: Consider using a Tailwind token for the background color.The hardcoded
#09090b(zinc-950) could be added as a token intailwind.config.js(e.g.,dark-bg) for consistency with other dark mode colors. This would make future palette adjustments easier.♻️ Suggested refactor
In
frontend/tailwind.config.js:colors: { + 'dark-bg': '#09090b', // Zinc-950 for main background primary: '#22c55e', ... }Then in this file:
body { font-family: 'Inter', sans-serif; - /* Use a slightly lifted dark color for better depth than pure black */ - background-color: `#09090b`; - `@apply` text-white antialiased; + `@apply` bg-dark-bg text-white antialiased; }
15-21: Use theaccessible-graytoken instead of hardcoded hex.The
accessible-gray: '#a1a1aa'token was defined intailwind.config.jsspecifically for this purpose. Using CSS custom properties or the token directly would improve maintainability.♻️ Suggested approach using CSS custom property
In
tailwind.config.js, Tailwind generates CSS variables. You can reference them:input::placeholder, textarea::placeholder { - color: `#a1a1aa` !important; /* Zinc-400 provides a 4.5:1 contrast ratio */ + color: theme('colors.accessible-gray') !important; opacity: 1; }Alternatively, if you're using Tailwind's CSS variable mode, you could use
var(--color-accessible-gray).
66-84: Consider adding Firefox scrollbar support for cross-browser consistency.The
::-webkit-scrollbarstyles only work in Chromium-based browsers and Safari. Firefox users will see the default scrollbar. You can add basic Firefox support withscrollbar-colorandscrollbar-width.♻️ Add Firefox scrollbar styling
+/* Firefox Scrollbar Support */ +* { + scrollbar-width: thin; + scrollbar-color: `#22c55e` `#000`; +} + /* Webkit Scrollbar Styling - Keeping your custom branding */ ::-webkit-scrollbar { width: 10px; height: 10px; }Note: Firefox doesn't support gradient scrollbar thumbs, so a solid color is the best approximation.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/tailwind.config.jslanding/src/index.css
🔇 Additional comments (2)
frontend/tailwind.config.js (1)
4-14: LGTM! Well-structured dark mode configuration.The color tokens are well-named and the values align with Tailwind's standard palette (green-500/600, zinc-400). The
darkMode: 'class'approach is the recommended method for explicit dark mode control.One consideration: verify that
accessible-grayis actually used via Tailwind classes in components rather than hardcoded hex values, to maintain consistency and ease future updates.landing/src/index.css (1)
30-60: Well-structured component classes with proper token usage.The component layer correctly references the custom color tokens (
bg-dark-card,bg-primary,hover:bg-primary-hover,bg-dark-lighter) and properly separates base and hover states for.hover-lift. This is the pattern that should be followed in the base layer as well.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
| /* Accessibility Fix: Table row hover state */ | ||
| tr:hover { | ||
| background-color: #1e1e1e !important; | ||
| transition: background-color 0.2s ease; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move transition to the base tr selector for proper animation.
The transition property is defined inside :hover, which means it only applies when hovering. On mouse-out, the background will snap back instantly instead of animating smoothly. Also, consider using the dark-lighter token.
🐛 Proposed fix
+ tr {
+ transition: background-color 0.2s ease;
+ }
+
/* Accessibility Fix: Table row hover state */
tr:hover {
- background-color: `#1e1e1e` !important;
- transition: background-color 0.2s ease;
+ background-color: theme('colors.dark-lighter') !important;
}🤖 Prompt for AI Agents
In `@landing/src/index.css` around lines 23 - 27, The transition is currently set
only on the tr:hover rule so the un-hover state snaps; move the transition
declaration from tr:hover into the base tr selector (target the tr selector, not
tr:hover) so both hover-in and hover-out animate, and replace the hardcoded
background-color value with the design token (e.g., --dark-lighter) in tr:hover
to use the themed color.
|
I am done implementing the issue raised by codeRabbit. |
Closes #236
Description
This PR significantly improves the Accessibility (A11y) and visual consistency of the DevR.ai interface in Dark Mode.
Previously, several UI components, particularly input placeholders and card borders, failed to meet the WCAG 2.1 AA contrast ratio standards, making the platform difficult to use for individuals with visual impairments or in high-glare environments.
By standardizing the color palette using a refined Zinc-based system, this PR ensures that all text elements are readable and interactive components provide clear visual feedback.
Changes Made
Color Tokenization: Extended the tailwind.config.js to include a semantic dark mode palette (dark-card, dark-lighter, accessible-gray).
Placeholder Legibility: Forced a global override for ::placeholder colors to #a1a1aa, ensuring a contrast ratio of at least 4.5:1.
Interactive Surfaces:
Accessibilty audit
Note: The score of 89 reflects a significant jump from the previous baseline. The remaining points are largely attributed to third-party scripts and non-color-related ARIA labels which were out of scope for this CSS-focused PR.
Checklist
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.