light/darkMode #154
light/darkMode #154
Conversation
WalkthroughIntroduces application-wide theming using CSS variables and a toggle in App.js. Adds theme.css with light/dark definitions and applies data-theme on the document element. Updates multiple component CSS files to use theme variables; adjusts some sizing in Chat.css. Simplifies Message.js classes to align with theme-based styling. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App as App (React)
participant DOM as document.documentElement
participant CSS as theme.css
User->>App: Click "Toggle Theme"
App->>App: toggle theme state (dark/light)
App->>DOM: setAttribute(data-theme, theme)
DOM-->>CSS: Attribute change observed
CSS-->>DOM: Apply CSS variables for theme
App->>App: Re-render components using themed styles
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (10)
client/src/components/TextContainer/TextContainer.css (1)
5-5: Good migration to theme variable; consider a safe fallback.Using
var(--text-color)aligns with the new theming system. Add a fallback to guard against missing variable definitions.Apply this diff:
- color: var(--text-color); + color: var(--text-color, #fff);client/src/components/InfoBar/InfoBar.css (1)
16-16: Use an “on-primary” text token for better contrast on the bar.
--text-coloris typically tuned for surfaces, not necessarily for text over the primary brand color. Consider introducing--on-primary-colorin theme.css and using it here to guarantee contrast in both light/dark themes.Proposed change:
- color: var(--text-color); + color: var(--on-primary-color);If
--on-primary-colorisn’t defined yet, add it in theme.css for both themes with accessible contrast.client/src/components/Messages/Messages.css (1)
5-5: Avoid identical backgrounds for container and bubbles.
.messagesusesvar(--secondary-color), and.messageBoxalso usesvar(--secondary-color). This flattens visual hierarchy. Prefer the page/surface background for the scroll area.Apply this diff:
- background: var(--secondary-color); + background: var(--background-color);Alternatively, consider a dedicated token like
--surface-elevatedfor message bubbles and keep the container neutral.client/src/components/Messages/Message/Message.css (3)
2-6: Theme variables look good; verify contrast on secondary surfaces.Using
--secondary-color+--text-coloris consistent. Ensure--text-colorremains readable over--secondary-coloracross themes; if not, introduce--on-surface-colorfor content on secondary surfaces.
33-35: Align “sentText” color with surface-specific token (optional).If you introduce
--on-surface-color, use it forsentTextto ensure consistent contrast on message rows.Proposed change:
- color: var(--text-color); + color: var(--on-surface-color, var(--text-color));
53-68: Remove legacy color classes if no longer used.With theme variables and simplified classes in Message.js, these legacy utility classes appear obsolete. Removing them reduces confusion and CSS bloat.
Apply this diff if they’re unused:
-.colorWhite { - color: white; -} - -.colorDark { - color: #353535; -} - -.backgroundBlue { - background: #2979FF; -} - -.backgroundLight { - background: #F3F3F3; -}If some are still referenced, consider mapping them to tokens instead (e.g.,
background: var(--primary-color)).client/src/components/Chat/Chat.css (1)
13-17: Check layout impact of vh/vw switch and container background.
- Container now uses
--secondary-colorwhile message bubbles also use--secondary-color, which may remove visual separation (see Messages.css note).- Height/width moved to
60vh/40vw. This improves predictability but may feel cramped on ultrawide screens or super tall viewports. Consider addingmax-width/min-widthguards.Apply this diff to improve responsiveness:
.container { @@ - height: 60vh; - width: 40vw; + height: 60vh; + width: 40vw; + max-width: 900px; + min-width: 320px; }Please verify on common breakpoints (768px, 1024px, 1440px) that the new dimensions work well with the media queries below.
client/src/components/Messages/Message/Message.js (1)
21-23: Confirm visual differentiation between sent vs. received messages.You’ve removed the color-specific classes. Ensure
Message.cssstill differentiates own vs. others’ messages sufficiently (e.g., background tint, border, or subtle accent), otherwise both bubbles will look identical aside from alignment. If needed, add a conditional class to enable theme-aware differentiation.Example tweak (JS) to enable styling hooks:
- <div className="messageBox"> + <div className={`messageBox ${isSentByCurrentUser ? 'me' : 'peer'}`}> @@ - <div className="messageBox"> + <div className={`messageBox ${isSentByCurrentUser ? 'me' : 'peer'}`}>Then style
.messageBox.mevs.messageBox.peerin CSS using theme tokens.Also applies to: 28-30
client/src/theme.css (1)
16-20: Advertise color-scheme support to browsers.Let the UA style built-in widgets appropriately in dark mode (scrollbars, form controls).
Apply this diff:
body { background: var(--background-color); color: var(--text-color); transition: background 0.3s, color 0.3s; + color-scheme: light dark; }client/src/App.js (1)
18-20: Use functional setState for the toggle.Prevents any stale-closure edge cases on rapid clicks.
Apply this diff:
const toggleTheme = () => { - setTheme(theme === 'dark' ? 'light' : 'dark'); + setTheme(prev => (prev === 'dark' ? 'light' : 'dark')); };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
client/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (10)
client/src/App.js(1 hunks)client/src/components/Chat/Chat.css(1 hunks)client/src/components/InfoBar/InfoBar.css(2 hunks)client/src/components/Input/Input.css(2 hunks)client/src/components/Join/Join.css(2 hunks)client/src/components/Messages/Message/Message.css(2 hunks)client/src/components/Messages/Message/Message.js(1 hunks)client/src/components/Messages/Messages.css(1 hunks)client/src/components/TextContainer/TextContainer.css(1 hunks)client/src/theme.css(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
client/src/App.js (1)
client/src/components/Chat/Chat.js (1)
Chat(16-66)
🔇 Additional comments (7)
client/src/components/InfoBar/InfoBar.css (1)
5-5: LGTM: Primary color is now theme-driven.Switching to
var(--primary-color)matches the theming approach and keeps InfoBar consistent across themes.client/src/components/Chat/Chat.css (1)
6-6: LGTM: Background now theme-driven.Adopting
var(--background-color)for the outer container aligns with the global theming strategy.client/src/components/Input/Input.css (2)
3-3: Good move to theme the form border.Border now follows the theme palette via CSS variables.
12-13: Input field colors correctly derive from theme tokens.Using
--secondary-colorand--text-colorimproves consistency across themes.client/src/components/Join/Join.css (3)
21-21: Theme adoption for the page background looks good.
32-33: Input field theming looks correct.Background and text now align with light/dark tokens.
36-41: Heading theming looks consistent.
| const [theme, setTheme] = useState('dark'); | ||
|
|
||
| useEffect(() => { | ||
| document.documentElement.setAttribute('data-theme', theme); | ||
| }, [theme]); |
There was a problem hiding this comment.
Persist theme to localStorage and initialize from saved or system preference.
PR summary claims persistence, but it’s not implemented. Also set the attribute and save in one effect to avoid drift.
Apply this diff:
- const [theme, setTheme] = useState('dark');
+ const [theme, setTheme] = useState(getInitialTheme);
- useEffect(() => {
- document.documentElement.setAttribute('data-theme', theme);
- }, [theme]);
+ useEffect(() => {
+ document.documentElement.setAttribute('data-theme', theme);
+ try {
+ localStorage.setItem('theme', theme);
+ } catch {}
+ }, [theme]);Add this helper above const App = () => {:
// Initializes from localStorage, then falls back to OS preference, else light.
const getInitialTheme = () => {
try {
const saved = localStorage.getItem('theme');
if (saved === 'light' || saved === 'dark') return saved;
} catch {}
if (window.matchMedia && window.matchMedia('(prefers-color-scheme: dark)').matches) {
return 'dark';
}
return 'light';
};🤖 Prompt for AI Agents
In client/src/App.js around lines 12 to 16, the theme is not persisted nor
initialized from saved or system preference as claimed; add the provided
getInitialTheme helper function above "const App = () => {" and change the theme
state to use that helper as the initial value, then replace the separate effect
with a single useEffect that both sets
document.documentElement.setAttribute('data-theme', theme) and writes the theme
to localStorage (wrapped in try/catch to avoid errors in restricted
environments). Ensure saved values are validated to 'light' or 'dark' and fall
back to OS preference or 'light' when absent.
| <button | ||
| onClick={toggleTheme} | ||
| style={{ | ||
| position: 'fixed', | ||
| top: 10, | ||
| right: 10, | ||
| zIndex: 1000, | ||
| padding: '8px 16px', | ||
| borderRadius: '5px', | ||
| border: 'none', | ||
| background: 'var(--primary-color)', | ||
| color: 'var(--text-color)', | ||
| cursor: 'pointer', | ||
| }} | ||
| aria-label="Toggle theme" | ||
| > | ||
| Switch to {theme === 'dark' ? 'Light' : 'Dark'} Mode | ||
| </button> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix toggle button contrast and improve accessibility.
Use the new --on-primary-color for button text, and expose pressed state to ATs.
Apply this diff:
<button
onClick={toggleTheme}
+ type="button"
style={{
position: 'fixed',
top: 10,
right: 10,
zIndex: 1000,
padding: '8px 16px',
borderRadius: '5px',
border: 'none',
background: 'var(--primary-color)',
- color: 'var(--text-color)',
+ color: 'var(--on-primary-color)',
cursor: 'pointer',
}}
aria-label="Toggle theme"
+ aria-pressed={theme === 'dark'}
>Optional: move these inline styles into a CSS class to keep styling centralized with the rest of the theming.
📝 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.
| <button | |
| onClick={toggleTheme} | |
| style={{ | |
| position: 'fixed', | |
| top: 10, | |
| right: 10, | |
| zIndex: 1000, | |
| padding: '8px 16px', | |
| borderRadius: '5px', | |
| border: 'none', | |
| background: 'var(--primary-color)', | |
| color: 'var(--text-color)', | |
| cursor: 'pointer', | |
| }} | |
| aria-label="Toggle theme" | |
| > | |
| Switch to {theme === 'dark' ? 'Light' : 'Dark'} Mode | |
| </button> | |
| <button | |
| type="button" | |
| onClick={toggleTheme} | |
| style={{ | |
| position: 'fixed', | |
| top: 10, | |
| right: 10, | |
| zIndex: 1000, | |
| padding: '8px 16px', | |
| borderRadius: '5px', | |
| border: 'none', | |
| background: 'var(--primary-color)', | |
| color: 'var(--on-primary-color)', | |
| cursor: 'pointer', | |
| }} | |
| aria-label="Toggle theme" | |
| aria-pressed={theme === 'dark'} | |
| > | |
| Switch to {theme === 'dark' ? 'Light' : 'Dark'} Mode | |
| </button> |
🤖 Prompt for AI Agents
In client/src/App.js around lines 24 to 41, the theme toggle button uses the
wrong text color variable and lacks an accessible pressed state; change the
button text color to use CSS variable --on-primary-color instead of
--text-color, add an aria-pressed attribute that reflects the current state
(e.g., true when theme is 'dark', false otherwise) so assistive technologies can
detect the toggle state, and optionally move the inline style block into a CSS
class (using the same CSS variables) to centralize theming.
| color: var(--text-color) !important; | ||
| text-transform: uppercase; | ||
| text-decoration: none; | ||
| background: #2979FF; | ||
| background: var(--primary-color); | ||
| padding: 20px; |
There was a problem hiding this comment.
Fix contrast for primary buttons (text on primary).
color: var(--text-color) on background: var(--primary-color) will likely fail WCAG contrast in light mode (dark text on blue). Introduce an “on-primary” token and use it here to guarantee readable text across themes.
Apply this diff here (paired with adding --on-primary-color in theme.css):
- color: var(--text-color) !important;
+ color: var(--on-primary-color) !important;📝 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.
| color: var(--text-color) !important; | |
| text-transform: uppercase; | |
| text-decoration: none; | |
| background: #2979FF; | |
| background: var(--primary-color); | |
| padding: 20px; | |
| color: var(--on-primary-color) !important; | |
| text-transform: uppercase; | |
| text-decoration: none; | |
| background: var(--primary-color); | |
| padding: 20px; |
🤖 Prompt for AI Agents
In client/src/components/Input/Input.css around lines 21 to 25, the button text
uses color: var(--text-color) on background: var(--primary-color) which can fail
WCAG contrast in light mode; update the rule to use a dedicated on-primary token
instead: replace color: var(--text-color) with color: var(--on-primary-color)
(keep any necessary !important), and ensure you add and populate
--on-primary-color in theme.css for both light and dark themes with values that
meet contrast requirements against --primary-color.
| color: var(--text-color) !important; | ||
| text-transform: uppercase; | ||
| text-decoration: none; | ||
| background: var(--primary-color); | ||
| padding: 20px; | ||
| border-radius: 5px; | ||
| display: inline-block; | ||
| border: none; | ||
| width: 100%; |
There was a problem hiding this comment.
Fix button text contrast on primary background.
Like the Input send button, this is likely to fail contrast in light mode.
Apply this diff (paired with --on-primary-color in theme.css):
- color: var(--text-color) !important;
+ color: var(--on-primary-color) !important;Optional: add a visible focus style to retain keyboard accessibility (outline is removed later in the file). You can add the following rule elsewhere:
.button:focus-visible {
outline: 3px solid var(--on-primary-color);
outline-offset: 2px;
}🤖 Prompt for AI Agents
In client/src/components/Join/Join.css around lines 44 to 52, the button text
color on the primary background currently uses the page text color and may fail
contrast in light mode; change the text color to use the theme's on-primary
token (var(--on-primary-color)) instead of var(--text-color) so the label meets
contrast on the primary background, keep or remove !important as needed to
override other rules, and ensure you pair this with the existing
--on-primary-color in theme.css; also add a visible keyboard focus style
elsewhere (e.g., a :focus-visible outline using var(--on-primary-color) with an
offset) to preserve accessibility.
| :root { | ||
| --background-color: #fff; | ||
| --text-color: #222; | ||
| --primary-color: #2979FF; | ||
| --secondary-color: #f3f3f3; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add an “on-primary” token to guarantee readable text on primary backgrounds.
Buttons and CTAs use --primary-color as background. Provide a dedicated --on-primary-color so text reliably meets contrast in both themes (typically white on #2979FF).
Apply these diffs:
:root {
--background-color: #fff;
--text-color: #222;
--primary-color: #2979FF;
--secondary-color: #f3f3f3;
+ --on-primary-color: #fff;
} [data-theme="dark"] {
--background-color: #1a1a1a;
--text-color: #fff;
--primary-color: #2979FF;
--secondary-color: #23272a;
+ --on-primary-color: #fff;
}Also applies to: 9-14
🤖 Prompt for AI Agents
In client/src/theme.css around lines 2-7 (and similarly lines 9-14 for the other
theme block), add a new CSS custom property --on-primary-color to guarantee
readable text on primary backgrounds; set it to a high-contrast value (e.g.
#ffffff when primary is #2979FF) and update any components or button styles that
render text on --primary-color to use --on-primary-color instead of relying on
--text-color so contrast is consistent across themes.
1 participant
This pull request introduces an exciting Light/Dark Mode toggle, giving users full control over their browsing experience with a simple click. Whether you prefer a bright, vibrant look or a sleek, eye-friendly dark interface, you can now switch instantly without losing your settings—thanks to persistent theme storage via local storage. The design has been carefully crafted to ensure perfect color contrast, smooth transitions, and consistent styling across all pages. Tested on multiple browsers and devices, this enhancement not only improves accessibility but also adds a modern, professional touch to the application, making it more visually appealing and comfortable for prolonged use.
Summary by CodeRabbit
New Features
Style