-
Notifications
You must be signed in to change notification settings - Fork 625
Fix/win10 appbar #1048
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
Fix/win10 appbar #1048
Conversation
WalkthroughThis PR refactors platform detection by centralizing OS version metadata in the device presenter and extracting platform detection logic into a reusable Vue composable. The device presenter now exposes platform, osVersion, and osVersionMetadata with OS-specific name/build pairs, while the new Changes
Sequence DiagramsequenceDiagram
participant Component
participant useDeviceVersion
participant DevicePresenter
Component->>useDeviceVersion: Import & Initialize
useDeviceVersion->>DevicePresenter: onMounted: getDeviceInfo()
DevicePresenter-->>useDeviceVersion: Returns { platform, osVersion, osVersionMetadata }
useDeviceVersion->>useDeviceVersion: Derive isMacOS & isWinMacOS
useDeviceVersion-->>Component: Return reactive { isMacOS, isWinMacOS }
Component->>Component: Render with composable state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Rationale: The changes involve a new composable with platform detection logic, type additions, and refactoring across multiple components following a consistent pattern. While each individual file contains straightforward logic, the distributed nature across 5 files and the introduction of new detection semantics (Windows 11+ build parsing) warrant careful verification of detection correctness and state propagation. Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/renderer/settings/App.vue (1)
163-168: Remove duplicate device detection logic.The
useDeviceVersioncomposable already initializesisMacOSandisWinMacOSin its ownonMountedhook (line 76). This code duplicates that logic and will overwrite the composable's values, causing unnecessary device info fetches and potential race conditions.Apply this diff to remove the redundant logic:
-onMounted(() => { - // Listen for window maximize/unmaximize events - devicePresenter.getDeviceInfo().then((deviceInfo: any) => { - isMacOS.value = deviceInfo.platform === 'darwin' - }) -})
🧹 Nitpick comments (1)
src/renderer/src/composables/useDeviceVersion.ts (1)
30-63: Consider adding explicit return type annotation.While TypeScript can infer the return type, adding an explicit annotation improves code clarity and helps catch type errors earlier.
+type UseDeviceVersionReturn = { + isWinMacOS: Ref<boolean> + isMacOS: Ref<boolean> +} + -export function useDeviceVersion() { +export function useDeviceVersion(): UseDeviceVersionReturn { // ... implementation }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/presenter/devicePresenter/index.ts(1 hunks)src/renderer/settings/App.vue(2 hunks)src/renderer/shell/App.vue(1 hunks)src/renderer/src/composables/useDeviceVersion.ts(1 hunks)src/shared/types/presenters/legacy.presenters.d.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (24)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)
**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写
Files:
src/main/presenter/devicePresenter/index.tssrc/shared/types/presenters/legacy.presenters.d.tssrc/renderer/src/composables/useDeviceVersion.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/main/presenter/devicePresenter/index.tssrc/renderer/src/composables/useDeviceVersion.ts
src/main/**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/electron-best-practices.mdc)
Use Electron's built-in APIs for file system and native dialogs
Files:
src/main/presenter/devicePresenter/index.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)
**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪(如适用)、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别
Files:
src/main/presenter/devicePresenter/index.tssrc/shared/types/presenters/legacy.presenters.d.tssrc/renderer/src/composables/useDeviceVersion.ts
src/main/**/*.{ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
主进程代码放在
src/main
Files:
src/main/presenter/devicePresenter/index.ts
**/*.{ts,tsx,js,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Use English for all logs and comments
Files:
src/main/presenter/devicePresenter/index.tssrc/renderer/settings/App.vuesrc/shared/types/presenters/legacy.presenters.d.tssrc/renderer/shell/App.vuesrc/renderer/src/composables/useDeviceVersion.ts
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (CLAUDE.md)
Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)
Use PascalCase for TypeScript types and classes
Files:
src/main/presenter/devicePresenter/index.tssrc/renderer/settings/App.vuesrc/shared/types/presenters/legacy.presenters.d.tssrc/renderer/shell/App.vuesrc/renderer/src/composables/useDeviceVersion.ts
src/main/presenter/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Place Electron main-process presenters under src/main/presenter/ (Window, Tab, Thread, Mcp, Config, LLMProvider)
Files:
src/main/presenter/devicePresenter/index.ts
**/*.{ts,tsx,js,jsx,vue,css,scss,md,json,yml,yaml}
📄 CodeRabbit inference engine (AGENTS.md)
Prettier style: single quotes, no semicolons, print width 100; run pnpm run format
Files:
src/main/presenter/devicePresenter/index.tssrc/renderer/settings/App.vuesrc/shared/types/presenters/legacy.presenters.d.tssrc/renderer/shell/App.vuesrc/renderer/src/composables/useDeviceVersion.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,vue}: Use OxLint for JS/TS code; keep lint clean
Use camelCase for variables and functions
Use SCREAMING_SNAKE_CASE for constants
Files:
src/main/presenter/devicePresenter/index.tssrc/renderer/settings/App.vuesrc/shared/types/presenters/legacy.presenters.d.tssrc/renderer/shell/App.vuesrc/renderer/src/composables/useDeviceVersion.ts
src/renderer/**/*.{vue,ts,js,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
渲染进程代码放在
src/renderer
Files:
src/renderer/settings/App.vuesrc/renderer/shell/App.vuesrc/renderer/src/composables/useDeviceVersion.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/settings/App.vuesrc/renderer/shell/App.vuesrc/renderer/src/composables/useDeviceVersion.ts
src/renderer/**/*.{vue,ts}
📄 CodeRabbit inference engine (.cursor/rules/vue-shadcn.mdc)
Implement lazy loading for routes and components.
Files:
src/renderer/settings/App.vuesrc/renderer/shell/App.vuesrc/renderer/src/composables/useDeviceVersion.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.Use Pinia for frontend state management (do not introduce alternative state libraries)
Files:
src/renderer/settings/App.vuesrc/renderer/shell/App.vuesrc/renderer/src/composables/useDeviceVersion.ts
src/renderer/**/*.vue
📄 CodeRabbit inference engine (AGENTS.md)
Name Vue component files in PascalCase (e.g., ChatInput.vue)
Files:
src/renderer/settings/App.vuesrc/renderer/shell/App.vue
src/shared/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)
共享类型定义放在
shared目录
Files:
src/shared/types/presenters/legacy.presenters.d.ts
src/shared/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Place shared types, utilities, constants, and IPC contract definitions under src/shared/
Files:
src/shared/types/presenters/legacy.presenters.d.ts
src/shared/**
📄 CodeRabbit inference engine (AGENTS.md)
Put shared TypeScript types and utilities under src/shared
Files:
src/shared/types/presenters/legacy.presenters.d.ts
src/renderer/{src,shell,floating}/**/*.vue
📄 CodeRabbit inference engine (CLAUDE.md)
src/renderer/{src,shell,floating}/**/*.vue: Use Vue 3 Composition API for all components
All user-facing strings must use i18n keys via vue-i18n (no hard-coded UI strings)
Use Tailwind CSS utilities and ensure styles are scoped in Vue components
Files:
src/renderer/shell/App.vue
src/renderer/shell/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep shell UI code under src/renderer/shell
Files:
src/renderer/shell/App.vue
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/composables/useDeviceVersion.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/composables/useDeviceVersion.ts
src/renderer/src/**
📄 CodeRabbit inference engine (AGENTS.md)
Place Vue 3 app source under src/renderer/src (components, stores, views, i18n, lib)
Files:
src/renderer/src/composables/useDeviceVersion.ts
src/renderer/src/**/*.{vue,ts}
📄 CodeRabbit inference engine (AGENTS.md)
All user-facing strings must use vue-i18n ($t/keys) rather than hardcoded literals
Files:
src/renderer/src/composables/useDeviceVersion.ts
🧠 Learnings (3)
📚 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} : Use the Composition API for better code organization and reusability
Applied to files:
src/renderer/settings/App.vue
📚 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/**/*.{vue} : Use composition API and declarative programming patterns; avoid options API.
Applied to files:
src/renderer/settings/App.vue
📚 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/{composables,utils}/**/*.ts : Use VueUse for common composables and utility functions.
Applied to files:
src/renderer/settings/App.vuesrc/renderer/src/composables/useDeviceVersion.ts
🧬 Code graph analysis (2)
src/main/presenter/devicePresenter/index.ts (1)
build/generate-version-files.mjs (1)
os(61-61)
src/renderer/src/composables/useDeviceVersion.ts (1)
src/renderer/src/composables/usePresenter.ts (1)
usePresenter(103-105)
⏰ 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 (5)
src/shared/types/presenters/legacy.presenters.d.ts (1)
944-947: LGTM! Clean type addition for OS version metadata.The new
osVersionMetadatafield properly extendsDeviceInfowith platform-specific version information. The structure is well-defined withnameandbuildproperties.src/main/presenter/devicePresenter/index.ts (1)
28-59: LGTM! Well-structured OS version metadata implementation.The platform detection and metadata population logic is clean and correct. The build numbers for Windows (22000 for Win11, 10240 for Win10) and macOS (Darwin kernel versions) are accurate.
src/renderer/settings/App.vue (1)
64-76: Good refactor to composable-based platform detection.The migration to
useDeviceVersioncomposable successfully centralizes platform detection logic and eliminates duplicate code (once the redundantonMountedat lines 163-168 is removed).src/renderer/shell/App.vue (1)
12-15: LGTM! Clean composable integration.The migration to
useDeviceVersionis well-executed. The old platform detection logic has been properly removed and replaced with the composable.src/renderer/src/composables/useDeviceVersion.ts (1)
7-29: Excellent documentation and technical notes.The detailed comments explaining platform detection rules and version format specifics (Windows build numbers, macOS Darwin kernel versions) are very helpful for maintainability.
| onMounted(() => { | ||
| devicePresenter.getDeviceInfo().then((deviceInfo) => { | ||
| // Detect macOS (all versions) | ||
| const isMacOSPlatform = deviceInfo.platform === 'darwin' | ||
| isMacOS.value = isMacOSPlatform | ||
|
|
||
| // Check if it's Windows 11+ | ||
| // Note: Windows osVersion format is "10.0.22621", we need the 3rd part (build number) | ||
| let isWin11Plus = false | ||
| if (deviceInfo.platform === 'win32') { | ||
| const buildNumber = parseInt(deviceInfo.osVersion.split('.')[2] || '0', 10) | ||
| const win11Metadata = deviceInfo.osVersionMetadata.find((v) => v.name === 'Windows 11') | ||
| isWin11Plus = win11Metadata ? buildNumber >= win11Metadata.build : false | ||
| } | ||
|
|
||
| // isWinMacOS is true for: macOS (all versions) OR Windows 11+ | ||
| isWinMacOS.value = isMacOSPlatform || isWin11Plus | ||
| }) | ||
| }) |
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.
Add error handling for device info retrieval.
The getDeviceInfo() promise has no error handling. If the call fails, the platform flags will remain false, potentially causing incorrect UI rendering.
Apply this diff to add error handling:
onMounted(() => {
- devicePresenter.getDeviceInfo().then((deviceInfo) => {
+ devicePresenter.getDeviceInfo().then((deviceInfo) => {
// Detect macOS (all versions)
const isMacOSPlatform = deviceInfo.platform === 'darwin'
isMacOS.value = isMacOSPlatform
// Check if it's Windows 11+
// Note: Windows osVersion format is "10.0.22621", we need the 3rd part (build number)
let isWin11Plus = false
if (deviceInfo.platform === 'win32') {
const buildNumber = parseInt(deviceInfo.osVersion.split('.')[2] || '0', 10)
const win11Metadata = deviceInfo.osVersionMetadata.find((v) => v.name === 'Windows 11')
isWin11Plus = win11Metadata ? buildNumber >= win11Metadata.build : false
}
// isWinMacOS is true for: macOS (all versions) OR Windows 11+
isWinMacOS.value = isMacOSPlatform || isWin11Plus
+ }).catch((error) => {
+ console.error('Failed to load device info:', error)
+ // Fallback to safe defaults (already set to false)
})
})📝 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(() => { | |
| devicePresenter.getDeviceInfo().then((deviceInfo) => { | |
| // Detect macOS (all versions) | |
| const isMacOSPlatform = deviceInfo.platform === 'darwin' | |
| isMacOS.value = isMacOSPlatform | |
| // Check if it's Windows 11+ | |
| // Note: Windows osVersion format is "10.0.22621", we need the 3rd part (build number) | |
| let isWin11Plus = false | |
| if (deviceInfo.platform === 'win32') { | |
| const buildNumber = parseInt(deviceInfo.osVersion.split('.')[2] || '0', 10) | |
| const win11Metadata = deviceInfo.osVersionMetadata.find((v) => v.name === 'Windows 11') | |
| isWin11Plus = win11Metadata ? buildNumber >= win11Metadata.build : false | |
| } | |
| // isWinMacOS is true for: macOS (all versions) OR Windows 11+ | |
| isWinMacOS.value = isMacOSPlatform || isWin11Plus | |
| }) | |
| }) | |
| onMounted(() => { | |
| devicePresenter.getDeviceInfo().then((deviceInfo) => { | |
| // Detect macOS (all versions) | |
| const isMacOSPlatform = deviceInfo.platform === 'darwin' | |
| isMacOS.value = isMacOSPlatform | |
| // Check if it's Windows 11+ | |
| // Note: Windows osVersion format is "10.0.22621", we need the 3rd part (build number) | |
| let isWin11Plus = false | |
| if (deviceInfo.platform === 'win32') { | |
| const buildNumber = parseInt(deviceInfo.osVersion.split('.')[2] || '0', 10) | |
| const win11Metadata = deviceInfo.osVersionMetadata.find((v) => v.name === 'Windows 11') | |
| isWin11Plus = win11Metadata ? buildNumber >= win11Metadata.build : false | |
| } | |
| // isWinMacOS is true for: macOS (all versions) OR Windows 11+ | |
| isWinMacOS.value = isMacOSPlatform || isWin11Plus | |
| }).catch((error) => { | |
| console.error('Failed to load device info:', error) | |
| // Fallback to safe defaults (already set to false) | |
| }) | |
| }) |
🤖 Prompt for AI Agents
In src/renderer/src/composables/useDeviceVersion.ts around lines 37 to 55, the
call to devicePresenter.getDeviceInfo() lacks error handling; add a .catch
handler (or convert to async/await with try/catch) to handle rejected promises,
log the error (console.error or a logger), and explicitly set isMacOS.value and
isWinMacOS.value to safe defaults (false) so UI rendering won't rely on
stale/undefined values; ensure the error path still leaves the component in a
predictable state (optionally set an error flag or emit an event) and keep the
existing success logic unchanged.
|
@codex review |
|
fix #1044 |
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| onMounted(() => { | ||
| devicePresenter.getDeviceInfo().then((deviceInfo) => { | ||
| // Detect macOS (all versions) | ||
| const isMacOSPlatform = deviceInfo.platform === 'darwin' | ||
| isMacOS.value = isMacOSPlatform | ||
|
|
||
| // Check if it's Windows 11+ | ||
| // Note: Windows osVersion format is "10.0.22621", we need the 3rd part (build number) | ||
| let isWin11Plus = false | ||
| if (deviceInfo.platform === 'win32') { | ||
| const buildNumber = parseInt(deviceInfo.osVersion.split('.')[2] || '0', 10) | ||
| const win11Metadata = deviceInfo.osVersionMetadata.find((v) => v.name === 'Windows 11') | ||
| isWin11Plus = win11Metadata ? buildNumber >= win11Metadata.build : false |
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.
Guard against missing OS metadata in tests
The new useDeviceVersion composable assumes that devicePresenter.getDeviceInfo() always returns osVersion and osVersionMetadata. In the repo’s renderer test setup the mock still returns { platform, arch, version } only, so deviceInfo.osVersion.split('.') will throw as soon as any test mounts a component that uses this composable. Either provide defensive defaults here or update the shared test mock to include the new fields, otherwise the test suite will crash rather than exercising the component logic.
Useful? React with 👍 / 👎.
fix appbar transparent problem
#1044
Summary by CodeRabbit
Release Notes