style: fix dark/light colors #293
Conversation
WalkthroughUpdates apply presentational styling: adds text color classes to NotesList and Search empty/results states, changes PdfViewer page background to a fixed dark color, and introduces a new CSS variable --sidebar-foreground with a theme alias in tailwind.css. No logic, state, or public API changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
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
Status, Documentation and Community
|
✅ Deploy Preview for graypaper-reader ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/Search/Search.tsx (1)
177-177: Use twMerge instead of string interpolation for class merging.Keeps Tailwind class handling consistent in this file (you already import
twMergeabove) and avoids accidental duplicates.Apply:
- <div className={`search-results text-sidebar-foreground ${isLoading ? "search-loading" : ""}`}> + <div className={twMerge("search-results text-sidebar-foreground", isLoading && "search-loading")}>
📜 Review details
Configuration used: Path: .coderabbit.yaml
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 selected for processing (4)
src/components/NoteManager/components/NotesList.tsx(1 hunks)src/components/PdfViewer/PdfViewer.css(1 hunks)src/components/Search/Search.tsx(1 hunks)src/tailwind.css(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,jsx,ts,tsx}
⚙️ CodeRabbit configuration file
When reviewing Tailwind CSS classes, ensure they follow Tailwind 4.x conventions and suggest modern 4.x alternatives for deprecated patterns.
Files:
src/components/NoteManager/components/NotesList.tsxsrc/components/Search/Search.tsx
**/*.css
⚙️ CodeRabbit configuration file
Review CSS code with focus on Tailwind CSS 4.x best practices and conventions. Ensure compatibility with Tailwind 4.x features and syntax.
Files:
src/components/PdfViewer/PdfViewer.csssrc/tailwind.css
🔇 Additional comments (3)
src/components/NoteManager/components/NotesList.tsx (1)
15-15: Semantic color token usage looks good.Applying
text-sidebar-foregroundto the empty state aligns with the new Tailwind v4@themecolor alias and keeps the UI consistent across themes.src/tailwind.css (2)
13-16: Good: introduces a semantic foreground variable and sets the default color-scheme.Using
--sidebar-foregroundwithcolor-scheme: lightat:rootis aligned with Tailwind v4 token-driven theming.
34-35: Theme alias for Tailwind v4 utilities is correct.
--color-sidebar-foregroundexposestext-sidebar-foregroundwhich you’re using in components—this is the recommended v4 pattern.
| .pdfViewer .page{ | ||
| background-color: #101010; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid hard-coding the page background; keep it themable.
Replacing var(--sidebar) with #101010 makes the PDF page background non-configurable and may diverge from light/gray modes or future themes. Prefer a semantic token with a safe fallback.
Apply:
-.pdfViewer .page{
- background-color: #101010;
-}
+.pdfViewer .page{
+ /* Use a semantic token so themes can control page bg; default to current dark */
+ background-color: var(--page-bg, #101010);
+}Then define the token (outside this hunk) to preserve dark/light behavior. For example, in your theme stylesheet:
:root { --page-bg: var(--color-gray-50); } /* light/gray default */
.dark { --page-bg: #101010; } /* dark override */Alternatively, if var(--sidebar) was the intended source of truth, consider reverting to it to keep behavior aligned with the rest of the app theme.
I can wire this token into @theme as --color-page-bg to get bg-page-bg utilities if you prefer Tailwind-first usage.
🤖 Prompt for AI Agents
In src/components/PdfViewer/PdfViewer.css around lines 20 to 22, the PDF page
background is hard-coded to #101010 which breaks theming; replace the literal
color with a CSS variable (e.g., var(--page-bg) or revert to var(--sidebar)) so
the background is themeable and provide a safe fallback (e.g., var(--page-bg,
#101010)); then add the --page-bg token in the global theme stylesheet with a
light default and a .dark override (or wire it into @theme as --color-page-bg)
so the component respects light/dark and future themes.
| .dark{ | ||
| --sidebar-foreground: rgb(255 255 255); | ||
| --sidebar-foreground: var(--color-gray-300); | ||
| /* Force light mode rendering */ | ||
|
|
||
| color-scheme: dark; | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Duplicate assignment of --sidebar-foreground; second one overrides the first. Clarify intent and keep a single value.
In .dark you set:
--sidebar-foreground: rgb(255 255 255);- then immediately
--sidebar-foreground: var(--color-gray-300);(this wins)
If white was intended, the current code never uses it. If gray-300 was intended, drop the first line. You can also use a CSS var fallback if you truly want “use gray-300 if defined, otherwise white”.
Pick one of these:
Option A (prefer gray-300):
.dark{
- --sidebar-foreground: rgb(255 255 255);
- --sidebar-foreground: var(--color-gray-300);
+ --sidebar-foreground: var(--color-gray-300);
/* Force light mode rendering */
color-scheme: dark;
}Option B (prefer white with gray-300 fallback if token is missing):
.dark{
- --sidebar-foreground: rgb(255 255 255);
- --sidebar-foreground: var(--color-gray-300);
+ --sidebar-foreground: var(--color-gray-300, rgb(255 255 255));
/* Force light mode rendering */
color-scheme: dark;
}Also, the comment “Force light mode rendering” inside the dark block is misleading. Consider updating it:
- /* Force light mode rendering */
+ /* Advertise dark color scheme to the UA */Please sanity-check contrast of text-sidebar-foreground on dark surfaces after this change in both themes.
🏁 Script executed:
#!/bin/bash
# Locate the `.dark` rule in src/tailwind.css and show its contents with surrounding context
rg -nP '\.dark\s*\{' -C5 src/tailwind.cssLength of output: 306
Remove or consolidate the duplicate --sidebar-foreground assignment in .dark
The .dark rule in src/tailwind.css currently sets:
.dark {
--sidebar-foreground: rgb(255 255 255);
--sidebar-foreground: var(--color-gray-300);
/* Force light mode rendering */
color-scheme: dark;
}The second declaration immediately overrides the first—so white is never used. Decide which token you actually need, or combine them via a fallback. Also update the misleading comment inside a dark‐mode block.
Locations to update:
src/tailwind.css:18–24
Suggested fixes (pick one):
Option A: Prefer gray-300
.dark {
- --sidebar-foreground: rgb(255 255 255);
- --sidebar-foreground: var(--color-gray-300);
+ --sidebar-foreground: var(--color-gray-300);
/* Advertise dark color scheme to the UA */
color-scheme: dark;
}Option B: Prefer white, fall back to gray-300
.dark {
- --sidebar-foreground: rgb(255 255 255);
- --sidebar-foreground: var(--color-gray-300);
+ --sidebar-foreground: var(--color-gray-300, rgb(255 255 255));
/* Advertise dark color scheme to the UA */
color-scheme: dark;
}• Replace the comment /* Force light mode rendering */ with something like /* Advertise dark color scheme to the UA */.
• After you update these values, please sanity-check the contrast of your .text-sidebar-foreground utility on dark backgrounds.
🤖 Prompt for AI Agents
In src/tailwind.css around lines 18–24, the .dark rule has a duplicate
--sidebar-foreground that overrides the first; remove the duplicate and
implement one of the suggested fixes: either set --sidebar-foreground:
var(--color-gray-300); (Option A) or set --sidebar-foreground: rgb(255 255 255,
fallback to var(--color-gray-300)) using a CSS fallback pattern (Option B);
replace the misleading comment "/* Force light mode rendering */" with "/*
Advertise dark color scheme to the UA */"; after making the change, verify the
.text-sidebar-foreground utility still meets contrast requirements on dark
backgrounds.
1 participant
What?
light-dark() functionwas not setup with app's dark/light themeSummary by CodeRabbit