Skip to content

style: fix dark/light colors #293

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.

Sign up for GitHub

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

Merged
chmurson merged 1 commit into from
Aug 26, 2025
Merged

style: fix dark/light colors #293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/NoteManager/components/NotesList.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export const NotesList = ({
onDeleteNote: (noteToDelete: IDecoratedNote) => void;
}) => {
if (notes.length === 0) {
return <div className="no-notes">No notes available.</div>;
return <div className="no-notes text-sidebar-foreground">No notes available.</div>;
}

return (
Expand Down
9 changes: 4 additions & 5 deletions src/components/PdfViewer/PdfViewer.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
min-width: fit-content;
}

.pdfViewer .page{
background-color: #101010;
}
Comment on lines +20 to +22
Copy link

@coderabbitai coderabbitai bot Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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.


.pdfViewer.light .page{
filter: invert(1) contrast(1.6);
}
Expand All @@ -40,8 +44,3 @@
.pdfViewer .textLayer ::selection {
background: rgba(0, 50, 255, 0.25);
}

.pdfViewer .page {
background-color: var(--sidebar)

}
2 changes: 1 addition & 1 deletion src/components/Search/Search.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ function SearchResults({ query, onSearchFinished }: SearchResultsProps) {
➡️
</button>
</div>
<div className={`search-results ${isLoading ? "search-loading" : ""}`}>
<div className={`search-results text-sidebar-foreground ${isLoading ? "search-loading" : ""}`}>
Found {matches.count} results on {matches.pagesAndCount.length} pages.
<ul>
{matches.pagesAndCount.map((res) => (
Expand Down
14 changes: 14 additions & 0 deletions src/tailwind.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,18 @@
--brand-dark: rgb(18 152 141);
--brand-very-dark: rgb(14 116 107);
--brand-darkest: rgb(14 53 50);

--sidebar-foreground: var(--color-gray-600);

color-scheme: light;
}

.dark{
--sidebar-foreground: rgb(255 255 255);
--sidebar-foreground: var(--color-gray-300);
/* Force light mode rendering */

color-scheme: dark;
}
Comment on lines +18 to 24
Copy link

@coderabbitai coderabbitai bot Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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.css

Length 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.


@theme{
Expand All @@ -19,6 +31,8 @@
--color-brand-light: var(--brand-light);
--color-brand-very-light: var(--brand-very-light);

--color-sidebar-foreground: var(--sidebar-foreground);

--animate-fade-in: fade-in 0.75s ease-in forwards;

@keyframes fade-in {
Expand Down