-
Notifications
You must be signed in to change notification settings - Fork 2k
fix(source filter): dark mode support #5505
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
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
Greptile Overview
Summary
This PR implements dark mode support for source filter components by updating color classes from legacy text-*
and background-*
to standardized neutral-*
variants with dark mode variants.
Key Changes:
- ActionManagement.tsx: Updated color classes throughout component, improved dropdown styling with proper dark mode backgrounds/shadows, and added SVG filter styling for icons
- Dropdown.tsx: Replaced legacy color classes with neutral variants for consistent theming
- TagFilter.tsx: Updated border colors to use neutral color classes
- hooks.ts: Cleaned up useEffect dependency array by removing unused
llmProviders
Issues Found:
- Inline styling with DOM queries for dark mode detection could cause SSR hydration mismatches and may not update properly with theme changes
Confidence Score: 4/5
- This PR is safe to merge with minor style considerations
- The changes are primarily cosmetic color class updates for dark mode support. The inline SVG styling approach could be improved but doesn't break functionality. The hooks dependency cleanup is correct.
- ActionManagement.tsx requires attention for the inline dark mode detection approach
Important Files Changed
File Analysis
Filename | Score | Overview |
---|---|---|
web/src/app/chat/components/input/ActionManagement.tsx | 4/5 | Updated color classes from text-* to neutral-* for dark mode support, improved styling for dropdown and toggle components |
web/src/components/Dropdown.tsx | 5/5 | Replaced text-* and background-* color classes with neutral-* variants for proper dark mode theming |
Sequence Diagram
sequenceDiagram
participant User
participant Browser
participant ActionManagement
participant Dropdown
participant TagFilter
participant Theme
User->>Browser: Toggle dark mode
Browser->>Theme: Update documentElement.classList
Theme->>ActionManagement: Apply dark mode styles
ActionManagement->>ActionManagement: Update neutral-* color classes
ActionManagement->>ActionManagement: Apply inline SVG filter styles
Theme->>Dropdown: Apply dark mode styles
Dropdown->>Dropdown: Update neutral-* background/text colors
Theme->>TagFilter: Apply dark mode styles
TagFilter->>TagFilter: Update neutral-* border colors
User->>ActionManagement: Interact with source filters
ActionManagement->>User: Display with proper dark mode styling
3 files reviewed, 1 comment
filter: | ||
document.documentElement.classList.contains( | ||
"dark" | ||
) | ||
? "invert(0.6) sepia(1) saturate(0) hue-rotate(0deg) brightness(1.2)" | ||
: "invert(0.4) sepia(1) saturate(0) hue-rotate(0deg) brightness(0.8)", | ||
opacity: | ||
document.documentElement.classList.contains( | ||
"dark" | ||
) | ||
? 0.9 | ||
: 0.7, | ||
}} |
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.
style: The inline style with document.documentElement.classList.contains("dark")
checks can cause hydration mismatches in SSR and may not update properly with theme changes. Consider using CSS variables or a theme context instead.
filter: | |
document.documentElement.classList.contains( | |
"dark" | |
) | |
? "invert(0.6) sepia(1) saturate(0) hue-rotate(0deg) brightness(1.2)" | |
: "invert(0.4) sepia(1) saturate(0) hue-rotate(0deg) brightness(0.8)", | |
opacity: | |
document.documentElement.classList.contains( | |
"dark" | |
) | |
? 0.9 | |
: 0.7, | |
}} | |
className="absolute right-3 top-1/2 transform -translate-y-1/2 w-4 h-4 pointer-events-none dark:invert-[0.6] dark:brightness-125 invert-[0.4] brightness-75 dark:opacity-90 opacity-70" |
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.
1 issue found across 3 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="web/src/app/chat/components/input/ActionManagement.tsx">
<violation number="1" location="web/src/app/chat/components/input/ActionManagement.tsx:983">
Inline style reads document.documentElement.classList in render to set theme-specific filter/opacity, making them non-reactive to dark-mode toggles and coupling logic to DOM; prefer CSS (dark: classes) or state-driven updates.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai
to give feedback, ask questions, or re-run the review.
alt="Disable All Sources" | ||
className="absolute right-3 top-1/2 transform -translate-y-1/2 w-4 h-4 pointer-events-none opacity-60" | ||
className="absolute right-3 top-1/2 transform -translate-y-1/2 w-4 h-4 pointer-events-none" | ||
style={{ |
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.
Inline style reads document.documentElement.classList in render to set theme-specific filter/opacity, making them non-reactive to dark-mode toggles and coupling logic to DOM; prefer CSS (dark: classes) or state-driven updates.
Prompt for AI agents
Address the following comment on web/src/app/chat/components/input/ActionManagement.tsx at line 983:
<comment>Inline style reads document.documentElement.classList in render to set theme-specific filter/opacity, making them non-reactive to dark-mode toggles and coupling logic to DOM; prefer CSS (dark: classes) or state-driven updates.</comment>
<file context>
@@ -972,18 +971,29 @@ export function ActionToggle({
alt="Disable All Sources"
- className="absolute right-3 top-1/2 transform -translate-y-1/2 w-4 h-4 pointer-events-none opacity-60"
+ className="absolute right-3 top-1/2 transform -translate-y-1/2 w-4 h-4 pointer-events-none"
+ style={{
+ filter:
+ document.documentElement.classList.contains(
</file context>
✅ Addressed in 2b7b7a7
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.
Trust it looks good!
Description
[Provide a brief description of the changes in this PR]
How Has This Been Tested?
[Describe the tests you ran to verify your changes]
Additional Options