Conversation
Implements theme switchi
using Tailwind CSS dark mode 'class' strategy. Includes ThemeContext, ThemeToggle component, and extensive style updates across
components to support both themes.
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive light/dark theme support to the frontend application using Tailwind CSS's class-based dark mode. The implementation includes a theme toggle button, context-based theme management with localStorage persistence, and system preference detection.
Key changes:
- Theme context and provider with localStorage persistence and system preference detection
- Theme toggle component with accessible controls and icon switching
- Complete UI update with dark mode variants for all color classes across components and CSS utilities
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/tailwind.config.js | Enables class-based dark mode strategy |
| frontend/src/contexts/ThemeContext.jsx | Creates theme context with state management, persistence, and system preference detection |
| frontend/src/hooks/useTheme.js | Provides theme hook with error boundary for context usage |
| frontend/src/main.jsx | Wraps app with ThemeProvider |
| frontend/src/components/ThemeToggle.jsx | Adds accessible theme toggle button with sun/moon icons |
| frontend/src/styles/index.css | Updates all CSS utility classes with dark mode variants |
| frontend/src/components/SynthesisPanel.jsx | Applies theme-aware color classes throughout |
| frontend/src/components/QueryInput.jsx | Updates labels, inputs, and model cards with dark mode support |
| frontend/src/components/HistoryBrowser.jsx | Applies theme-aware styling to history items |
| frontend/src/components/CouncilMemberCard.jsx | Updates card styling with dark mode variants |
| frontend/src/App.jsx | Adds ThemeToggle to header and updates all color classes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,35 @@ | |||
| import { createContext, useState, useEffect } from 'react'; | |||
|
|
|||
| export const ThemeContext = createContext(); | |||
There was a problem hiding this comment.
The ThemeContext is created without a default value. While this works with the error handling in useTheme, providing a default value (even if it's undefined) makes the context type clearer and more explicit. Consider: 'export const ThemeContext = createContext(undefined);'
| export const ThemeContext = createContext(); | |
| export const ThemeContext = createContext(undefined); |
| </div> | ||
| </div> | ||
| <span className="text-xs text-gray-500 whitespace-nowrap ml-2"> | ||
| <span className="text-xs text-gray-400 dark:text-gray-500 whitespace-nowrap ml-2"> |
There was a problem hiding this comment.
The color progression for the date timestamp is reversed. In light mode, it should be lighter (gray-400) and in dark mode it should be darker (gray-500). Currently it's set to gray-400 in light mode and gray-500 in dark mode, which means the text will be darker in dark mode, making it less readable. It should be 'text-gray-500 dark:text-gray-400' to maintain proper contrast in both modes.
| <span className="text-xs text-gray-400 dark:text-gray-500 whitespace-nowrap ml-2"> | |
| <span className="text-xs text-gray-500 dark:text-gray-400 whitespace-nowrap ml-2"> |
| className="bg-green-100 dark:bg-green-900/20 border-l-4 border-green-500 p-3 rounded" | ||
| > | ||
| <div className="text-sm text-gray-300 prose prose-invert prose-sm max-w-none"> | ||
| <div className="text-sm text-gray-700 dark:text-gray-300 prose dark:prose-invert prose-sm max-w-none"> |
There was a problem hiding this comment.
The prose classes are being applied alongside explicit text color classes. When using Tailwind Typography's 'prose' class, the text colors are managed by the prose styles, so explicitly setting 'text-gray-700 dark:text-gray-300' may conflict with prose's internal color scheme. Either remove the explicit text color classes and let prose handle the colors, or ensure the text colors properly override prose's defaults for your use case.
| className="bg-orange-100 dark:bg-orange-900/20 border-l-4 border-orange-500 p-3 rounded" | ||
| > | ||
| <div className="font-medium text-sm mb-1 text-gray-200 prose prose-invert prose-sm max-w-none"> | ||
| <div className="font-medium text-sm mb-1 text-gray-800 dark:text-gray-200 prose dark:prose-invert prose-sm max-w-none"> |
There was a problem hiding this comment.
The prose classes are being applied alongside explicit text color classes. When using Tailwind Typography's 'prose' class, the text colors are managed by the prose styles, so explicitly setting 'text-gray-800 dark:text-gray-200' may conflict with prose's internal color scheme. Either remove the explicit text color classes and let prose handle the colors, or ensure the text colors properly override prose's defaults for your use case.
| </div> | ||
| {debate.positions && ( | ||
| <div className="text-sm text-gray-300 prose prose-invert prose-sm max-w-none"> | ||
| <div className="text-sm text-gray-700 dark:text-gray-300 prose dark:prose-invert prose-sm max-w-none"> |
There was a problem hiding this comment.
The prose classes are being applied alongside explicit text color classes. When using Tailwind Typography's 'prose' class, the text colors are managed by the prose styles, so explicitly setting 'text-gray-700 dark:text-gray-300' may conflict with prose's internal color scheme. Either remove the explicit text color classes and let prose handle the colors, or ensure the text colors properly override prose's defaults for your use case.
| <div className="bg-purple-900/20 border-l-4 border-purple-500 p-4 rounded"> | ||
| <div className="prose prose-invert prose-sm max-w-none"> | ||
| <div className="bg-purple-100 dark:bg-purple-900/20 border-l-4 border-purple-500 p-4 rounded"> | ||
| <div className="prose dark:prose-invert prose-sm max-w-none text-gray-900 dark:text-gray-100"> |
There was a problem hiding this comment.
The prose classes are being applied alongside explicit text color classes. When using Tailwind Typography's 'prose' class, the text colors are managed by the prose styles, so explicitly setting 'text-gray-900 dark:text-gray-100' may conflict with prose's internal color scheme. Either remove the explicit text color classes and let prose handle the colors, or ensure the text colors properly override prose's defaults for your use case.
| <p className="text-red-600 dark:text-red-400">{response.error}</p> | ||
| ) : response.text ? ( | ||
| <div className="prose prose-invert prose-sm max-w-none"> | ||
| <div className="prose dark:prose-invert prose-sm max-w-none text-gray-900 dark:text-gray-100"> |
There was a problem hiding this comment.
The prose classes are being applied alongside explicit text color classes. When using Tailwind Typography's 'prose' class, the text colors are managed by the prose styles, so explicitly setting 'text-gray-900 dark:text-gray-100' may conflict with prose's internal color scheme. Either remove the explicit text color classes and let prose handle the colors, or ensure the text colors properly override prose's defaults for your use case.
| <div className="prose dark:prose-invert prose-sm max-w-none text-gray-900 dark:text-gray-100"> | |
| <div className="prose dark:prose-invert prose-sm max-w-none"> |
| export function ThemeProvider({ children }) { | ||
| const [theme, setTheme] = useState(() => { | ||
| // Check local storage first | ||
| const savedTheme = localStorage.getItem('theme'); | ||
| if (savedTheme) { | ||
| return savedTheme; | ||
| } | ||
| // Check system preference | ||
| if (window.matchMedia('(prefers-color-scheme: dark)').matches) { | ||
| return 'dark'; | ||
| } | ||
| return 'light'; | ||
| }); | ||
|
|
||
| useEffect(() => { | ||
| const root = window.document.documentElement; | ||
| root.classList.remove('light', 'dark'); | ||
| root.classList.add(theme); | ||
| localStorage.setItem('theme', theme); |
There was a problem hiding this comment.
Accessing localStorage directly in the initial state could cause issues in server-side rendering environments or if localStorage is unavailable. Consider wrapping localStorage access in a try-catch block or checking if window is defined to handle these edge cases gracefully.
| export function ThemeProvider({ children }) { | |
| const [theme, setTheme] = useState(() => { | |
| // Check local storage first | |
| const savedTheme = localStorage.getItem('theme'); | |
| if (savedTheme) { | |
| return savedTheme; | |
| } | |
| // Check system preference | |
| if (window.matchMedia('(prefers-color-scheme: dark)').matches) { | |
| return 'dark'; | |
| } | |
| return 'light'; | |
| }); | |
| useEffect(() => { | |
| const root = window.document.documentElement; | |
| root.classList.remove('light', 'dark'); | |
| root.classList.add(theme); | |
| localStorage.setItem('theme', theme); | |
| function getInitialTheme() { | |
| // Default to light if window is not available (e.g., SSR) | |
| if (typeof window === 'undefined') { | |
| return 'light'; | |
| } | |
| try { | |
| // Check local storage first | |
| const savedTheme = window.localStorage.getItem('theme'); | |
| if (savedTheme) { | |
| return savedTheme; | |
| } | |
| } catch (e) { | |
| // Ignore localStorage errors and fall back to system preference / default | |
| } | |
| try { | |
| // Check system preference | |
| if (window.matchMedia && window.matchMedia('(prefers-color-scheme: dark)').matches) { | |
| return 'dark'; | |
| } | |
| } catch (e) { | |
| // Ignore matchMedia errors and fall back to default | |
| } | |
| return 'light'; | |
| } | |
| export function ThemeProvider({ children }) { | |
| const [theme, setTheme] = useState(getInitialTheme); | |
| useEffect(() => { | |
| if (typeof window === 'undefined') { | |
| return; | |
| } | |
| try { | |
| const root = window.document.documentElement; | |
| root.classList.remove('light', 'dark'); | |
| root.classList.add(theme); | |
| window.localStorage.setItem('theme', theme); | |
| } catch (e) { | |
| // Ignore errors interacting with DOM or localStorage | |
| } |
Refactor ThemeContext for robustness and safety. Fix
HistoryBrowser timestamp contrast. Remove conflicting prose text colors.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <svg | ||
| className="w-5 h-5" | ||
| fill="none" | ||
| stroke="currentColor" | ||
| viewBox="0 0 24 24" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| > | ||
| <path | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| strokeWidth={2} | ||
| d="M12 3v1m0 16v1m9-9h-1M4 12H3m15.364 6.364l-.707-.707M6.343 6.343l-.707-.707m12.728 0l-.707.707M6.343 17.657l-.707.707M16 12a4 4 0 11-8 0 4 4 0 018 0z" | ||
| /> | ||
| </svg> |
There was a problem hiding this comment.
The SVG icons lack proper accessibility attributes. While the button has an aria-label, the SVGs themselves should have aria-hidden="true" since they're decorative and the button already has a text label for screen readers. This prevents screen readers from announcing redundant information.
| <svg | ||
| className="w-5 h-5" | ||
| fill="none" | ||
| stroke="currentColor" | ||
| viewBox="0 0 24 24" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| > | ||
| <path | ||
| strokeLinecap="round" | ||
| strokeLinejoin="round" | ||
| strokeWidth={2} | ||
| d="M20.354 15.354A9 9 0 018.646 3.646 9.003 9.003 0 0012 21a9.003 9.003 0 008.354-5.646z" | ||
| /> | ||
| </svg> |
There was a problem hiding this comment.
The SVG icons lack proper accessibility attributes. While the button has an aria-label, the SVGs themselves should have aria-hidden="true" since they're decorative and the button already has a text label for screen readers. This prevents screen readers from announcing redundant information.
| .warning.severity-high { | ||
| @apply bg-red-900/20 border border-red-700 text-red-300; | ||
| @apply bg-red-100 dark:bg-red-900/20 border border-red-200 dark:border-red-700 text-red-700 dark:text-red-300; | ||
| } | ||
|
|
||
| .warning.severity-medium { | ||
| @apply bg-yellow-900/20 border border-yellow-700 text-yellow-300; | ||
| @apply bg-yellow-100 dark:bg-yellow-900/20 border border-yellow-200 dark:border-yellow-700 text-yellow-700 dark:text-yellow-300; | ||
| } | ||
|
|
||
| .warning.severity-info { | ||
| @apply bg-blue-900/20 border border-blue-700 text-blue-300; | ||
| @apply bg-blue-100 dark:bg-blue-900/20 border border-blue-200 dark:border-blue-700 text-blue-700 dark:text-blue-300; | ||
| } | ||
|
|
||
| /* Warning Items (for SelectionAnalysis) */ | ||
| .warning-item { | ||
| @apply px-3 py-2 rounded-lg; | ||
| } | ||
|
|
||
| .warning-item.severity-high { | ||
| @apply bg-red-100 dark:bg-red-900/20 border border-red-200 dark:border-red-700 text-red-700 dark:text-red-300; | ||
| } | ||
|
|
||
| .warning-item.severity-medium { | ||
| @apply bg-yellow-100 dark:bg-yellow-900/20 border border-yellow-200 dark:border-yellow-700 text-yellow-700 dark:text-yellow-300; | ||
| } | ||
|
|
||
| .warning-item.severity-info { | ||
| @apply bg-blue-100 dark:bg-blue-900/20 border border-blue-200 dark:border-blue-700 text-blue-700 dark:text-blue-300; | ||
| } |
There was a problem hiding this comment.
The .warning classes (lines 110-120) and .warning-item classes (lines 127-137) have duplicate styling for severity variants. This creates maintainability issues - if colors need to be updated for a severity level, they must be changed in two places. Consider consolidating these into a single set of classes or using CSS composition to avoid duplication.
| @layer base { | ||
| body { | ||
| @apply bg-[#0a0e1a] text-gray-100; | ||
| @apply bg-gray-50 dark:bg-[#0a0e1a] text-gray-900 dark:text-gray-100 transition-colors duration-200; |
There was a problem hiding this comment.
The body tag's background color is set to bg-gray-50 for light mode and dark:bg-[#0a0e1a] for dark mode. However, the main app container in App.jsx has the same classes, which creates redundancy. The body tag's background should be sufficient. Consider removing the background classes from the App.jsx container or from the body, not both, to avoid duplication.
| @apply bg-gray-50 dark:bg-[#0a0e1a] text-gray-900 dark:text-gray-100 transition-colors duration-200; | |
| @apply text-gray-900 dark:text-gray-100 transition-colors duration-200; |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This PR implements light and dark theme support using Tailwind CSS. It adds a theme toggle, persists user preference, and updates all UI components to support both modes.
Closes #4