-
Notifications
You must be signed in to change notification settings - Fork 516
fix: theme change issue #770
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds a system-aware theme flow: introduces THEME_CHANGED and SYSTEM_THEME_UPDATED events, renames toggleTheme to setTheme, adds getCurrentThemeIsDark, updates presenter to accept 'system', and refactors renderer to compute effective theme, react to system/user theme events, and apply dark mode accordingly. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant App.vue
participant ThemeStore
participant RendererIPC as Renderer Events
participant Main as Main ConfigPresenter
participant NativeTheme
rect rgb(240,248,255)
note over ThemeStore: Init
ThemeStore->>Main: getCurrentThemeIsDark()
Main->>NativeTheme: shouldUseDarkColors
NativeTheme-->>Main: boolean
Main-->>ThemeStore: isDark
ThemeStore->>App.vue: apply toggleDark(isDark)
end
rect rgb(245,255,240)
note over User,ThemeStore: User changes theme
User->>ThemeStore: cycleTheme()
ThemeStore->>Main: setTheme('light'|'dark'|'system')
Main->>RendererIPC: broadcast CONFIG_EVENTS.THEME_CHANGED(theme)
RendererIPC-->>ThemeStore: THEME_CHANGED(theme)
ThemeStore->>Main: getCurrentThemeIsDark()
Main-->>ThemeStore: isDark
ThemeStore->>App.vue: apply toggleDark(isDark)
end
rect rgb(255,250,240)
note over NativeTheme,ThemeStore: System theme changed
NativeTheme-->>Main: updated theme
Main->>RendererIPC: SYSTEM_EVENTS.SYSTEM_THEME_UPDATED(isDark)
RendererIPC-->>ThemeStore: SYSTEM_THEME_UPDATED(isDark)
alt themeMode == 'system'
ThemeStore->>App.vue: apply toggleDark(isDark)
else themeMode != 'system'
note over ThemeStore: ignore system change
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/src/stores/theme.ts (1)
19-27
: Add error handling, fallback, and structured logging in initTheme()IPC can fail and getTheme may be missing on first run. Also, you can avoid an extra round-trip when theme ≠ 'system'.
Apply:
const initTheme = async () => { - const currentTheme = (await configPresenter.getTheme()) as ThemeMode - themeMode.value = currentTheme - - // 获取当前实际的深色模式状态 - const isDarkMode = await configPresenter.getCurrentThemeIsDark() - console.log('initTheme - theme:', currentTheme, 'isDark:', isDarkMode) - toggleDark(isDarkMode) + try { + const reportedTheme = (await configPresenter.getTheme()) as ThemeMode | null | undefined + const currentTheme: ThemeMode = reportedTheme ?? 'system' + themeMode.value = currentTheme + const isDarkMode = + currentTheme === 'system' + ? await configPresenter.getCurrentThemeIsDark() + : currentTheme === 'dark' + log('INFO', 'initTheme', { theme: currentTheme, isDark: isDarkMode }) + toggleDark(isDarkMode) + } catch (err) { + log('ERROR', 'initTheme failed, defaulting to system', { err }) + } }Add a tiny structured logger (place inside the store scope, e.g., near other consts):
const log = ( level: 'ERROR' | 'WARN' | 'INFO' | 'DEBUG', message: string, context?: Record<string, unknown> ) => { const payload = { ts: new Date().toISOString(), level, scope: 'ThemeStore', message, ...context } if (level === 'ERROR') console.error(payload) else if (level === 'WARN') console.warn(payload) else if (level === 'INFO') console.info(payload) else console.debug(payload) }
🧹 Nitpick comments (4)
src/main/events.ts (1)
34-36
: Add trailing comma and keep comment in English for consistencyStyle nit: add a trailing comma after the last entry to minimize diffs on future additions and align with the surrounding style. Also prefer English comments per guidelines.
- OAUTH_LOGIN_ERROR: 'config:oauth-login-error', // OAuth登录失败 - THEME_CHANGED: 'config:theme-changed' // 主题变更事件 + OAUTH_LOGIN_ERROR: 'config:oauth-login-error', // OAuth login failed + THEME_CHANGED: 'config:theme-changed', // Theme changed eventsrc/renderer/src/events.ts (1)
23-25
: Add trailing comma after THEME_CHANGEDKeeps diffs smaller and matches existing style.
- THEME_CHANGED: 'config:theme-changed' + THEME_CHANGED: 'config:theme-changed',src/renderer/src/stores/theme.ts (2)
73-79
: Use structured logs in cycleTheme()Align with logging guidelines; keep logs consistent and machine-readable.
- const cycleTheme = async () => { - console.log('cycleTheme', themeMode.value) + const cycleTheme = async () => { + log('INFO', 'cycleTheme', { from: themeMode.value }) if (themeMode.value === 'light') await setThemeMode('dark') else if (themeMode.value === 'dark') await setThemeMode('system') else await setThemeMode('light') }
15-16
: Translate comments to English per project guidelinesLogs and comments must be in English. Suggested replacements only (no behavior change).
- // 存储当前主题模式 + // Current theme mode - // 初始化主题 + // Initialize theme from config and system - // 监听系统主题变化事件 + // Listen to system theme updates - // 监听用户主题变化事件 + // Listen to user-initiated theme changes - // 注册和清理主题变化监听器 + // Register/unregister theme change listeners - // 设置主题模式 + // Set theme mode - // 循环切换主题:light -> dark -> system -> light + // Cycle theme: light -> dark -> system -> lightAlso applies to: 18-18, 31-31, 39-39, 50-50, 64-64, 73-73
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
src/main/events.ts
(1 hunks)src/main/presenter/configPresenter/index.ts
(1 hunks)src/renderer/src/App.vue
(1 hunks)src/renderer/src/events.ts
(2 hunks)src/renderer/src/stores/theme.ts
(2 hunks)src/shared/presenter.d.ts
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (20)
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for logs and comments
Files:
src/renderer/src/App.vue
src/shared/presenter.d.ts
src/renderer/src/events.ts
src/main/events.ts
src/renderer/src/stores/theme.ts
src/main/presenter/configPresenter/index.ts
src/renderer/src/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/src/**/*.vue
: Use Composition API for all Vue 3 components
Use Tailwind CSS with scoped styles for styling
Organize components by feature in src/renderer/src/
Follow existing component patterns in src/renderer/src/ when creating new UI components
Use Composition API with proper TypeScript typing for new UI components
Implement responsive design with Tailwind CSS for new UI components
Add proper error handling and loading states for new UI componentsUse scoped styles to prevent CSS conflicts between components
Files:
src/renderer/src/App.vue
src/renderer/src/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/src/**/*.{ts,tsx,vue}
: Use Pinia for frontend state management
Renderer to Main: Use usePresenter.ts composable for direct presenter method calls
Files:
src/renderer/src/App.vue
src/renderer/src/events.ts
src/renderer/src/stores/theme.ts
src/renderer/src/**/*
📄 CodeRabbit inference engine (.cursor/rules/i18n.mdc)
src/renderer/src/**/*
: All user-facing strings must use i18n keys (avoid hardcoded user-visible text in code)
Use the 'vue-i18n' framework for all internationalization in the renderer
Ensure all user-visible text in the renderer uses the translation system
Files:
src/renderer/src/App.vue
src/renderer/src/events.ts
src/renderer/src/stores/theme.ts
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/src/App.vue
src/renderer/src/events.ts
src/renderer/src/stores/theme.ts
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)
src/renderer/src/**/*.{vue,ts,tsx,js,jsx}
: Use the Composition API for better code organization and reusability
Implement proper state management with Pinia
Utilize Vue Router for navigation and route management
Leverage Vue's built-in reactivity system for efficient data handling
Files:
src/renderer/src/App.vue
src/renderer/src/events.ts
src/renderer/src/stores/theme.ts
src/renderer/**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,tsx,vue}
: Use descriptive variable names with auxiliary verbs (e.g., isLoading, hasError).
Use TypeScript for all code; prefer types over interfaces.
Avoid enums; use const objects instead.
Use arrow functions for methods and computed properties.
Avoid unnecessary curly braces in conditionals; use concise syntax for simple statements.
Files:
src/renderer/src/App.vue
src/renderer/src/events.ts
src/renderer/src/stores/theme.ts
src/renderer/**/*.{vue,ts}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
Implement lazy loading for routes and components.
Files:
src/renderer/src/App.vue
src/renderer/src/events.ts
src/renderer/src/stores/theme.ts
src/renderer/**/*.{ts,vue}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
src/renderer/**/*.{ts,vue}
: Use useFetch and useAsyncData for data fetching.
Implement SEO best practices using Nuxt's useHead and useSeoMeta.
Files:
src/renderer/src/App.vue
src/renderer/src/events.ts
src/renderer/src/stores/theme.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Strict type checking enabled for TypeScript
**/*.{ts,tsx}
: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪(如适用)、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/shared/presenter.d.ts
src/renderer/src/events.ts
src/main/events.ts
src/renderer/src/stores/theme.ts
src/main/presenter/configPresenter/index.ts
src/shared/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Shared types in src/shared/
Files:
src/shared/presenter.d.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}
: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/shared/presenter.d.ts
src/renderer/src/events.ts
src/main/events.ts
src/renderer/src/stores/theme.ts
src/main/presenter/configPresenter/index.ts
src/shared/*.d.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
The shared/*.d.ts files are used to define the types of objects exposed by the main process to the renderer process
Files:
src/shared/presenter.d.ts
src/shared/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
共享类型定义放在
shared
目录
Files:
src/shared/presenter.d.ts
src/{main,renderer}/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
src/{main,renderer}/**/*.ts
: Use context isolation for improved security
Implement proper inter-process communication (IPC) patterns
Optimize application startup time with lazy loading
Implement proper error handling and logging for debugging
Files:
src/renderer/src/events.ts
src/main/events.ts
src/renderer/src/stores/theme.ts
src/main/presenter/configPresenter/index.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Main to Renderer: Use EventBus to broadcast events via mainWindow.webContents.send()
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/events.ts
src/main/presenter/configPresenter/index.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/events.ts
src/main/presenter/configPresenter/index.ts
src/renderer/src/stores/**/*.{vue,ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/pinia-best-practices.mdc)
src/renderer/src/stores/**/*.{vue,ts,tsx,js,jsx}
: Use modules to organize related state and actions
Implement proper state persistence for maintaining data across sessions
Use getters for computed state properties
Utilize actions for side effects and asynchronous operations
Keep the store focused on global state, not component-specific data
Files:
src/renderer/src/stores/theme.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
One presenter per functional domain
Files:
src/main/presenter/configPresenter/index.ts
src/main/presenter/configPresenter/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Centralize configuration in configPresenter/
Files:
src/main/presenter/configPresenter/index.ts
🧠 Learnings (4)
📚 Learning: 2025-07-21T01:45:33.790Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T01:45:33.790Z
Learning: Applies to src/main/presenter/configPresenter/**/*.ts : Centralize configuration in configPresenter/
Applied to files:
src/shared/presenter.d.ts
src/main/presenter/configPresenter/index.ts
📚 Learning: 2025-07-21T01:45:33.790Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-21T01:45:33.790Z
Learning: Applies to src/renderer/src/**/*.{ts,tsx,vue} : Use Pinia for frontend state management
Applied to files:
src/renderer/src/stores/theme.ts
📚 Learning: 2025-07-23T00:45:57.322Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/vue-shadcn.mdc:0-0
Timestamp: 2025-07-23T00:45:57.322Z
Learning: Applies to src/renderer/stores/**/*.ts : Use Pinia for state management.
Applied to files:
src/renderer/src/stores/theme.ts
📚 Learning: 2025-07-21T01:47:28.817Z
Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/vue-best-practices.mdc:0-0
Timestamp: 2025-07-21T01:47:28.817Z
Learning: Applies to src/renderer/src/**/*.{vue,ts,tsx,js,jsx} : Implement proper state management with Pinia
Applied to files:
src/renderer/src/stores/theme.ts
🧬 Code graph analysis (3)
src/renderer/src/events.ts (1)
src/main/events.ts (1)
SYSTEM_EVENTS
(57-59)
src/renderer/src/stores/theme.ts (3)
src/main/presenter/configPresenter/index.ts (1)
initTheme
(997-1009)src/main/events.ts (2)
SYSTEM_EVENTS
(57-59)CONFIG_EVENTS
(12-36)src/renderer/src/events.ts (2)
SYSTEM_EVENTS
(148-150)CONFIG_EVENTS
(11-25)
src/main/presenter/configPresenter/index.ts (2)
src/main/eventbus.ts (1)
eventBus
(151-151)src/main/events.ts (2)
SYSTEM_EVENTS
(57-59)CONFIG_EVENTS
(12-36)
⏰ 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-check (x64)
🔇 Additional comments (7)
src/renderer/src/events.ts (1)
147-150
: LGTM: New SYSTEM_EVENTS mapping is coherent with mainThe renderer mirror for SYSTEM_THEME_UPDATED matches main/events.ts. No issues.
src/shared/presenter.d.ts (2)
431-431
: LGTM: setTheme signature now returns PromiseReturn type aligns with implementation and lets renderer know the effective dark state.
433-435
: Confirmed: No staletoggleTheme
references;getCurrentThemeIsDark
API is wired and consumed correctly
- No occurrences of the old
toggleTheme()
API found across.ts
or.vue
files.getCurrentThemeIsDark()
is declared insrc/shared/presenter.d.ts
(lines 433–435) and implemented insrc/main/presenter/configPresenter/index.ts
(lines 1023–1025).- The renderer store consumes the new API in
src/renderer/src/stores/theme.ts
at lines 24 and 42.- Listeners for
SYSTEM_EVENTS.SYSTEM_THEME_UPDATED
andCONFIG_EVENTS.THEME_CHANGED
are correctly registered (lines 52–53) and removed (lines 58–61) insrc/renderer/src/stores/theme.ts
.- Main‐process propagation of theme‐change events is handled in
src/main/presenter/tabPresenter.ts
(line 94) andsrc/main/presenter/windowPresenter/index.ts
(line 136).All checks out—no further changes needed.
src/main/presenter/configPresenter/index.ts (3)
1000-1001
: LGTM: Accept 'system' in nativeTheme.themeSourceThis correctly honors the stored theme preference.
1011-1017
: LGTM: setTheme persists, updates Electron theme, and broadcasts THEME_CHANGEDBroadcasting to ALL_WINDOWS matches the event model; returning shouldUseDarkColors is convenient for immediate renderer alignment.
1023-1025
: LGTM: getCurrentThemeIsDark mirrors nativeTheme.shouldUseDarkColorsSimple and correct.
src/renderer/src/stores/theme.ts (1)
6-6
: Good: centralizing event names via '@/events'Importing CONFIG_EVENTS and SYSTEM_EVENTS eliminates magic strings and keeps renderer/main aligned.
nativeTheme.on('updated', () => { | ||
// 只有当主题设置为 system 时,才需要通知渲染进程 | ||
// 只有当主题设置为 system 时,才需要通知渲染进程系统主题变化 | ||
if (nativeTheme.themeSource === 'system') { | ||
eventBus.sendToMain(SYSTEM_EVENTS.SYSTEM_THEME_UPDATED, nativeTheme.shouldUseDarkColors) | ||
} | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
System theme update is sent to main instead of renderer
You intend to notify renderers on OS theme changes when themeSource === 'system', but eventBus.sendToMain delivers only within main. Use sendToRenderer and broadcast to all windows.
- nativeTheme.on('updated', () => {
- // 只有当主题设置为 system 时,才需要通知渲染进程系统主题变化
- if (nativeTheme.themeSource === 'system') {
- eventBus.sendToMain(SYSTEM_EVENTS.SYSTEM_THEME_UPDATED, nativeTheme.shouldUseDarkColors)
- }
- })
+ nativeTheme.on('updated', () => {
+ // Notify renderers only when app is following the system theme
+ if (nativeTheme.themeSource === 'system') {
+ eventBus.sendToRenderer(
+ SYSTEM_EVENTS.SYSTEM_THEME_UPDATED,
+ SendTarget.ALL_WINDOWS,
+ nativeTheme.shouldUseDarkColors
+ )
+ }
+ })
📝 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.
nativeTheme.on('updated', () => { | |
// 只有当主题设置为 system 时,才需要通知渲染进程 | |
// 只有当主题设置为 system 时,才需要通知渲染进程系统主题变化 | |
if (nativeTheme.themeSource === 'system') { | |
eventBus.sendToMain(SYSTEM_EVENTS.SYSTEM_THEME_UPDATED, nativeTheme.shouldUseDarkColors) | |
} | |
}) | |
nativeTheme.on('updated', () => { | |
// Notify renderers only when app is following the system theme | |
if (nativeTheme.themeSource === 'system') { | |
eventBus.sendToRenderer( | |
SYSTEM_EVENTS.SYSTEM_THEME_UPDATED, | |
SendTarget.ALL_WINDOWS, | |
nativeTheme.shouldUseDarkColors | |
) | |
} | |
}) |
🤖 Prompt for AI Agents
In src/main/presenter/configPresenter/index.ts around lines 1003-1008, the
handler currently calls eventBus.sendToMain when the OS theme changes, but that
only sends within the main process; replace that call with the eventBus API that
sends to renderers and ensure the message is broadcast to all renderer windows
(use eventBus.sendToRenderer and/or the eventBus.broadcast helper so every
BrowserWindow receives SYSTEM_EVENTS.SYSTEM_THEME_UPDATED with
nativeTheme.shouldUseDarkColors).
let newThemeName = newTheme | ||
if (newTheme === 'system') { | ||
newThemeName = themeStore.isDark ? 'dark' : 'light' | ||
} | ||
if (oldTheme) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix stale CSS classes when switching to/from 'system' theme and align target element
Bug: removing oldTheme (which can be 'system') won’t remove the actually applied 'dark'/'light' class, causing class accumulation and theme flicker. Also, elsewhere you add classes to body, but here you mutate documentElement, leading to inconsistency.
Replace removal with explicit removal of 'dark' and 'light', then add the effective theme. Keep logs in English and prefer debug level.
- let newThemeName = newTheme
- if (newTheme === 'system') {
- newThemeName = themeStore.isDark ? 'dark' : 'light'
- }
- if (oldTheme) {
- document.documentElement.classList.remove(oldTheme)
- }
+ const effectiveTheme =
+ newTheme === 'system' ? (themeStore.isDark ? 'dark' : 'light') : newTheme
+ // Always normalize classes before applying the new one
+ document.documentElement.classList.remove('dark')
+ document.documentElement.classList.remove('light')
@@
- document.documentElement.classList.add(newThemeName)
+ document.documentElement.classList.add(effectiveTheme)
@@
- console.log('newTheme', newThemeName)
+ console.debug('[theme] applied', { themeMode: newTheme, effectiveTheme })
Additionally, align initial mount with the same element and effective theme (outside the changed range):
// onMounted initial classes
const initialEffectiveTheme =
themeStore.themeMode === 'system' ? (themeStore.isDark ? 'dark' : 'light') : themeStore.themeMode
document.documentElement.classList.add(initialEffectiveTheme)
document.documentElement.classList.add(settingsStore.fontSizeClass)
Also applies to: 52-55
const handleUserThemeChange = (_event: IpcRendererEvent, theme: ThemeMode) => { | ||
if (themeMode.value !== theme) { | ||
configPresenter.getCurrentThemeIsDark().then((isDark) => { | ||
console.log('handleUserThemeChange', theme, isDark) | ||
themeMode.value = theme | ||
toggleDark(isDark) | ||
}) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid unnecessary IPC and add try/catch in handleUserThemeChange()
Compute isDark locally for 'dark'/'light' and only query main for 'system'. Also switch to await + try/catch for clearer flow and required error handling.
- const handleUserThemeChange = (_event: IpcRendererEvent, theme: ThemeMode) => {
- if (themeMode.value !== theme) {
- configPresenter.getCurrentThemeIsDark().then((isDark) => {
- console.log('handleUserThemeChange', theme, isDark)
- themeMode.value = theme
- toggleDark(isDark)
- })
- }
- }
+ const handleUserThemeChange = async (_event: IpcRendererEvent, theme: ThemeMode) => {
+ if (themeMode.value === theme) return
+ try {
+ const isDark =
+ theme === 'system'
+ ? await configPresenter.getCurrentThemeIsDark()
+ : theme === 'dark'
+ log('INFO', 'handleUserThemeChange', { theme, isDark })
+ themeMode.value = theme
+ toggleDark(isDark)
+ } catch (err) {
+ log('ERROR', 'handleUserThemeChange failed', { theme, err })
+ }
+ }
📝 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.
const handleUserThemeChange = (_event: IpcRendererEvent, theme: ThemeMode) => { | |
if (themeMode.value !== theme) { | |
configPresenter.getCurrentThemeIsDark().then((isDark) => { | |
console.log('handleUserThemeChange', theme, isDark) | |
themeMode.value = theme | |
toggleDark(isDark) | |
}) | |
} | |
} | |
const handleUserThemeChange = async (_event: IpcRendererEvent, theme: ThemeMode) => { | |
if (themeMode.value === theme) return | |
try { | |
const isDark = | |
theme === 'system' | |
? await configPresenter.getCurrentThemeIsDark() | |
: theme === 'dark' | |
log('INFO', 'handleUserThemeChange', { theme, isDark }) | |
themeMode.value = theme | |
toggleDark(isDark) | |
} catch (err) { | |
log('ERROR', 'handleUserThemeChange failed', { theme, err }) | |
} | |
} |
🤖 Prompt for AI Agents
In src/renderer/src/stores/theme.ts around lines 40 to 48, handleUserThemeChange
currently always calls configPresenter.getCurrentThemeIsDark() via IPC and uses
.then; change it to compute isDark locally when theme is 'dark' or 'light' and
only invoke the IPC call when theme === 'system', use async/await and wrap the
IPC await in try/catch to handle errors, set themeMode.value and then call
toggleDark(isDark) after resolving isDark, and ensure no IPC is invoked when the
incoming theme equals the current theme.
onMounted(() => { | ||
window.electron.ipcRenderer.on('system-theme-updated', handleSystemThemeChange) | ||
window.electron.ipcRenderer.on(SYSTEM_EVENTS.SYSTEM_THEME_UPDATED, handleSystemThemeChange) | ||
window.electron.ipcRenderer.on(CONFIG_EVENTS.THEME_CHANGED, handleUserThemeChange) | ||
}) | ||
|
||
onUnmounted(() => { | ||
window.electron.ipcRenderer.removeListener('system-theme-updated', handleSystemThemeChange) | ||
window.electron.ipcRenderer.removeListener( | ||
SYSTEM_EVENTS.SYSTEM_THEME_UPDATED, | ||
handleSystemThemeChange | ||
) | ||
window.electron.ipcRenderer.removeListener(CONFIG_EVENTS.THEME_CHANGED, handleUserThemeChange) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Vue lifecycle hooks don’t run inside Pinia stores — IPC listeners won’t register
onMounted/onUnmounted are component hooks; in a Pinia store they are no-ops, so your handlers never get attached. Register listeners once at store creation with an idempotent guard and clean up on HMR.
- // 注册和清理主题变化监听器
- onMounted(() => {
- window.electron.ipcRenderer.on(SYSTEM_EVENTS.SYSTEM_THEME_UPDATED, handleSystemThemeChange)
- window.electron.ipcRenderer.on(CONFIG_EVENTS.THEME_CHANGED, handleUserThemeChange)
- })
-
- onUnmounted(() => {
- window.electron.ipcRenderer.removeListener(
- SYSTEM_EVENTS.SYSTEM_THEME_UPDATED,
- handleSystemThemeChange
- )
- window.electron.ipcRenderer.removeListener(CONFIG_EVENTS.THEME_CHANGED, handleUserThemeChange)
- })
+ // Register/unregister IPC listeners once per store instance (no component lifecycle here)
+ let listenersRegistered = false
+ const registerIpcListeners = () => {
+ if (listenersRegistered) return
+ try {
+ window.electron.ipcRenderer.on(
+ SYSTEM_EVENTS.SYSTEM_THEME_UPDATED,
+ handleSystemThemeChange
+ )
+ window.electron.ipcRenderer.on(
+ CONFIG_EVENTS.THEME_CHANGED,
+ handleUserThemeChange
+ )
+ listenersRegistered = true
+ log('INFO', 'IPC listeners registered')
+ } catch (err) {
+ log('ERROR', 'Failed to register IPC listeners', { err })
+ }
+ }
+ const unregisterIpcListeners = () => {
+ if (!listenersRegistered) return
+ try {
+ window.electron.ipcRenderer.removeListener(
+ SYSTEM_EVENTS.SYSTEM_THEME_UPDATED,
+ handleSystemThemeChange
+ )
+ window.electron.ipcRenderer.removeListener(
+ CONFIG_EVENTS.THEME_CHANGED,
+ handleUserThemeChange
+ )
+ } finally {
+ listenersRegistered = false
+ log('INFO', 'IPC listeners unregistered')
+ }
+ }
+ registerIpcListeners()
+ // Ensure cleanup during HMR
+ if (import.meta.hot) {
+ import.meta.hot.dispose(unregisterIpcListeners)
+ }
📝 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.
onMounted(() => { | |
window.electron.ipcRenderer.on('system-theme-updated', handleSystemThemeChange) | |
window.electron.ipcRenderer.on(SYSTEM_EVENTS.SYSTEM_THEME_UPDATED, handleSystemThemeChange) | |
window.electron.ipcRenderer.on(CONFIG_EVENTS.THEME_CHANGED, handleUserThemeChange) | |
}) | |
onUnmounted(() => { | |
window.electron.ipcRenderer.removeListener('system-theme-updated', handleSystemThemeChange) | |
window.electron.ipcRenderer.removeListener( | |
SYSTEM_EVENTS.SYSTEM_THEME_UPDATED, | |
handleSystemThemeChange | |
) | |
window.electron.ipcRenderer.removeListener(CONFIG_EVENTS.THEME_CHANGED, handleUserThemeChange) | |
}) | |
// Register/unregister IPC listeners once per store instance (no component lifecycle here) | |
let listenersRegistered = false | |
const registerIpcListeners = () => { | |
if (listenersRegistered) return | |
try { | |
window.electron.ipcRenderer.on( | |
SYSTEM_EVENTS.SYSTEM_THEME_UPDATED, | |
handleSystemThemeChange | |
) | |
window.electron.ipcRenderer.on( | |
CONFIG_EVENTS.THEME_CHANGED, | |
handleUserThemeChange | |
) | |
listenersRegistered = true | |
log('INFO', 'IPC listeners registered') | |
} catch (err) { | |
log('ERROR', 'Failed to register IPC listeners', { err }) | |
} | |
} | |
const unregisterIpcListeners = () => { | |
if (!listenersRegistered) return | |
try { | |
window.electron.ipcRenderer.removeListener( | |
SYSTEM_EVENTS.SYSTEM_THEME_UPDATED, | |
handleSystemThemeChange | |
) | |
window.electron.ipcRenderer.removeListener( | |
CONFIG_EVENTS.THEME_CHANGED, | |
handleUserThemeChange | |
) | |
} finally { | |
listenersRegistered = false | |
log('INFO', 'IPC listeners unregistered') | |
} | |
} | |
registerIpcListeners() | |
// Ensure cleanup during HMR | |
if (import.meta.hot) { | |
import.meta.hot.dispose(unregisterIpcListeners) | |
} |
🤖 Prompt for AI Agents
In src/renderer/src/stores/theme.ts around lines 51 to 62, onMounted/onUnmounted
are component hooks and do not run in a Pinia store, so the IPC listeners never
get attached; change to register the listeners at store creation (top-level
inside the store factory) and protect registration with an idempotent guard
(e.g., a module-level or store-scoped boolean) so repeated imports/HMR don't
double-register, use window.electron.ipcRenderer.on(...) to attach and
window.electron.ipcRenderer.removeListener(...) to detach, and add HMR cleanup
(import.meta.hot?.dispose) to remove listeners and reset the guard on module
replacement.
const setThemeMode = async (mode: ThemeMode) => { | ||
themeMode.value = mode | ||
const isDarkMode = await configPresenter.toggleTheme(mode) | ||
const isDarkMode = await configPresenter.setTheme(mode) | ||
|
||
// 如果不是系统模式,直接设置深色/浅色状态 | ||
// 如果是系统模式,toggleTheme 会返回系统当前的深色状态 | ||
// 设置界面深色/浅色状态 | ||
toggleDark(isDarkMode) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Set theme atomically with error handling; update state only on success
Currently themeMode is set before setTheme() resolves. If IPC fails, UI and persisted setting diverge. Move assignment after success and add required structured logging.
- const setThemeMode = async (mode: ThemeMode) => {
- themeMode.value = mode
- const isDarkMode = await configPresenter.setTheme(mode)
-
- // 设置界面深色/浅色状态
- toggleDark(isDarkMode)
- }
+ const setThemeMode = async (mode: ThemeMode) => {
+ try {
+ const isDarkMode = await configPresenter.setTheme(mode)
+ themeMode.value = mode
+ log('INFO', 'setThemeMode', { mode, isDark: isDarkMode })
+ toggleDark(isDarkMode)
+ } catch (err) {
+ log('ERROR', 'setThemeMode failed', { mode, err })
+ }
+ }
📝 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.
const setThemeMode = async (mode: ThemeMode) => { | |
themeMode.value = mode | |
const isDarkMode = await configPresenter.toggleTheme(mode) | |
const isDarkMode = await configPresenter.setTheme(mode) | |
// 如果不是系统模式,直接设置深色/浅色状态 | |
// 如果是系统模式,toggleTheme 会返回系统当前的深色状态 | |
// 设置界面深色/浅色状态 | |
toggleDark(isDarkMode) | |
} | |
const setThemeMode = async (mode: ThemeMode) => { | |
try { | |
const isDarkMode = await configPresenter.setTheme(mode) | |
themeMode.value = mode | |
log('INFO', 'setThemeMode', { mode, isDark: isDarkMode }) | |
toggleDark(isDarkMode) | |
} catch (err) { | |
log('ERROR', 'setThemeMode failed', { mode, err }) | |
} | |
} |
🤖 Prompt for AI Agents
In src/renderer/src/stores/theme.ts around lines 65 to 71, themeMode is
currently assigned before awaiting configPresenter.setTheme, which can lead to
UI and persisted state divergence if the IPC call fails; change the code to
await configPresenter.setTheme first, only update themeMode and call toggleDark
when the call succeeds, and wrap the await in a try/catch that logs structured
error details (e.g., { action: "setTheme", mode, error }) using the existing
logger so failures do not mutate UI state.
return { | ||
isDark, | ||
toggleDark, | ||
themeMode, | ||
setThemeMode, | ||
cycleTheme | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Don’t export toggleDark; expose setThemeMode to keep renderer and main in sync
Exporting toggleDark enables “local-only” toggles that bypass configPresenter and events, causing desync across windows and restarts. Surface setThemeMode instead and keep toggleDark private.
return {
isDark,
- toggleDark,
themeMode,
- cycleTheme
+ cycleTheme,
+ setThemeMode
}
Run to find any direct usages of toggleDark you’ll need to migrate:
🏁 Script executed:
#!/bin/bash
rg -nP -C2 --type=ts --type=tsx --type=vue '\b(useThemeStore\(\)|themeStore)\.toggleDark\b|toggleDark\s*\('
Length of output: 137
🏁 Script executed:
#!/bin/bash
# Re-run search for direct usages of toggleDark across .ts, .tsx, and .vue files
rg -nP -C2 '\b(useThemeStore\(\)|themeStore)\.toggleDark\b|toggleDark\s*\(' --glob '*.ts' --glob '*.tsx' --glob '*.vue'
Length of output: 1750
Action Required: Remove toggleDark
from exports and Migrate Its Consumers
Based on the grep search, toggleDark
is only used internally within theme.ts
(lines 26, 36, 45, 70) and externally in:
src/renderer/src/components/SideBar.vue
at line 59:@click="themeStore.toggleDark()"
After you remove toggleDark
from the store’s returned API, that click handler will break. To resolve:
• In src/renderer/src/stores/theme.ts
, update the return value as suggested:
return {
isDark,
- toggleDark,
themeMode,
cycleTheme,
+ setThemeMode
}
• In src/renderer/src/components/SideBar.vue
, replace the direct toggle call with one of:
- @click="themeStore.toggleDark()"
+ @click="themeStore.setThemeMode(themeStore.isDark ? 'light' : 'dark')"
—or, if you prefer cycling through modes (including system
):
- @click="themeStore.toggleDark()"
+ @click="themeStore.cycleTheme()"
No other external usages of toggleDark
were found.
Please update the component(s) accordingly before removing the exported toggleDark
.
🤖 Prompt for AI Agents
In src/renderer/src/stores/theme.ts around lines 81-86 and
src/renderer/src/components/SideBar.vue at line 59: remove toggleDark from the
store's returned API (so it is no longer exported) and update any external
callers (SideBar.vue) to use either themeStore.cycleTheme() or directly flip the
mode by assigning themeStore.themeMode = themeStore.themeMode === 'dark' ?
'light' : 'dark' (or set to a specific string), then test the click handler to
ensure it still toggles theme; do not change internal uses inside theme.ts that
remain private.
Fix the issue with theme switching disorder.
Summary by CodeRabbit
New Features
Bug Fixes