Repository navigation
Conversation
- Store now syncs theme-id (not just theme-type) to localStorage - getDefaultTerminalTheme() looks up actual theme by ID from builtInThemes - Monokai/One Dark users get correct terminal colors on first paint 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Store now caches theme.terminal JSON in localStorage - getDefaultTerminalTheme() reads cached colors first - Future-proofs custom themes without needing builtInThemes lookup 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Tests cached terminal colors from localStorage - Tests fallback to theme-id lookup - Tests fallback to default dark theme - Tests graceful handling of invalid JSON - Mocks trpc-client to avoid electronTRPC dependency 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughAdds a stateful TerminalEscapeFilter and wires it per-terminal session (with flush-on-exit), expands terminal escape patterns, and implements a full theming system (types, built-in themes, store, utilities, boot script) plus Settings UI and terminal theme wiring. Changes
Sequence Diagram(s)sequenceDiagram
participant PTY as PTY Process
participant Session as TerminalSession
participant Filter as TerminalEscapeFilter
participant Scroll as Scrollback
participant XTerm as XTerm
PTY->>Session: data(chunk)
Session->>Filter: filter(chunk)
activate Filter
Filter->>Filter: buffer & detect complete sequences
alt complete sequences
Filter-->>Session: filteredOutput
else partial tail
Filter-->>Session: filteredOutput (without incomplete tail)
end
deactivate Filter
Session->>XTerm: emit original data (unfiltered)
Session->>Scroll: write filteredOutput
Note right of Session: on PTY exit
Session->>Filter: flush()
Filter-->>Session: remaining filtered data
Session->>Scroll: write flushed data
sequenceDiagram
participant App as Renderer
participant Store as Theme Store
participant Utils as Theme Utils
participant DOM
App->>Store: initialize (rehydrate)
activate Store
Store->>Store: read persisted activeThemeId & terminal cache
Store->>Utils: applyUIColors(theme.ui)
Utils->>DOM: set CSS variables
Store->>Utils: updateThemeClass(type)
Utils->>DOM: set root class (dark|light)
Store->>Store: toXtermTheme(theme.terminal)
deactivate Store
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas needing extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/desktop/src/renderer/screens/main/index.tsx (1)
19-59: Hotkey shortcuts must be centralized in a dedicated system per coding guidelinesThe code embeds hotkeys directly in components, but the coding guideline requires defining keyboard shortcuts using
apps/desktop/src/renderer/lib/shortcuts.tswith Arc Browser-inspired shortcut definitions. This centralized system doesn't currently exist. Createshortcuts.tsto define all keyboard shortcuts in one place, then import and register them in components.As a secondary improvement, once centralized, you can use the
enabledoption fromreact-hotkeys-hookv5.2.1 (which supports conditional registration viaenabled: isWorkspaceView) instead of manual guards inside callbacks for cleaner separation of concerns.apps/desktop/src/renderer/globals.css (1)
204-232: Scrollbar styling is hardcoded for dark mode.The scrollbar colors use fixed
rgb(63 63 70 / ...)values which are optimized for dark backgrounds. In light theme, these dark scrollbars may appear visually inconsistent. Consider using theme-aware variables or adding.lightoverrides.+/* Light mode scrollbar styling */ +:root.light * { + scrollbar-color: rgb(200 200 200 / 0.5) transparent; +} + +:root.light *::-webkit-scrollbar-thumb { + background-color: rgb(180 180 180 / 0.5); +} + +:root.light *::-webkit-scrollbar-thumb:hover { + background-color: rgb(160 160 160 / 0.7); +} + +:root.light *::-webkit-scrollbar-thumb:active { + background-color: rgb(140 140 140 / 0.8); +}
🧹 Nitpick comments (4)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.ts (1)
39-54: Consider splitting test with multiple assertions.This test has three assertions checking different properties. Per coding guidelines, tests should have one assert per test for clarity and easier debugging when failures occur. Based on learnings, tests should be readable and independent.
- it("should return cached terminal colors from localStorage", () => { + it("should return cached background from localStorage", () => { const cachedTerminal = { background: "#272822", foreground: "#f8f8f2", cursor: "#f8f8f0", red: "#f92672", green: "#a6e22e", }; localStorage.setItem("theme-terminal", JSON.stringify(cachedTerminal)); const theme = getDefaultTerminalTheme(); expect(theme.background).toBe("#272822"); - expect(theme.foreground).toBe("#f8f8f2"); - expect(theme.cursor).toBe("#f8f8f0"); }); + + it("should return cached foreground from localStorage", () => { + const cachedTerminal = { + background: "#272822", + foreground: "#f8f8f2", + cursor: "#f8f8f0", + }; + localStorage.setItem("theme-terminal", JSON.stringify(cachedTerminal)); + + const theme = getDefaultTerminalTheme(); + + expect(theme.foreground).toBe("#f8f8f2"); + }); + + it("should return cached cursor from localStorage", () => { + const cachedTerminal = { + background: "#272822", + foreground: "#f8f8f2", + cursor: "#f8f8f0", + }; + localStorage.setItem("theme-terminal", JSON.stringify(cachedTerminal)); + + const theme = getDefaultTerminalTheme(); + + expect(theme.cursor).toBe("#f8f8f0"); + });apps/desktop/src/renderer/globals.css (1)
57-93: Missing--radiusin light theme block.The
:rootblock defines--radius: 0.625rem(line 46), but:root.lightdoesn't include it. While this may be intentional (same radius for both themes), explicitly including it would maintain consistency and make the light theme self-contained.--sidebar-border: oklch(0.922 0 0); --sidebar-ring: oklch(0.708 0 0); + --radius: 0.625rem; }apps/desktop/src/shared/themes/built-in/index.ts (1)
10-27: LGTM! Well-organized theme aggregation.The aggregation pattern is clean and the
getBuiltInThemehelper provides convenient lookup. The array size (4 themes) makes thefind()approach perfectly acceptable.One optional improvement for type safety:
-export const DEFAULT_THEME_ID = "dark"; +export const DEFAULT_THEME_ID = "dark" as const; -export const builtInThemes: Theme[] = [ +export const builtInThemes: readonly Theme[] = [apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (1)
5-10: Theme bootstrap via localStorage looks solid; consider centralizing key constantsThe helpers correctly prefer cached terminal colors, fall back to a theme-id lookup, and finally to a hard-coded safe default, with try/catch to tolerate missing/localStorage issues. This should minimize terminal theme flash on startup.
To avoid drift between this file and
renderer/stores/theme/store.ts, consider exporting shared constants for"theme-id"/"theme-terminal"(and"theme-type"if relevant) from a small themes-storage util and reusing them here instead of literal strings.Also applies to: 14-48
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
apps/desktop/src/main/lib/terminal-escape-filter.test.ts(3 hunks)apps/desktop/src/main/lib/terminal-escape-filter.ts(2 hunks)apps/desktop/src/main/lib/terminal-history.ts(1 hunks)apps/desktop/src/main/lib/terminal-manager.ts(3 hunks)apps/desktop/src/renderer/globals.css(3 hunks)apps/desktop/src/renderer/index.html(1 hunks)apps/desktop/src/renderer/screens/main/components/SettingsView/AppearanceSettings.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/SettingsView/index.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/SettingsButton/SettingsButton.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/SettingsButton/index.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/TopBar/index.tsx(2 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SetupTerminal/SetupTerminal.tsx(4 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx(4 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.ts(1 hunks)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts(1 hunks)apps/desktop/src/renderer/screens/main/index.tsx(4 hunks)apps/desktop/src/renderer/stores/app-state.ts(1 hunks)apps/desktop/src/renderer/stores/index.ts(1 hunks)apps/desktop/src/renderer/stores/theme/index.ts(1 hunks)apps/desktop/src/renderer/stores/theme/store.ts(1 hunks)apps/desktop/src/renderer/stores/theme/utils/css-variables.ts(1 hunks)apps/desktop/src/renderer/stores/theme/utils/index.ts(1 hunks)apps/desktop/src/renderer/stores/theme/utils/terminal-theme.ts(1 hunks)apps/desktop/src/resources/public/theme-boot.js(1 hunks)apps/desktop/src/shared/themes/built-in/dark.ts(1 hunks)apps/desktop/src/shared/themes/built-in/index.ts(1 hunks)apps/desktop/src/shared/themes/built-in/light.ts(1 hunks)apps/desktop/src/shared/themes/built-in/monokai.ts(1 hunks)apps/desktop/src/shared/themes/built-in/one-dark.ts(1 hunks)apps/desktop/src/shared/themes/index.ts(1 hunks)apps/desktop/src/shared/themes/types.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Biome formatting, linting, and import organization at the root level for all code files
Files:
apps/desktop/src/renderer/screens/main/components/SettingsView/index.tsxapps/desktop/src/shared/themes/built-in/monokai.tsapps/desktop/src/shared/themes/types.tsapps/desktop/src/renderer/stores/theme/utils/terminal-theme.tsapps/desktop/src/shared/themes/built-in/light.tsapps/desktop/src/renderer/stores/theme/utils/index.tsapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/resources/public/theme-boot.jsapps/desktop/src/shared/themes/built-in/dark.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.tsxapps/desktop/src/shared/themes/built-in/one-dark.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar.tsxapps/desktop/src/renderer/stores/theme/utils/css-variables.tsapps/desktop/src/renderer/screens/main/components/TopBar/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/stores/theme/index.tsapps/desktop/src/main/lib/terminal-escape-filter.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/SettingsView/AppearanceSettings.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SettingsButton/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.tsapps/desktop/src/shared/themes/index.tsapps/desktop/src/renderer/stores/index.tsapps/desktop/src/main/lib/terminal-history.tsapps/desktop/src/main/lib/terminal-manager.tsapps/desktop/src/shared/themes/built-in/index.tsapps/desktop/src/renderer/stores/theme/store.tsapps/desktop/src/main/lib/terminal-escape-filter.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SetupTerminal/SetupTerminal.tsxapps/desktop/src/renderer/stores/app-state.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid usinganytype; use type-safe approaches instead, unless necessary
Ensure TypeScript type checking passes across all packages using bun run typecheck
Files:
apps/desktop/src/renderer/screens/main/components/SettingsView/index.tsxapps/desktop/src/shared/themes/built-in/monokai.tsapps/desktop/src/shared/themes/types.tsapps/desktop/src/renderer/stores/theme/utils/terminal-theme.tsapps/desktop/src/shared/themes/built-in/light.tsapps/desktop/src/renderer/stores/theme/utils/index.tsapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/shared/themes/built-in/dark.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.tsxapps/desktop/src/shared/themes/built-in/one-dark.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar.tsxapps/desktop/src/renderer/stores/theme/utils/css-variables.tsapps/desktop/src/renderer/screens/main/components/TopBar/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/stores/theme/index.tsapps/desktop/src/main/lib/terminal-escape-filter.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/SettingsView/AppearanceSettings.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SettingsButton/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.tsapps/desktop/src/shared/themes/index.tsapps/desktop/src/renderer/stores/index.tsapps/desktop/src/main/lib/terminal-history.tsapps/desktop/src/main/lib/terminal-manager.tsapps/desktop/src/shared/themes/built-in/index.tsapps/desktop/src/renderer/stores/theme/store.tsapps/desktop/src/main/lib/terminal-escape-filter.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SetupTerminal/SetupTerminal.tsxapps/desktop/src/renderer/stores/app-state.ts
apps/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
apps/**/*.{tsx,ts}: Structure React applications with one folder per component following the pattern: ComponentName/ComponentName.tsx with index.ts barrel export
Co-locate component dependencies (hooks, utils, constants, tests, stories) next to the file using them
Files:
apps/desktop/src/renderer/screens/main/components/SettingsView/index.tsxapps/desktop/src/shared/themes/built-in/monokai.tsapps/desktop/src/shared/themes/types.tsapps/desktop/src/renderer/stores/theme/utils/terminal-theme.tsapps/desktop/src/shared/themes/built-in/light.tsapps/desktop/src/renderer/stores/theme/utils/index.tsapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/shared/themes/built-in/dark.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.tsxapps/desktop/src/shared/themes/built-in/one-dark.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar.tsxapps/desktop/src/renderer/stores/theme/utils/css-variables.tsapps/desktop/src/renderer/screens/main/components/TopBar/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/stores/theme/index.tsapps/desktop/src/main/lib/terminal-escape-filter.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/SettingsView/AppearanceSettings.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SettingsButton/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.tsapps/desktop/src/shared/themes/index.tsapps/desktop/src/renderer/stores/index.tsapps/desktop/src/main/lib/terminal-history.tsapps/desktop/src/main/lib/terminal-manager.tsapps/desktop/src/shared/themes/built-in/index.tsapps/desktop/src/renderer/stores/theme/store.tsapps/desktop/src/main/lib/terminal-escape-filter.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SetupTerminal/SetupTerminal.tsxapps/desktop/src/renderer/stores/app-state.ts
apps/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use one component per file; do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/SettingsView/index.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/AppearanceSettings.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SetupTerminal/SetupTerminal.tsx
apps/desktop/src/renderer/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Define keyboard shortcuts using the centralized system in apps/desktop/src/renderer/lib/shortcuts.ts with Arc Browser-inspired shortcut definitions
Files:
apps/desktop/src/renderer/screens/main/components/SettingsView/index.tsxapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/screens/main/components/SettingsView/AppearanceSettings.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SetupTerminal/SetupTerminal.tsx
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/renderer/screens/main/components/SettingsView/index.tsxapps/desktop/src/shared/themes/built-in/monokai.tsapps/desktop/src/shared/themes/types.tsapps/desktop/src/renderer/stores/theme/utils/terminal-theme.tsapps/desktop/src/shared/themes/built-in/light.tsapps/desktop/src/renderer/stores/theme/utils/index.tsapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/resources/public/theme-boot.jsapps/desktop/src/shared/themes/built-in/dark.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.tsxapps/desktop/src/shared/themes/built-in/one-dark.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar.tsxapps/desktop/src/renderer/stores/theme/utils/css-variables.tsapps/desktop/src/renderer/screens/main/components/TopBar/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/stores/theme/index.tsapps/desktop/src/main/lib/terminal-escape-filter.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/SettingsView/AppearanceSettings.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SettingsButton/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.tsapps/desktop/src/shared/themes/index.tsapps/desktop/src/renderer/stores/index.tsapps/desktop/src/main/lib/terminal-history.tsapps/desktop/src/main/lib/terminal-manager.tsapps/desktop/src/shared/themes/built-in/index.tsapps/desktop/src/renderer/stores/theme/store.tsapps/desktop/src/main/lib/terminal-escape-filter.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SetupTerminal/SetupTerminal.tsxapps/desktop/src/renderer/stores/app-state.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/renderer/screens/main/components/SettingsView/index.tsxapps/desktop/src/shared/themes/built-in/monokai.tsapps/desktop/src/shared/themes/types.tsapps/desktop/src/renderer/stores/theme/utils/terminal-theme.tsapps/desktop/src/shared/themes/built-in/light.tsapps/desktop/src/renderer/stores/theme/utils/index.tsapps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/shared/themes/built-in/dark.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.tsxapps/desktop/src/shared/themes/built-in/one-dark.tsapps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar.tsxapps/desktop/src/renderer/stores/theme/utils/css-variables.tsapps/desktop/src/renderer/screens/main/components/TopBar/SettingsButton/SettingsButton.tsxapps/desktop/src/renderer/stores/theme/index.tsapps/desktop/src/main/lib/terminal-escape-filter.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/SettingsView/AppearanceSettings.tsxapps/desktop/src/renderer/screens/main/components/TopBar/SettingsButton/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.tsapps/desktop/src/shared/themes/index.tsapps/desktop/src/renderer/stores/index.tsapps/desktop/src/main/lib/terminal-history.tsapps/desktop/src/main/lib/terminal-manager.tsapps/desktop/src/shared/themes/built-in/index.tsapps/desktop/src/renderer/stores/theme/store.tsapps/desktop/src/main/lib/terminal-escape-filter.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SetupTerminal/SetupTerminal.tsxapps/desktop/src/renderer/stores/app-state.ts
apps/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate tests with their corresponding components (e.g., Component.test.tsx next to Component.tsx)
Files:
apps/desktop/src/main/lib/terminal-escape-filter.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.ts
apps/desktop/**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.test.{ts,tsx,js,jsx}: Tests should have one assert per test
Tests should be readable
Tests should be fast
Tests should be independent
Tests should be repeatable
Files:
apps/desktop/src/main/lib/terminal-escape-filter.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.ts
🧠 Learnings (16)
📚 Learning: 2025-11-24T21:32:46.559Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:32:46.559Z
Learning: Applies to apps/desktop/src/renderer/**/*.tsx : Define keyboard shortcuts using the centralized system in apps/desktop/src/renderer/lib/shortcuts.ts with Arc Browser-inspired shortcut definitions
Applied to files:
apps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/TopBar/index.tsxapps/desktop/src/renderer/stores/app-state.ts
📚 Learning: 2025-11-24T21:32:46.559Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:32:46.559Z
Learning: Applies to apps/desktop/src/main/lib/*-ipcs.ts : Implement Electron IPC handlers in dedicated files under apps/desktop/src/main/lib/ (e.g., workspace-ipcs.ts, terminal-ipcs.ts)
Applied to files:
apps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.tsapps/desktop/src/renderer/stores/index.tsapps/desktop/src/main/lib/terminal-history.tsapps/desktop/src/main/lib/terminal-manager.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SetupTerminal/SetupTerminal.tsxapps/desktop/src/renderer/stores/app-state.ts
📚 Learning: 2025-11-24T21:33:13.244Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.244Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Please use alias as defined in `tsconfig.json` when possible
Applied to files:
apps/desktop/src/renderer/screens/main/index.tsxapps/desktop/src/renderer/stores/theme/index.tsapps/desktop/src/shared/themes/index.tsapps/desktop/src/renderer/stores/index.tsapps/desktop/src/renderer/stores/app-state.ts
📚 Learning: 2025-11-24T21:32:46.559Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:32:46.559Z
Learning: Applies to packages/ui/**/*.{tsx,ts} : Use TailwindCSS v4 and shadcn/ui for UI component styling and design
Applied to files:
apps/desktop/src/renderer/stores/theme/utils/css-variables.tsapps/desktop/src/renderer/screens/main/components/SettingsView/AppearanceSettings.tsx
📚 Learning: 2025-11-24T21:32:46.559Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:32:46.559Z
Learning: Applies to apps/**/*.{tsx,ts} : Co-locate component dependencies (hooks, utils, constants, tests, stories) next to the file using them
Applied to files:
apps/desktop/src/renderer/stores/theme/index.tsapps/desktop/src/renderer/stores/index.ts
📚 Learning: 2025-11-24T21:33:13.244Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.244Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be readable
Applied to files:
apps/desktop/src/main/lib/terminal-escape-filter.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.ts
📚 Learning: 2025-11-24T21:33:13.244Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.244Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be independent
Applied to files:
apps/desktop/src/main/lib/terminal-escape-filter.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.ts
📚 Learning: 2025-11-24T21:33:13.244Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.244Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be repeatable
Applied to files:
apps/desktop/src/main/lib/terminal-escape-filter.test.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.ts
📚 Learning: 2025-11-24T21:32:46.559Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:32:46.559Z
Learning: Applies to apps/**/*.test.{ts,tsx} : Co-locate tests with their corresponding components (e.g., Component.test.tsx next to Component.tsx)
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.ts
📚 Learning: 2025-11-24T21:33:13.244Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.244Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be fast
Applied to files:
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.ts
📚 Learning: 2025-11-24T21:32:46.559Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:32:46.559Z
Learning: Applies to @(apps/desktop/src/renderer|apps/desktop/src/shared)/**/*.{ts,tsx} : Never import Node.js modules in Electron renderer process or shared code; use IPC for communication between main and renderer processes
Applied to files:
apps/desktop/src/renderer/stores/index.ts
📚 Learning: 2025-11-24T21:32:46.559Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:32:46.559Z
Learning: Applies to apps/desktop/src/lib/electron-router-dom.ts : Do not import Node.js modules in apps/desktop/src/lib/electron-router-dom.ts as it is shared between main and renderer processes
Applied to files:
apps/desktop/src/renderer/stores/index.ts
📚 Learning: 2025-11-24T21:32:46.559Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:32:46.559Z
Learning: Applies to apps/**/*.{tsx,ts} : Structure React applications with one folder per component following the pattern: ComponentName/ComponentName.tsx with index.ts barrel export
Applied to files:
apps/desktop/src/renderer/stores/index.ts
📚 Learning: 2025-11-24T21:32:46.559Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:32:46.559Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables from monorepo root .env file in apps/desktop/src/main/index.ts before any imports with override: true
Applied to files:
apps/desktop/src/renderer/stores/index.ts
📚 Learning: 2025-11-24T21:33:13.244Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.244Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Applied to files:
apps/desktop/src/renderer/stores/theme/store.tsapps/desktop/src/renderer/stores/app-state.ts
📚 Learning: 2025-11-24T21:33:13.244Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.244Z
Learning: Applies to apps/desktop/**/*.{ts,tsx,js,jsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`
Applied to files:
apps/desktop/src/renderer/screens/main/components/TopBar/index.tsx
🧬 Code graph analysis (16)
apps/desktop/src/renderer/screens/main/components/SettingsView/index.tsx (2)
apps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar.tsx (1)
SettingsSidebar(23-65)apps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.tsx (1)
SettingsContent(8-14)
apps/desktop/src/shared/themes/built-in/monokai.ts (3)
apps/desktop/src/shared/themes/built-in/index.ts (1)
monokaiTheme(30-30)apps/desktop/src/shared/themes/index.ts (2)
monokaiTheme(10-10)Theme(13-13)apps/desktop/src/shared/themes/types.ts (1)
Theme(108-131)
apps/desktop/src/shared/themes/types.ts (1)
apps/desktop/src/shared/themes/index.ts (4)
UIColors(13-13)TerminalColors(13-13)Theme(13-13)ThemeMetadata(13-13)
apps/desktop/src/renderer/screens/main/index.tsx (3)
apps/desktop/src/renderer/stores/app-state.ts (1)
useCurrentView(35-35)apps/desktop/src/renderer/screens/main/components/SettingsView/index.tsx (1)
SettingsView(7-22)apps/desktop/src/renderer/screens/main/components/WorkspaceView/index.tsx (1)
WorkspaceView(14-88)
apps/desktop/src/shared/themes/built-in/dark.ts (3)
apps/desktop/src/shared/themes/built-in/index.ts (1)
darkTheme(30-30)apps/desktop/src/shared/themes/index.ts (2)
darkTheme(7-7)Theme(13-13)apps/desktop/src/shared/themes/types.ts (1)
Theme(108-131)
apps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.tsx (2)
apps/desktop/src/renderer/screens/main/components/SettingsView/index.tsx (1)
SettingsSection(5-5)apps/desktop/src/renderer/screens/main/components/SettingsView/AppearanceSettings.tsx (1)
AppearanceSettings(6-49)
apps/desktop/src/shared/themes/built-in/one-dark.ts (3)
apps/desktop/src/shared/themes/built-in/index.ts (1)
oneDarkTheme(30-30)apps/desktop/src/shared/themes/index.ts (2)
oneDarkTheme(11-11)Theme(13-13)apps/desktop/src/shared/themes/types.ts (1)
Theme(108-131)
apps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar.tsx (3)
apps/desktop/src/renderer/screens/main/components/SettingsView/index.tsx (1)
SettingsSection(5-5)apps/desktop/src/renderer/stores/app-state.ts (1)
useCloseSettings(37-38)packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop/src/renderer/screens/main/components/TopBar/SettingsButton/SettingsButton.tsx (2)
apps/desktop/src/renderer/screens/main/components/TopBar/SettingsButton/index.ts (1)
SettingsButton(1-1)apps/desktop/src/renderer/stores/app-state.ts (1)
useOpenSettings(36-36)
apps/desktop/src/main/lib/terminal-escape-filter.test.ts (1)
apps/desktop/src/main/lib/terminal-escape-filter.ts (3)
filterTerminalQueryResponses(246-248)filter(115-138)TerminalEscapeFilter(108-236)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.ts (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (2)
getDefaultTerminalTheme(19-40)getDefaultTerminalBg(46-48)
apps/desktop/src/main/lib/terminal-manager.ts (1)
apps/desktop/src/main/lib/terminal-escape-filter.ts (1)
TerminalEscapeFilter(108-236)
apps/desktop/src/shared/themes/built-in/index.ts (4)
apps/desktop/src/shared/themes/types.ts (1)
Theme(108-131)apps/desktop/src/shared/themes/built-in/dark.ts (1)
darkTheme(6-77)apps/desktop/src/shared/themes/built-in/light.ts (1)
lightTheme(6-77)apps/desktop/src/shared/themes/built-in/monokai.ts (1)
monokaiTheme(6-78)
apps/desktop/src/renderer/stores/theme/store.ts (7)
apps/desktop/src/shared/themes/index.ts (4)
Theme(13-13)ThemeMetadata(13-13)builtInThemes(5-5)DEFAULT_THEME_ID(6-6)apps/desktop/src/shared/themes/types.ts (2)
Theme(108-131)ThemeMetadata(136-143)apps/desktop/src/shared/themes/built-in/index.ts (2)
builtInThemes(10-15)DEFAULT_THEME_ID(20-20)apps/desktop/src/renderer/stores/theme/utils/css-variables.ts (2)
applyUIColors(46-55)updateThemeClass(60-69)apps/desktop/src/renderer/stores/theme/utils/index.ts (3)
applyUIColors(2-2)updateThemeClass(4-4)toXtermTheme(6-6)apps/desktop/src/renderer/stores/theme/utils/terminal-theme.ts (1)
toXtermTheme(7-36)apps/desktop/src/renderer/lib/electron-storage.ts (1)
electronStorage(20-20)
apps/desktop/src/renderer/screens/main/components/TopBar/index.tsx (1)
apps/desktop/src/renderer/screens/main/components/TopBar/SettingsButton/SettingsButton.tsx (1)
SettingsButton(4-17)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SetupTerminal/SetupTerminal.tsx (2)
apps/desktop/src/renderer/stores/theme/store.ts (1)
useTerminalTheme(184-185)apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (2)
createTerminalInstance(50-113)getDefaultTerminalBg(46-48)
🔇 Additional comments (48)
apps/desktop/src/main/lib/terminal-history.ts (1)
1-1: LGTM - Clean import consolidation.The consolidated import statement improves readability without any functional changes.
apps/desktop/src/renderer/screens/main/components/TopBar/SettingsButton/index.ts (1)
1-1: LGTM - Correct barrel export pattern.Follows the component structure guidelines properly.
apps/desktop/src/resources/public/theme-boot.js (1)
1-12: LGTM - Effective FOUC prevention.The implementation correctly applies the saved theme before React hydration with proper error handling and a sensible fallback to dark mode.
apps/desktop/src/renderer/stores/index.ts (1)
7-10: LGTM - Clean store exports.The new exports properly surface the app-state and theme store APIs through the centralized barrel.
apps/desktop/src/renderer/screens/main/components/TopBar/SettingsButton/SettingsButton.tsx (1)
1-17: LGTM - Well-structured component.The component follows all guidelines correctly:
- Proper file structure (SettingsButton/SettingsButton.tsx with barrel export)
- Good accessibility with aria-label
- Appropriate use of zustand store hook
- Explicit button type
apps/desktop/src/renderer/stores/theme/utils/index.ts (1)
1-6: LGTM - Clean utility exports.The barrel export properly centralizes theme utility access.
apps/desktop/src/renderer/screens/main/components/TopBar/index.tsx (2)
2-2: LGTM - Clean component integration.The SettingsButton is properly imported and integrated into the top bar.
24-24: LGTM - Logical placement.The settings button is appropriately positioned with other top bar controls.
apps/desktop/src/renderer/screens/main/index.tsx (1)
61-69: View-based switching betweenSettingsViewandWorkspaceViewis clean and centralizedUsing
currentViewfrom the app-state store to toggle between<SettingsView />and<WorkspaceView />keeps main-screen navigation centralized and predictable. This fits well with the new settings screen without adding extra local state.apps/desktop/src/shared/themes/built-in/monokai.ts (1)
6-77: Monokai theme definition is consistent and completeThe
monokaiThemeobject cleanly matches theTheme/TerminalColorsshape and covers all the standard + bright ANSI slots, so it should plug into the theme store and xterm theming without surprises.apps/desktop/src/renderer/stores/theme/utils/terminal-theme.ts (1)
7-35:toXtermThemeis a straightforward, correct adapterThe function cleanly forwards every
TerminalColorsfield to the correspondingIThemekey, so xterm will see exactly the configured palette with no transformations or omissions. This should be easy to keep in sync if either type evolves.apps/desktop/src/shared/themes/built-in/dark.ts (1)
6-76: Dark theme aligns with the sharedThememodel and looks ready as a defaultThe
darkThemeobject follows the sharedThemeshape, uses OKLCH for UI colors, and provides a complete terminal ANSI palette, making it a solid choice for the default/built-in dark theme.apps/desktop/src/shared/themes/built-in/one-dark.ts (1)
6-77: One Dark theme is well-structured and consistent with other built-insThe
oneDarkThemedefinition mirrors the other built-in themes (metadata, UI tokens, full ANSI + bright terminal palette), so it should integrate smoothly with the theme store and selector UI.apps/desktop/src/renderer/index.html (1)
5-19: Theme boot + stricter CSP look good; please verify dev/HMR and preload behaviorLoading
theme-boot.jsbefore the body plus tightening CSP toscript-src 'self'and restrictedconnect-srcgreatly improves security and should reduce theme flash, but it can also break dev tooling or any non-local resources if they’re still in use (e.g., HMR over HTTP, external fonts/scripts). Please run through your dev and packaged builds to confirm everything still works as expected with this CSP and withtheme-boot.jsbeing a non-module script.apps/desktop/src/renderer/stores/theme/index.ts (1)
1-7: Barrel exporting theme hooks keeps the public store API tidyRe-exporting the theme hooks from
./storegives consumers a clean, stable import path and matches the pattern used by other stores.apps/desktop/src/renderer/screens/main/components/SettingsView/AppearanceSettings.tsx (1)
6-48: Theme selection flow is straightforward and integrates cleanly with the theme storeComposing
allThemesfrombuiltInThemesandcustomThemes, then wiringThemeCard’sonSelecttosetTheme(theme.id)gives you a simple, predictable theme picker that’s fully driven by the centralized theme store. The copy and layout are clear, and using therenderer/storesandshared/themesaliases aligns with the path-alias guidelines.As per coding guidelines, this alias usage is spot on.
apps/desktop/src/shared/themes/built-in/light.ts (1)
1-77: LGTM! Well-structured light theme definition.The theme follows the established
Themeinterface pattern, with consistent use of oklch for UI colors and hex for terminal colors. The metadata and color tokens align with the CSS fallback values inglobals.css(lines 58-93).apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.test.ts (1)
56-103: Good test coverage for fallback scenarios.The remaining tests properly validate the fallback chain: theme-id lookup → default dark theme → graceful JSON error handling. Each test has a single assertion and is independent. The
getDefaultTerminalBgtests appropriately verify the wrapper function behavior.apps/desktop/src/shared/themes/index.ts (1)
1-13: LGTM! Clean barrel export structure.The centralized re-exports allow consumers to import all theme-related entities from a single location. Minor nit: the comment on line 1 says "Theme types" but the file exports both theme objects and types - consider updating to "Theme exports" or removing the comments since the exports are self-documenting.
apps/desktop/src/renderer/screens/main/components/SettingsView/SettingsSidebar.tsx (1)
1-65: LGTM! Clean settings sidebar implementation.The component follows good patterns: single responsibility, proper button types for accessibility, extensible
SECTIONSarray for future additions, and appropriate use of thecnutility for conditional styling. The back button correctly integrates with the app store viauseCloseSettings.apps/desktop/src/renderer/screens/main/components/SettingsView/SettingsContent.tsx (1)
8-14: LGTM! Appropriate content routing pattern.The conditional rendering is suitable for the current single-section setup. As more sections are added, consider refactoring to a switch statement or object lookup pattern for better maintainability.
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx (2)
78-198:terminalThemeused but not in dependency array.
terminalThemeis passed tocreateTerminalInstance(line 85) but isn't included in the effect's dependency array (line 198). IfterminalThemechanges before the initial mount effect runs, the terminal could be created with a stale theme. While the second effect (lines 201-208) handles runtime theme updates, the initial creation might use incorrect colors.This may be intentional to prevent terminal recreation on theme change. Please verify if this is the desired behavior, or if
terminalThemeshould be added to the deps with appropriate handling:- }, [tabId, workspaceId, setActiveTab, workspaceCwd, tabTitle]); + }, [tabId, workspaceId, setActiveTab, workspaceCwd, tabTitle, terminalTheme]);Note: Adding it would cause terminal recreation on theme change, which the second effect already handles more gracefully. The current pattern may be correct.
200-220: Good theme update pattern.The separate effect for theme updates is a clean approach that avoids recreating the terminal instance. The null guards and comment about xterm v5 behavior are helpful. The background fallback via
getDefaultTerminalBg()ensures consistent styling before hydration.apps/desktop/src/main/lib/terminal-escape-filter.test.ts (6)
1-5: LGTM!Import updated to include
TerminalEscapeFilteralongside the existingfilterTerminalQueryResponses. This aligns with the new stateful filter class being tested.
217-232: LGTM!Good addition of edge case tests for short-form hex color responses. These tests cover 2-digit (
f5/f5/f5), 3-digit (fff/fff/fff), and both BEL and ST terminators, ensuring the filter handles various real-world OSC color response formats.
234-244: LGTM!Standard mode report tests provide good coverage for the
ESC[n;m$yformat with different parameter combinations.
366-459: LGTM!Excellent test coverage for the stateful
TerminalEscapeFilter. The chunked data tests verify critical behaviors:
- Split sequence reassembly (DA1, CPR, OSC, standard mode reports)
- Conservative buffering decisions (ESC alone and ESC[ alone pass through)
- Color codes correctly pass through after completion despite initial buffering
The tests are readable, independent, and repeatable. Based on learnings, this aligns with the test guidelines.
461-486: LGTM!Flush and reset tests verify proper session cleanup semantics. The flush behavior test at line 469 correctly expects the incomplete sequence to pass through (since incomplete sequences can't be definitively identified as query responses).
488-504: LGTM!These tests ensure the stateful filter doesn't inadvertently delay or corrupt normal terminal output. The chunk boundary test at lines 495-502 verifies ANSI colors remain intact even when split across data events.
apps/desktop/src/shared/themes/types.ts (3)
1-69: LGTM!Well-structured
UIColorsinterface with comprehensive coverage of UI chrome colors. The JSDoc comments clearly document the purpose of each color category (core backgrounds, cards, popovers, actions, etc.).
71-103: LGTM!
TerminalColorsinterface correctly models the ANSI 16-color palette with appropriate optional fields for cursor accent and selection colors. This aligns with xterm.js theming requirements.
105-143: LGTM!The
ThemeandThemeMetadatainterfaces are well-designed. The distinction between optionalisBuiltIn/isCustomonThemeand required onThemeMetadatamakes sense—ThemeMetadatais specifically for list displays where these flags are always needed, whileThemeobjects may be constructed without these flags initially.apps/desktop/src/renderer/stores/theme/utils/css-variables.ts (3)
6-41: LGTM!The
UI_COLOR_TO_CSS_VARmapping is complete and correctly maps allUIColorsproperties to their corresponding CSS custom property names. The naming convention follows kebab-case for CSS variables.
46-55: LGTM!The
applyUIColorsfunction correctly applies theme colors to CSS variables. The truthy check on line 51 appropriately skips undefined or empty values, preventing invalid CSS variable assignments.
60-79: LGTM!
updateThemeClasscorrectly manages the dark/light class toggle, ensuring only one is active at a time.clearThemeVariablesprovides a clean reset mechanism for reverting to stylesheet defaults.apps/desktop/src/main/lib/terminal-manager.ts (4)
6-6: LGTM!Import and type definition correctly updated to use the new stateful
TerminalEscapeFilterclass. AddingescapeFiltertoTerminalSessioninterface ensures each session maintains its own buffer state for chunked data handling.Also applies to: 22-22
139-139: LGTM!Per-session
TerminalEscapeFilterinstance ensures buffer state isolation between terminal sessions.
142-150: LGTM!The data handling design is correct:
- Filtered data goes to scrollback/history storage (removes terminal query responses)
- Raw data goes to xterm (preserves DA/DSR/OSC responses needed for terminal behavior)
The comments clearly document this intentional divergence in data paths.
152-161: LGTM!Flush on exit ensures any buffered partial sequences are processed and written to storage before the session closes. The conditional check
if (remaining)correctly skips empty flush results.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/SetupTerminal/SetupTerminal.tsx (6)
6-11: LGTM!Imports correctly added for
useTerminalThemehook andgetDefaultTerminalBghelper. This enables theme-aware terminal initialization and background styling.
52-52: LGTM!Using
useTerminalTheme()from the theme store aligns with the coding guidelines preference for zustand state management.
107-111: LGTM!
createTerminalInstancecall correctly passesterminalThemefor theme-aware terminal initialization, preventing a flash of default colors on mount.
206-213: Intentional omission ofterminalThemefrom dependency array.The exclusion of
terminalThemefrom this effect's dependencies is correct. Theme changes are handled by the dedicated effect at lines 216-223, which updates the existing terminal instance without recreation. Including it here would cause unnecessary terminal recreation on theme changes.
215-223: LGTM!The dedicated effect for theme updates is the correct pattern. Using
xterm.options.themeproperty setter (rather than recreating the terminal) is efficient and leverages xterm.js v5's built-in repaint mechanism.
225-234: LGTM!Background color styling with inline style and
getDefaultTerminalBg()fallback prevents flash of unstyled content during initial render and theme loading. The wrapper structure correctly separates the background container from the terminal container.apps/desktop/src/renderer/stores/app-state.ts (1)
1-38: Simple view store is clear and type-safeThe AppView union plus small Zustand store cleanly model workspace vs settings navigation, and the convenience hooks keep consumers terse. No issues from a correctness or style perspective.
apps/desktop/src/renderer/stores/theme/store.ts (1)
1-187: I'll verify the review comment by checking the path aliases configuration, locating the terminal helpers file, and confirming the zustand API compatibility.
<function_calls>
{
"command": "cat apps/desktop/tsconfig.json | grep -A 20 'paths'"
}
</function_calls>Now let me find the terminal helpers file to check for key duplication:
<function_calls>
{
"command": "fd -e ts -e tsx 'terminal' apps/desktop/src/renderer --type f | head -20"
}
</function_calls>Let me also search for the specific localStorage keys mentioned in the review:
<function_calls>
{
"command": "rg -n 'theme-id|theme-type|theme-terminal' apps/desktop/src/renderer --type ts --type tsx -C 2"
}
</function_calls>Let me check the file structure to understand where electronStorage is located:
<function_calls>
{
"command": "find apps/desktop/src/renderer -name 'electron-storage*' -type f"
}
</function_calls>Now I'll search the web for zustand persist middleware API documentation to verify the onRehydrateStorage callback pattern:
<function_calls>
{
"query": "zustand persist middleware onRehydrateStorage callback API documentation 2024 2025"
}
</function_calls>apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts (1)
50-60: All call sites properly updated; theming is safe and consistent across the codebase.The review comment's concern has been verified as addressed:
- SetupTerminal.tsx (line 107): Passes
terminalThemefromuseTerminalTheme()hook- Terminal.tsx (line 82): Passes
terminalThemefromuseTerminalTheme()hookBoth callers pass the same hook value, which has type
ITheme | nulland can benullinitially. The function correctly handles this with the fallback logicinitialTheme ?? getDefaultTerminalTheme(), ensuring no un-themed terminals are created. The type system and runtime behavior are aligned—all terminals will either use the provided theme or fall back to the default, preventing any flash or inconsistency.
| import { useState } from "react"; | ||
| import { SettingsContent } from "./SettingsContent"; | ||
| import { SettingsSidebar } from "./SettingsSidebar"; | ||
|
|
||
| export type SettingsSection = "appearance"; | ||
|
|
||
| export function SettingsView() { | ||
| const [activeSection, setActiveSection] = | ||
| useState<SettingsSection>("appearance"); | ||
|
|
||
| return ( | ||
| <div className="flex flex-1 bg-tertiary"> | ||
| <SettingsSidebar | ||
| activeSection={activeSection} | ||
| onSectionChange={setActiveSection} | ||
| /> | ||
| <div className="flex-1 m-3 bg-background rounded overflow-hidden"> | ||
| <SettingsContent activeSection={activeSection} /> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Refactor to follow component structure guidelines.
The component implementation should be in SettingsView.tsx with index.ts serving as a barrel export.
As per coding guidelines: "Structure React applications with one folder per component following the pattern: ComponentName/ComponentName.tsx with index.ts barrel export"
Suggested structure:
Create SettingsView/SettingsView.tsx:
import { useState } from "react";
import { SettingsContent } from "./SettingsContent";
import { SettingsSidebar } from "./SettingsSidebar";
export type SettingsSection = "appearance";
export function SettingsView() {
const [activeSection, setActiveSection] =
useState<SettingsSection>("appearance");
return (
<div className="flex flex-1 bg-tertiary">
<SettingsSidebar
activeSection={activeSection}
onSectionChange={setActiveSection}
/>
<div className="flex-1 m-3 bg-background rounded overflow-hidden">
<SettingsContent activeSection={activeSection} />
</div>
</div>
);
}Update SettingsView/index.ts:
export { SettingsView } from "./SettingsView";
export type { SettingsSection } from "./SettingsView";🤖 Prompt for AI Agents
In apps/desktop/src/renderer/screens/main/components/SettingsView/index.tsx
(lines 1-22) the component is implemented directly in the index file instead of
following the ComponentName/ComponentName.tsx + index.ts barrel pattern; move
the component implementation into a new file
apps/desktop/src/renderer/screens/main/components/SettingsView/SettingsView.tsx
containing the current component code, create
apps/desktop/src/renderer/screens/main/components/SettingsView/index.ts that
re-exports the component and its type (export { SettingsView } from
"./SettingsView"; export type { SettingsSection } from "./SettingsView";), and
update any imports elsewhere to import from the folder path (the barrel) if
needed.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/desktop/src/main/lib/terminal-escape-filter.ts (1)
199-206: CSI completeness check still examines tail's last character instead of first final byte.This bug was flagged in a previous review. The current logic checks
csiBody[csiBody.length - 1], which means a CSI sequence like\x1b[31mHello\nis incorrectly marked incomplete because the last character is\n. The fix should search for the first CSI final byte ([a-zA-Z~]) in the body:if (secondChar === "[") { const csiBody = str.slice(2); if (csiBody.length === 0) return true; - // CSI ends with a letter - const lastChar = csiBody[csiBody.length - 1]; - return !/[a-zA-Z~]/.test(lastChar); + // CSI is complete once we see the first final byte (letter or ~) + const finalIndex = csiBody.search(/[a-zA-Z~]/); + return finalIndex === -1; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/main/lib/terminal-escape-filter.ts(2 hunks)apps/desktop/src/renderer/screens/main/components/SettingsView/AppearanceSettings.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/desktop/src/renderer/screens/main/components/SettingsView/AppearanceSettings.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Biome formatting, linting, and import organization at the root level for all code files
Files:
apps/desktop/src/main/lib/terminal-escape-filter.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid usinganytype; use type-safe approaches instead, unless necessary
Ensure TypeScript type checking passes across all packages using bun run typecheck
Files:
apps/desktop/src/main/lib/terminal-escape-filter.ts
apps/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
apps/**/*.{tsx,ts}: Structure React applications with one folder per component following the pattern: ComponentName/ComponentName.tsx with index.ts barrel export
Co-locate component dependencies (hooks, utils, constants, tests, stories) next to the file using them
Files:
apps/desktop/src/main/lib/terminal-escape-filter.ts
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/main/lib/terminal-escape-filter.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/main/lib/terminal-escape-filter.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (5)
apps/desktop/src/main/lib/terminal-escape-filter.ts (5)
61-66: LGTM!The
standardModeReportpattern correctly matches the non-private mode report variant (ESC [ Ps ; Pm $ y), complementing the existingdecPrivateModeReportfor private modes.
68-80: LGTM!Good enhancement to handle varying color precision levels (8-bit, 12-bit, 16-bit) that different terminals may report.
87-91: LGTM!Pragmatic addition to handle real-world terminal quirks. The pattern is specific enough to avoid false positives.
223-238: LGTM on flush/reset API.The
flush()andreset()methods provide proper lifecycle management for the filter state.flush()correctly applies the filter to any remaining buffered data before returning.
241-251: LGTM!Clear deprecation notice with guidance to use the new
TerminalEscapeFilterclass. Maintaining the stateless function provides backward compatibility.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/desktop/src/main/lib/terminal-escape-filter.test.ts (1)
408-419: Multiple assertions in single test.This test has two separate
expectstatements checking different behaviors (buffering vs. completion). Per coding guidelines, tests should have one assert per test.Consider splitting into two tests:
- it("should buffer digit CSI but pass through color codes when complete", () => { + it("should buffer incomplete digit CSI at chunk boundary", () => { const filter = new TerminalEscapeFilter(); - // Color sequence is buffered initially (could be CPR/mode report) const chunk1 = `text${ESC}[32`; - const chunk2 = `mgreen`; const result1 = filter.filter(chunk1); - const result2 = filter.filter(chunk2); - // First chunk buffered, second chunk completes sequence - // Color code doesn't match filter patterns, so passes through expect(result1).toBe("text"); + }); + + it("should pass through color codes when completed in next chunk", () => { + const filter = new TerminalEscapeFilter(); + filter.filter(`text${ESC}[32`); // Buffer it + const chunk2 = `mgreen`; + const result2 = filter.filter(chunk2); expect(result2).toBe(`${ESC}[32mgreen`); });Based on learnings, tests should be readable.
apps/desktop/src/main/lib/terminal-escape-filter.ts (1)
88-92: Document the source of this pattern.The
unknownCSI_Opattern filtersESC[Osequences, but without additional context, it's unclear when this appears and whether it might inadvertently filter legitimate terminal output. Consider adding a comment explaining which terminal(s) emit this sequence and under what circumstances.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/main/lib/terminal-escape-filter.test.ts(3 hunks)apps/desktop/src/main/lib/terminal-escape-filter.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Biome formatting, linting, and import organization at the root level for all code files
Files:
apps/desktop/src/main/lib/terminal-escape-filter.test.tsapps/desktop/src/main/lib/terminal-escape-filter.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid usinganytype; use type-safe approaches instead, unless necessary
Ensure TypeScript type checking passes across all packages using bun run typecheck
Files:
apps/desktop/src/main/lib/terminal-escape-filter.test.tsapps/desktop/src/main/lib/terminal-escape-filter.ts
apps/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
apps/**/*.{tsx,ts}: Structure React applications with one folder per component following the pattern: ComponentName/ComponentName.tsx with index.ts barrel export
Co-locate component dependencies (hooks, utils, constants, tests, stories) next to the file using them
Files:
apps/desktop/src/main/lib/terminal-escape-filter.test.tsapps/desktop/src/main/lib/terminal-escape-filter.ts
apps/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Co-locate tests with their corresponding components (e.g., Component.test.tsx next to Component.tsx)
Files:
apps/desktop/src/main/lib/terminal-escape-filter.test.ts
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/main/lib/terminal-escape-filter.test.tsapps/desktop/src/main/lib/terminal-escape-filter.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/main/lib/terminal-escape-filter.test.tsapps/desktop/src/main/lib/terminal-escape-filter.ts
apps/desktop/**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.test.{ts,tsx,js,jsx}: Tests should have one assert per test
Tests should be readable
Tests should be fast
Tests should be independent
Tests should be repeatable
Files:
apps/desktop/src/main/lib/terminal-escape-filter.test.ts
🧠 Learnings (2)
📚 Learning: 2025-11-24T21:33:13.244Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2025-11-24T21:33:13.244Z
Learning: Applies to apps/desktop/**/*.test.{ts,tsx,js,jsx} : Tests should be readable
Applied to files:
apps/desktop/src/main/lib/terminal-escape-filter.test.ts
📚 Learning: 2025-11-24T21:32:46.559Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T21:32:46.559Z
Learning: Applies to apps/desktop/src/main/lib/*-ipcs.ts : Implement Electron IPC handlers in dedicated files under apps/desktop/src/main/lib/ (e.g., workspace-ipcs.ts, terminal-ipcs.ts)
Applied to files:
apps/desktop/src/main/lib/terminal-escape-filter.ts
🧬 Code graph analysis (1)
apps/desktop/src/main/lib/terminal-escape-filter.test.ts (1)
apps/desktop/src/main/lib/terminal-escape-filter.ts (3)
filterTerminalQueryResponses(250-252)filter(115-141)TerminalEscapeFilter(108-240)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (13)
apps/desktop/src/main/lib/terminal-escape-filter.test.ts (6)
2-5: LGTM!The import correctly adds the new
TerminalEscapeFilterclass alongside the existingfilterTerminalQueryResponsesfunction.
218-232: LGTM!These tests correctly verify the expanded OSC color response pattern now supports 2-4 digit hex values per channel.
234-244: LGTM!The tests appropriately cover the new standard mode report pattern (
ESC[Ps;Pm$y).
478-494: LGTM!Flush behavior tests correctly verify that incomplete buffered sequences are returned (filtered) and that flushing with no buffer returns empty string.
496-503: LGTM!Reset behavior test correctly verifies that the buffer is cleared after calling
reset().
505-522: LGTM!These tests correctly verify that normal text and ANSI color codes are not buffered or delayed, ensuring the filter doesn't introduce false positives.
apps/desktop/src/main/lib/terminal-escape-filter.ts (7)
61-66: LGTM!The new standard mode report pattern correctly matches non-private mode status responses (
ESC[Ps;Pm$y), complementing the existing private mode report pattern.
68-80: LGTM!The expanded OSC color response pattern now correctly supports 2-4 hex digits per channel, accommodating different terminal implementations.
115-141: LGTM!The
filtermethod correctly implements stateful filtering with a conservative buffering strategy: only sequences that look like query responses AND are incomplete are buffered.
143-189: LGTM!The
looksLikeQueryResponsemethod implements a conservative heuristic that correctly identifies potential query response sequences while minimizing false buffering through early rejection of ambiguous patterns.
199-207: LGTM! Critical issue resolved.The CSI completeness check now correctly scans for the first final byte in the CSI body, resolving the critical issue where colored output (like
ESC[31mHello) was incorrectly buffered. This fix ensures normal SGR sequences pass through immediately while still handling truly split query responses.
224-239: LGTM!The
flush()andreset()methods correctly handle buffer cleanup.flush()appropriately filters any remaining buffered data before returning it.
242-252: LGTM!The deprecation notice and updated documentation clearly communicate the function's limitations and provide a clear migration path to the new
TerminalEscapeFilterclass.
| // OSC sequence: ESC ] | ||
| if (secondChar === "]") { | ||
| // OSC ends with BEL or ST (ESC \) | ||
| return !str.includes(BEL) && !str.includes(`${ESC}\\`); | ||
| } | ||
|
|
||
| // DCS sequence: ESC P | ||
| if (secondChar === "P") { | ||
| // DCS ends with ST (ESC \) | ||
| return !str.includes(`${ESC}\\`); | ||
| } |
There was a problem hiding this comment.
ST split across chunks may pass through unfiltered OSC/DCS sequences.
If a String Terminator (ESC \) is split such that ESC arrives at the end of one chunk and \ arrives in the next, the incomplete OSC/DCS sequence will not be buffered (because a solo trailing ESC is rejected by looksLikeQueryResponse), causing the incomplete sequence to pass through unfiltered.
For example:
- Chunk 1:
text ESC]10;rgb:ff/ff/ff ESC - The trailing
ESCalone won't matchlooksLikeQueryResponse, so the incomplete OSCESC]10;rgb:ff/ff/ffpasses through
This is an edge case (terminals typically don't split ST), but for robustness, consider detecting when the combined string ends with a partial OSC/DCS sequence (even if the final ESC is not part of a recognizable pattern).
🤖 Prompt for AI Agents
In apps/desktop/src/main/lib/terminal-escape-filter.ts around lines 209 to 219,
OSC/DCS handling currently only checks for BEL or a complete ST (`ESC\`) inside
the chunk, which lets a trailing lone ESC (split ST) slip through; modify the
checks so that when secondChar is ']' or 'P' you also detect a chunk that ends
with a lone ESC (i.e., str.endsWith(ESC)) and treat that as incomplete (return
false / buffer it) — ensure you still allow a complete ST (`ESC\`) to pass.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/SettingsView/components/ThemeCard/ThemeCard.tsx (1)
11-21: Consider memoizing theaccentColorsarray.The
accentColorsarray is recreated on every render. While the performance impact is minimal for a Settings UI, memoizing it follows React best practices.Apply this diff to optimize:
+import { useMemo } from "react"; import { cn } from "@superset/ui/utils"; import { HiCheck } from "react-icons/hi2"; import type { Theme } from "shared/themes";export function ThemeCard({ theme, isSelected, onSelect }: ThemeCardProps) { const bgColor = theme.terminal.background; const fgColor = theme.terminal.foreground; - const accentColors = [ + const accentColors = useMemo(() => [ theme.terminal.red, theme.terminal.green, theme.terminal.yellow, theme.terminal.blue, theme.terminal.magenta, theme.terminal.cyan, - ]; + ], [theme.terminal]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/renderer/index.html(1 hunks)apps/desktop/src/renderer/screens/main/components/SettingsView/AppearanceSettings.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/SettingsView/components/ThemeCard/ThemeCard.tsx(1 hunks)apps/desktop/src/renderer/screens/main/components/SettingsView/components/ThemeCard/index.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/desktop/src/renderer/screens/main/components/SettingsView/components/ThemeCard/index.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/desktop/src/renderer/index.html
- apps/desktop/src/renderer/screens/main/components/SettingsView/AppearanceSettings.tsx
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx,json}
📄 CodeRabbit inference engine (AGENTS.md)
Apply Biome formatting, linting, and import organization at the root level for all code files
Files:
apps/desktop/src/renderer/screens/main/components/SettingsView/components/ThemeCard/ThemeCard.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid usinganytype; use type-safe approaches instead, unless necessary
Ensure TypeScript type checking passes across all packages using bun run typecheck
Files:
apps/desktop/src/renderer/screens/main/components/SettingsView/components/ThemeCard/ThemeCard.tsx
apps/**/*.{tsx,ts}
📄 CodeRabbit inference engine (AGENTS.md)
apps/**/*.{tsx,ts}: Structure React applications with one folder per component following the pattern: ComponentName/ComponentName.tsx with index.ts barrel export
Co-locate component dependencies (hooks, utils, constants, tests, stories) next to the file using them
Files:
apps/desktop/src/renderer/screens/main/components/SettingsView/components/ThemeCard/ThemeCard.tsx
apps/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Use one component per file; do not create multi-component files
Files:
apps/desktop/src/renderer/screens/main/components/SettingsView/components/ThemeCard/ThemeCard.tsx
apps/desktop/src/renderer/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Define keyboard shortcuts using the centralized system in apps/desktop/src/renderer/lib/shortcuts.ts with Arc Browser-inspired shortcut definitions
Files:
apps/desktop/src/renderer/screens/main/components/SettingsView/components/ThemeCard/ThemeCard.tsx
apps/desktop/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
For Electron interprocess communication, ALWAYS use tRPC as defined in
src/lib/trpc
Files:
apps/desktop/src/renderer/screens/main/components/SettingsView/components/ThemeCard/ThemeCard.tsx
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)
apps/desktop/**/*.{ts,tsx}: Please use alias as defined intsconfig.jsonwhen possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary
Files:
apps/desktop/src/renderer/screens/main/components/SettingsView/components/ThemeCard/ThemeCard.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Build
🔇 Additional comments (4)
apps/desktop/src/renderer/screens/main/components/SettingsView/components/ThemeCard/ThemeCard.tsx (4)
1-3: LGTM!Imports are properly structured and use the workspace alias as per coding guidelines.
5-9: LGTM!Interface is properly typed without using
any, following TypeScript best practices.
34-76: LGTM!The theme preview section effectively demonstrates the terminal color scheme. Inline styles are appropriate here for dynamic theming.
78-91: LGTM!The theme info section correctly handles optional author metadata and provides clear visual feedback for selection state.
| return ( | ||
| <button | ||
| type="button" | ||
| onClick={onSelect} | ||
| className={cn( | ||
| "relative flex flex-col rounded-lg border-2 overflow-hidden transition-all text-left", | ||
| isSelected | ||
| ? "border-primary ring-2 ring-primary/20" | ||
| : "border-border hover:border-muted-foreground/50", | ||
| )} | ||
| > |
There was a problem hiding this comment.
Add accessibility attributes for screen reader users.
The button lacks an accessible name and does not communicate its selected state to assistive technologies. Screen reader users cannot identify which theme the button represents or whether it is currently selected.
Apply this diff to add accessibility attributes:
<button
type="button"
onClick={onSelect}
+ aria-label={`${theme.name} theme${theme.author ? ` by ${theme.author}` : ""}`}
+ aria-pressed={isSelected}
className={cn(📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return ( | |
| <button | |
| type="button" | |
| onClick={onSelect} | |
| className={cn( | |
| "relative flex flex-col rounded-lg border-2 overflow-hidden transition-all text-left", | |
| isSelected | |
| ? "border-primary ring-2 ring-primary/20" | |
| : "border-border hover:border-muted-foreground/50", | |
| )} | |
| > | |
| return ( | |
| <button | |
| type="button" | |
| onClick={onSelect} | |
| aria-label={`${theme.name} theme${theme.author ? ` by ${theme.author}` : ""}`} | |
| aria-pressed={isSelected} | |
| className={cn( | |
| "relative flex flex-col rounded-lg border-2 overflow-hidden transition-all text-left", | |
| isSelected | |
| ? "border-primary ring-2 ring-primary/20" | |
| : "border-border hover:border-muted-foreground/50", | |
| )} | |
| > |
🤖 Prompt for AI Agents
In
apps/desktop/src/renderer/screens/main/components/SettingsView/components/ThemeCard/ThemeCard.tsx
around lines 23–33, the button is missing an accessible name and does not expose
its selected state to assistive tech; update the button to include an accessible
name (e.g., aria-label={name} or aria-labelledby pointing to the theme title)
and expose selection with an appropriate ARIA attribute (e.g.,
aria-pressed={isSelected} for a toggle-like button or aria-checked/role="radio"
if it’s part of a radio group), and ensure the value comes from the component
props so screen readers know which theme it is and whether it is selected.
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
New Features
Enhancements
Enhancements / Terminal
Security
✏️ Tip: You can customize this high-level summary in your review settings.