feat: Implement Color Palette Extensibility & Dynamic Theme Registry (#3334) - #3510
Conversation
📝 WalkthroughWalkthroughThe PR adds a theme registry subsystem with palette and theme contracts. ChangesTheme registry
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related issues
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/core/src/theme/themeRegistry.ts`:
- Around line 22-34: Update the ThemeRegistry constructor to register the
required built-in Dracula, Nord, Monokai, and Cyberpunk presets alongside
default, using each preset’s defined name and palette. Add behavior tests
confirming each preset is available through listThemes() and can be selected
with setActiveTheme().
- Around line 36-59: The ThemeRegistry methods must validate runtime theme data
against the JSON theme schema before storage or mutation. Update registerTheme
to reject themes lacking required palette keys or containing non-string/invalid
color values, and update extendPalette to validate every customPalette entry
before merging so existing valid data is preserved on failure; return a
caller-handleable success/failure result from both operations and keep
this.themes.set and palette updates limited to valid input.
- Around line 1-13: Replace the undifferentiated string fields in ColorPalette
with a discriminated color value type carrying RGB, indexed, or ANSI metadata,
and add a capability-aware resolver in the theme registry. Resolve colors
appropriately for TrueColor, 256-color, and 16-color ANSI terminals, with
non-TrueColor fallbacks preserving usable output. Add tests covering RGB,
indexed, ANSI, and each fallback path.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ff54ddd8-374b-4f05-9cef-9d8b7ad1f19f
📒 Files selected for processing (2)
packages/core/src/theme/themeRegistry.tspackages/core/test/themeRegistry.test.ts
| export interface ColorPalette { | ||
| primary: string; | ||
| secondary: string; | ||
| background: string; | ||
| text: string; | ||
| border?: string; | ||
| accent?: string; | ||
| } | ||
|
|
||
| export interface ThemeDefinition { | ||
| name: string; | ||
| palette: ColorPalette; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Add color-mode metadata and fallback resolution.
ColorPalette stores every color as an undifferentiated string. The manager cannot resolve RGB values to 256-color or ANSI 16-color values when terminal support is limited.
Use a discriminated color value type and add a capability-aware resolver. Test RGB, indexed, and ANSI fallback behavior.
Based on PR objectives, the registry must support 16-color ANSI, 256-color, TrueColor, and non-TrueColor fallback behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/theme/themeRegistry.ts` around lines 1 - 13, Replace the
undifferentiated string fields in ColorPalette with a discriminated color value
type carrying RGB, indexed, or ANSI metadata, and add a capability-aware
resolver in the theme registry. Resolve colors appropriately for TrueColor,
256-color, and 16-color ANSI terminals, with non-TrueColor fallbacks preserving
usable output. Add tests covering RGB, indexed, ANSI, and each fallback path.
| constructor() { | ||
| this.registerTheme({ | ||
| name: 'default', | ||
| palette: { | ||
| primary: '#3b82f6', | ||
| secondary: '#64748b', | ||
| background: '#0f172a', | ||
| text: '#f8fafc', | ||
| border: '#334155', | ||
| accent: '#eab308', | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Register the required built-in presets.
The constructor registers only default. It does not register the required Dracula, Nord, Monokai, and Cyberpunk presets. listThemes() and setActiveTheme() cannot use those themes until a caller adds them manually.
Register the four presets during initialization and add behavior tests for each preset.
Based on PR objectives, these four presets are required defaults.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/theme/themeRegistry.ts` around lines 22 - 34, Update the
ThemeRegistry constructor to register the required built-in Dracula, Nord,
Monokai, and Cyberpunk presets alongside default, using each preset’s defined
name and palette. Add behavior tests confirming each preset is available through
listThemes() and can be selected with setActiveTheme().
| registerTheme(theme: ThemeDefinition): void { | ||
| if (!theme || !theme.name) return; | ||
| this.themes.set(theme.name, theme); | ||
| } | ||
|
|
||
| setActiveTheme(name: string): boolean { | ||
| if (this.themes.has(name)) { | ||
| this.activeThemeName = name; | ||
| return true; | ||
| } | ||
| return false; | ||
| } | ||
|
|
||
| getActiveTheme(): ThemeDefinition { | ||
| return this.themes.get(this.activeThemeName) || Array.from(this.themes.values())[0]; | ||
| } | ||
|
|
||
| extendPalette(themeName: string, customPalette: Partial<ColorPalette>): void { | ||
| const existing = this.themes.get(themeName); | ||
| if (existing) { | ||
| existing.palette = { | ||
| ...existing.palette, | ||
| ...customPalette, | ||
| }; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Validate runtime theme data before storage or merge.
registerTheme() checks only theme.name. A parsed external JSON theme can omit required palette fields or provide non-string values. extendPalette() can also overwrite a required color with an invalid value. The registry then returns data that violates ThemeDefinition.
Validate required palette keys and color value formats before this.themes.set() and before merging customPalette. Reject invalid input with a result that callers can handle.
Based on PR objectives, registerTheme() must support external JSON themes through a JSON theme schema.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/core/src/theme/themeRegistry.ts` around lines 36 - 59, The
ThemeRegistry methods must validate runtime theme data against the JSON theme
schema before storage or mutation. Update registerTheme to reject themes lacking
required palette keys or containing non-string/invalid color values, and update
extendPalette to validate every customPalette entry before merging so existing
valid data is preserved on failure; return a caller-handleable success/failure
result from both operations and keep this.themes.set and palette updates limited
to valid input.
|
Duplicates existing — ThemeRegistryManager parallels the tss theme system (BUILTIN_THEMES/ThemeTokens/engine/AutoThemeProvider) with no ThemeTokens compatibility and no wiring into ThemeSwitcher or the engine; unexported/unreferenced dead island. Build on the tss theme system. |
Description
Implements runtime theme registration and dynamic color palette extensibility (
ThemeRegistryManager) allowing developers to extend terminal UI color themes dynamically.Related Issue
Closes #3334
Which package(s)?
@termuijs/core
Type of Change
type:bug)type:feature)type:docs)type:testing)type:refactor)type:design)type:accessibility)type:performance)type:devops)type:security)Checklist
needs-starcheck blocks your merge otherwise.bun vitest runbun run buildbun run typecheckCONTRIBUTING.md.type: short description.markDirty()(if your change affects rendering).anytypes without an inline comment explaining why.GSSoC 2026 Participation
https://gssoc.girlscript.org/profile/knoxiboyScreenshots / Recordings (UI changes)
Supports dynamic theme swapping at runtime with zero restart overhead.
Notes for the Reviewer
Zero external dependencies; integrates with existing ANSI theme tokens.
Summary by CodeRabbit