Skip to content

feat: replace broken invert-filter light theme with real theming for app shell (#756)#766

Open
revatikadam0607 wants to merge 6 commits into
Coder-s-OG-s:mainfrom
revatikadam0607:feat/theme
Open

feat: replace broken invert-filter light theme with real theming for app shell (#756)#766
revatikadam0607 wants to merge 6 commits into
Coder-s-OG-s:mainfrom
revatikadam0607:feat/theme

Conversation

@revatikadam0607

Copy link
Copy Markdown

Summary

Closes #756

The sidebar already had a theme toggle button, but "light mode" was implemented as a CSS filter
(invert(1) hue-rotate(180deg)) applied to the whole page. That inverted images, videos, and
brand colors instead of actually re-theming the UI.

This PR replaces that hack with real CSS-variable-based theming for the app shell (sidebar + main
content area):

  • Added --shell-bg, --shell-bg-card, --shell-border, --shell-text, --shell-text-muted
    CSS variables in globals.css, with light-mode overrides under html[data-ms-theme="light"].
  • Updated layout.tsx and sidebar.tsx to use those variables instead of hardcoded hex/zinc
    colors.
  • The existing ThemeToggle component already persists the choice to localStorage
    (ms-theme) and applies it on load — no changes were needed there.

Scope

This covers the app shell (sidebar + page background), the largest and most visible surface.
Individual page components (dashboard cards, forms, tables, etc.) still use hardcoded dark colors
and aren't re-themed yet — that's a bigger effort across many more files. Happy to follow up with
additional PRs reusing the same --shell-* pattern if that's useful.

Testing

  • npm run dev, toggled theme from the sidebar — colors swap correctly, no more inverted look.
  • Refreshed the page — theme choice persists via localStorage.
  • npx tsc --noEmit passes for the changed files.

I've resolved the issue. Kindly review it, and if everything looks good, please consider merging the corresponding PR.
Hi @codersogs-code , could you please review this merged PR for a potential bonus label based on the metrics below? Thanks!
labels

@vercel

vercel Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

@revatikadam0607 is attempting to deploy a commit to the codersogs-3057's projects Team on Vercel.

A member of the Team first needs to authorize it.

@jakharmonika364

jakharmonika364 commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

@revatikadam0607 CI failure, can you please fix it

@revatikadam0607

Copy link
Copy Markdown
Author

@revatikadam0607 CI failure, can you please fix it

Sorry for your inconvenience
Let me check it

@Siddhartha-singh01 Siddhartha-singh01 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Review: feat: replace broken invert-filter light theme with real theming (#766 / #756)

Thanks @revatikadam0607 this is the right fix direction for #756. The old filter: invert(1) hue-rotate(180deg) “light mode” was a hack that wrecked images, brand greens, and overall contrast. Replacing it with CSS variables on the app shell is the correct foundation.

Verdict

Request changes CI Format check is still red, and layout.tsx formatting in the diff is broken. After Prettier is clean, this is close to approve as a scoped shell theming step (with the partial-coverage caveats below).


Blocker: CI Format check failing

Latest check fails on Prettier:

[warn] src/app/(app)/layout.tsx [warn] src/app/(app)/sidebar.tsx Code style issues found in 2 files. Run Prettier .

layout.tsx in the PR diff is also visibly smashed onto one line:

<div className="flex h-screen overflow-hidden bg-[var(--shell-bg)] font-mono text-[var(--shell-text)]">        <Sidebar

That is both a style failure and hard to maintain.

Please run:

npm run format
# or
npx prettier --write "src/app/(app)/layout.tsx" "src/app/(app)/sidebar.tsx" "src/app/globals.css"

@Siddhartha-singh01 Siddhartha-singh01 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-review: feat: replace broken invert-filter light theme (#766 / #756)

Thanks for the updates @revatikadam0607 (now 4 commits on feat/theme). Re-checked head 8c9a42e.

Verdict

Still request changes only because CI Format check is still failing.

Feature direction is still correct and the previous broken one-line layout.tsx issue is fixed. Typecheck and lint passed on the latest run; Prettier did not.


What improved

  1. layout.tsx is readable again

    • Shell uses bg-[var(--shell-bg)] + text-[var(--shell-text)]
    • Sidebar is properly nested (no smashed div + <Sidebar on one line)
  2. sidebar.tsx shell tokens look cleaner

    • Borders / cards / muted labels / primary text use --shell-*
    • Props wrapping is more consistent than the first commit
  3. globals.css still does the real fix for #756

    • Removes filter: invert(1) hue-rotate(180deg)
    • Adds dark defaults + light overrides under html[data-ms-theme="light"]

Core theming approach remains approve-worthy once CI is green.


Blocker: Prettier still red

Latest check (30019629024, head 8c9a42ec…):

[warn] src/app/(app)/layout.tsx [warn] src/app/(app)/sidebar.tsx Code style issues found in 2 files. Run Prettier with --write to fix.

Hand-formatting / partial refactors are not enough. CI only accepts what Prettier emits.

Please run this exactly, then commit only the Prettier output:

npm run format
# or explicitly:
npx prettier --write "src/app/(app)/layout.tsx" "src/app/(app)/sidebar.tsx" "src/app/globals.css"

npm run format:check

@revatikadam0607

Copy link
Copy Markdown
Author

Re-review: feat: replace broken invert-filter light theme (#766 / #756)

Thanks for the updates @revatikadam0607 (now 4 commits on feat/theme). Re-checked head 8c9a42e.

Verdict

Still request changes only because CI Format check is still failing.

Feature direction is still correct and the previous broken one-line layout.tsx issue is fixed. Typecheck and lint passed on the latest run; Prettier did not.

What improved

  1. layout.tsx is readable again

    • Shell uses bg-[var(--shell-bg)] + text-[var(--shell-text)]
    • Sidebar is properly nested (no smashed div + <Sidebar on one line)
  2. sidebar.tsx shell tokens look cleaner

    • Borders / cards / muted labels / primary text use --shell-*
    • Props wrapping is more consistent than the first commit
  3. globals.css still does the real fix for Implement Dark/Light Theme Toggle with Persistent User Preferences #756

    • Removes filter: invert(1) hue-rotate(180deg)
    • Adds dark defaults + light overrides under html[data-ms-theme="light"]

Core theming approach remains approve-worthy once CI is green.

Blocker: Prettier still red

Latest check (30019629024, head 8c9a42ec…):

[warn] src/app/(app)/layout.tsx [warn] src/app/(app)/sidebar.tsx Code style issues found in 2 files. Run Prettier with --write to fix.

Hand-formatting / partial refactors are not enough. CI only accepts what Prettier emits.

Please run this exactly, then commit only the Prettier output:

npm run format
# or explicitly:
npx prettier --write "src/app/(app)/layout.tsx" "src/app/(app)/sidebar.tsx" "src/app/globals.css"

npm run format:check

Now?

@Siddhartha-singh01 Siddhartha-singh01 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-review: shell theming (#766 / #756) head bd189b7 (6 commits)

Thanks for the extra commits @revatikadam0607. Re-checked the latest head.

Verdict

Still request changes

Theming logic is still good. Formatting is not fixed and the newest commits actually reintroduced messy indentation. CI check is still red on Format check only.


What is still correct (feature)

  1. Invert-filter light mode removed from globals.css.
  2. Real --shell-* tokens with light overrides under html[data-ms-theme="light"].
  3. Layout shell + sidebar use those variables for bg / border / text / cards.
  4. Typecheck + lint pass; only Prettier fails.

This remains the right fix for #756 once CI is green.


Blocker: Prettier still failing and latest diffs look worse

Latest CI (30019938750):

[warn] src/app/(app)/layout.tsx [warn] src/app/(app)/sidebar.tsx Code style issues found in 2 files. Run Prettier with --write to fix.

Looking at the current diff, hand-formatting is fighting Prettier:

layout.tsx uneven indent on the shell wrapper:

     <div
  className="flex h-screen overflow-hidden bg-[var(--shell-bg)] font-mono text-[var(--shell-text)]"
>

sidebar.tsx  aside jammed with the next line:

<aside
  className="... bg-[var(--shell-bg)]"
>      <div className="min-h-0 flex-1 overflow-y-auto">

Please stop editing indentation by hand. CI only accepts Prettier’s output.

Exact fix (do this and nothing else)

From a clean clone of your branch:

npm install
npx prettier --write "src/app/(app)/layout.tsx" "src/app/(app)/sidebar.tsx" "src/app/globals.css"
npm run format:check

You must see:

All matched files use Prettier code style!

Then:

git add src/app/(app)/layout.tsx src/app/(app)/sidebar.tsx src/app/globals.css
git commit -m "style: apply prettier to theme shell files"
git push

@Siddhartha-singh01

Copy link
Copy Markdown
Collaborator

@revatikadam0607 If your PR fail one more time we are going to close the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement Dark/Light Theme Toggle with Persistent User Preferences

3 participants