Skip to content

feat(theme): modular CSS, light/dark/auto theme system (Phase 1 - Foundation PR 1)#377

Draft
vitorvasc wants to merge 38 commits intoopen-telemetry:mainfrom
vitorvasc:feat/84-pr1-theme-system
Draft

feat(theme): modular CSS, light/dark/auto theme system (Phase 1 - Foundation PR 1)#377
vitorvasc wants to merge 38 commits intoopen-telemetry:mainfrom
vitorvasc:feat/84-pr1-theme-system

Conversation

@vitorvasc
Copy link
Copy Markdown
Member

@vitorvasc vitorvasc commented May 6, 2026 •
edited
Loading

Part of the Phase 1 foundation work for the explorer redesign (#84).

Contributes to #370.

What's in this PR

  • Splits src/index.css into focused partials under src/styles/:
    • tokens.css — all CSS custom properties (dark + new light theme block)
    • base.css — global resets and body defaults
    • syntax.css — code block / syntax highlight rules
    • index.css — entry point that @imports the partials
  • The monolithic file is removed; src/main.tsx now imports from src/styles/index.css.
  • Replaces the old { themeId, setThemeId } API with a cleaner { mode, resolved, setMode } shape:
    • mode — the user's stored preference: "light" | "dark" | "auto"
    • resolved — the value actually applied to the document ("light" | "dark")
    • setMode — writes to localStorage["td-color-theme"] and updates [data-theme] on <html>
  • Adds an inline <script> in index.html that runs before first paint, reads localStorage["td-color-theme"], and sets [data-theme] on <html>. Prevents the white flash users see on dark-preferred systems before React hydrates.
  • Adds a [data-theme="light"] block in tokens.css alongside the existing dark defaults. Reconciles the five remaining dark surface tokens (card, muted, border, and variants) to the navy palette.
  • Authors the <ThemeToggle /> component (cycles light → dark → auto). Not yet rendered.
  • Migrates remaining literal-hue usages to brand primitives following the primary = blue / secondary = orange realignment already on main

⚠️ Production-visible changes (no V1_REDESIGNgate):

  • Five dark-mode surface tokens (card, card-secondary, muted, muted-foreground, border) shift to the navy palette on merge.

What's not in this PR

  • The toggle is not visible yet. It mounts in PR 2 alongside the new NavBar.
  • Light theme colors are plumbed but the page-level design (hero, cards, etc.) stays unchanged until later PRs activate V1_REDESIGN in App.tsx.
  • No changes to the feature-flag system or routing.

Verification

  • bun run test — 781 tests pass
  • tsc -b — clean
  • Theme init: open DevTools → Application → Local Storage. Toggle the control and confirm td-color-theme updates. Reload; confirm the selected theme is applied before React loads (no flash).
  • Auto mode: change OS color scheme with the toggle set to auto; confirm the page updates without a reload.

Co-authored with Claude Opus 4.7. Reviewed with Gemini 3 (see the comment).

vitorvasc added 30 commits April 30, 2026 13:14
@vitorvasc
temp: add new layout mockup (AI-generated)
bc95a05 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
feat(projects): add project index and frontmatter schema for initiati…
47645dd 
…ve documentation

Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
chore: move design-brief and mockups to projects folder
475e21c 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
docs: initialize project overview documentation
af5341e 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
docs: add detailed layout plan for the home page design
1cda2c4 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
docs: add initial draft for ecosystem landing pages plan
a8a929e 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
docs: add initial draft for list page plan
2985a87 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
docs: add initial draft for detail page plan
9c49030 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
docs: add initial draft for Explorer UI/UX design workspace
dc0d029 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
docs: add roadmap for explorer redesign and outline next steps
369898a 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
feat(flags): add V1 redesign feature flag for development
c6942f8 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
Merge branch 'main' into feat/84-layout-mockups
0064df0 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
fix file format
4c5b3f1 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
fix file format
c6091b2 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
chore: restore DESIGN.md
3d20537 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
fix(design): clarify mockup visibility and update DESIGN.md reference
56cf058 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
npm run format
aaa28f4 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
fix markdown linter issues
bad8f25 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
Merge branch 'main' into feat/84-layout-mockups
acce91c
@vitorvasc
docs: update roadmap; kick off PR 1
a832687 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
refactor(styles): split index.css into src/styles/ with modular partials
9625866 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
refactor(theme): replace inline-style injection with typed metadata i…
6b0fee1 
…n themes.ts

Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
refactor(theme): switch ThemeContext to mode/resolved API with persis…
ce275bb 
…tence and auto mode

Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
feat(theme): add no-flash data-theme init script to index.html
e85a36b 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
feat(ui): add ThemeToggle component
5d6e993 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
refactor(styles): update import path for index.css to use modular styles
3f0b741 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
docs: update NEXT-STEPS.md
fcd8ce0 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
fix(theme): fix stale resolved state in auto mode and move no-flash s…
9fd84cc 
…cript to head

Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
fix(docs): update last_updated dates to 2026-05-06 in multiple design…
0360c72 
… documents

Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
Merge branch 'feat/84-layout-mockups' into feat/84-pr1-theme-system
5bf0a65 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
vitorvasc added 3 commits May 6, 2026 17:42
@vitorvasc
fix(theme): migrate literal-hue usages to brand primitives after prim…
d4fc205 
…ary/secondary swap

Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
Merge remote-tracking branch 'origin/main' into feat/84-pr1-theme-system
2eb9971 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
fix(icons): revert OtelLogo colors
3d111c2 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc vitorvasc self-assigned this May 6, 2026
@netlify
Copy link
Copy Markdown

netlify Bot commented May 6, 2026 •
edited
Loading

✅ Deploy Preview for otel-ecosystem-explorer ready!

Name Link
🔨 Latest commit c8c785b
🔍 Latest deploy log https://app.netlify.com/projects/otel-ecosystem-explorer/deploys/69fe5ac3c6e7df0008e41eaa
😎 Deploy Preview https://deploy-preview-377--otel-ecosystem-explorer.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@vitorvasc
run prettier, fix formatting
58b6bbe 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@vitorvasc
Copy link
Copy Markdown
Member Author

vitorvasc commented May 6, 2026

Code review using Gemini 3:

Code review

I've completed a thorough review of the feat/84-pr1-theme-system branch.
Overall, the architecture is sound, and the two-tier brand system is a
significant improvement for maintainability. However, I've identified two
correctness bugs regarding the "auto" mode and theme persistence, as well as a
discrepancy in the feature-flagging strategy.

1. Correctness: Stale State in "Auto" Mode

I identified a bug in src/theme-context.tsx where the systemTheme state can
become stale.

Scenario:

  1. User starts in auto mode (e.g., system is dark). resolved is dark.
  2. User switches to light mode. The useEffect cleanup removes the
    matchMedia listener.
  3. User changes OS preference to light while the app is still in light mode.
    The systemTheme state is NOT updated because the listener is gone.
  4. User switches back to auto mode. The resolved value uses the stale
    systemTheme (still dark) instead of the current OS value.
  5. The useEffect for the listener runs, but it only reacts to future
    changes, it doesn't sync the state with the current value immediately.

Recommended Fix: Sync the state immediately when entering auto mode in the
effect:

useEffect(() => {
  if (mode !== 'auto') return;
  const mql = window.matchMedia('(prefers-color-scheme: dark)');

  // Sync immediately upon entering auto mode
  setSystemTheme(mql.matches ? 'dark' : 'light');

  const handler = (e: MediaQueryListEvent) => {
    setSystemTheme(e.matches ? 'dark' : 'light');
  };
  mql.addEventListener('change', handler);
  return () => mql.removeEventListener('change', handler);
}, [mode]);

2. Correctness: Persistence Alias for dark-blue

The author mentioned keeping dark-blue as a backwards-compatible alias, but
it's currently broken for persistence.

In src/theme-context.tsx:

function isValidMode(value: string | null): value is ThemeMode {
  return ['light', 'dark', 'auto'].includes(value as ThemeMode);
}

// ... in ThemeProvider initialization
const stored = localStorage.getItem(STORAGE_KEY);
return isValidMode(stored) ? stored : 'auto';

Because isValidMode excludes dark-blue, any user with that value in their
localStorage will be reset to auto instead of dark.

Recommended Fix: Explicitly handle the alias during initialization:

const stored = localStorage.getItem(STORAGE_KEY);
if (stored === 'dark-blue') return 'dark';
return isValidMode(stored) ? stored : 'auto';

Additionally, the no-flash script in index.html should also handle this
alias to prevent a flash of the system theme for dark-blue users:

var resolved = (stored === "light" || stored === "dark" || stored === "dark-blue")
  ? (stored === "dark-blue" ? "dark" : stored)
  : matchMedia(...)

3. Strategy: The "Global" Theme Disruption

The rationale states: "Phase 1 is gated behind a V1_REDESIGN feature flag so
every PR lands on main without disrupting production."

Observation: This is currently inaccurate. Because ThemeProvider is
globally applied in main.tsx and the new CSS tokens in tokens.css replace
the old index.css values immediately, the app's primary color and background
will change for all users (cyan → dark blue / black → dark navy) the moment this
PR merges, regardless of the V1_REDESIGN flag.

If the intention is strictly zero disruption for production, the ThemeProvider
or the HSL variable definitions would need to be conditional on the flag.
However, if a global color update is acceptable, the rationale should be updated
to reflect that "V1_REDESIGN" gates UI components, but not the base brand
colors.

4. Semantic Risk: GlowBadge Variant Naming

I agree with the author's assessment that variant="primary" now rendering
orange (secondary tokens) is misleading. While acceptable as a temporary state
for Phase 1, I recommend adding a TODO comment in glow-badge.tsx explicitly
pointing to the PR 4 cleanup to ensure it isn't forgotten if the roadmap shifts.

Summary of focused areas

  • No-flash script: Correct logic, but lacks the dark-blue alias.
  • Effect timing: The resolved update is safe, but the systemTheme sync
    issue noted above is a real bug.
  • CSS chaining: Lightning CSS and Tailwind v4 handle the variable
    indirection correctly at build/runtime. No concerns there.
  • Keyboard cycle: light → dark → auto is a logical progression. No
    concerns.

Verdict: Fix the auto mode sync bug and the dark-blue persistence alias,
and clarify the global nature of the color change before merging.

@vitorvasc
fix(theme): address pre-merge review findings on PR 1
a5526f3 
Signed-off-by: Vitor Vasconcellos <vvasconcellos1@gmail.com>
@jaydeluca jaydeluca closed this May 7, 2026
@jaydeluca jaydeluca reopened this May 7, 2026
@vitorvasc vitorvasc force-pushed the feat/84-pr1-theme-system branch from eca689d to 1bb6c52 Compare May 8, 2026 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@vitorvasc vitorvasc

Labels

add-screenshots UI/UX

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@vitorvasc @jaydeluca