fix: add fix for setting page with shortcut #919

zerob13 · 2025-09-18T07:21:37Z

Summary by CodeRabbit

New Features
- Opening Settings now focuses an existing tab or creates one if needed, ensuring consistent behavior from the AppBar and keyboard shortcut across windows.
Bug Fixes
- Prevents duplicate Settings tabs and improves reliability when switching or restoring focus to Settings.
Refactor
- Centralized Settings navigation logic to a single main-process pathway and unified it behind a window-scoped action for consistency and maintainability.

coderabbitai · 2025-09-18T07:21:44Z

Walkthrough

Centralizes GO_SETTINGS handling in the main process. Shortcuts now send via eventBus to WindowPresenter, which opens or focuses a Settings tab in the target window. Renderer removes its GO_SETTINGS listener and delegates AppBar settings action to WindowPresenter. Types updated to expose the new method.

Changes

Cohort / File(s)	Summary
Shortcut routing (main) `src/main/presenter/shortcutPresenter.ts`	Route GO_SETTINGS via eventBus.sendToMain with (event, windowId), replacing per-tab dispatch; preserves focused-window gating.
Window presenter logic (main) `src/main/presenter/windowPresenter/index.ts`	Adds listener(s) for GO_SETTINGS (eventBus and ipcMain). Implements public openOrFocusSettingsTab(windowId): find/focus existing settings tab or create/activate new one; uses TabPresenter; adds error handling and window targeting fallback.
Renderer UI delegation `src/renderer/shell/components/AppBar.vue`, `src/renderer/src/App.vue`	AppBar openSettings now obtains windowId and calls windowPresenter.openOrFocusSettingsTab; removes renderer IPC GO_SETTINGS listener and handlers from App.vue.
Shared types `src/shared/types/presenters/legacy.presenters.d.ts`	Extends IWindowPresenter with openOrFocusSettingsTab(windowId: number): Promise.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User as User
  participant KS as ShortcutHandler
  participant EB as eventBus (renderer→main)
  participant WP as WindowPresenter (main)
  participant TP as TabPresenter
  participant W as BrowserWindow

  User->>KS: Press cmd+, (GO_SETTINGS)
  KS->>EB: sendToMain(GO_SETTINGS, focusedWindowId)
  EB->>WP: GO_SETTINGS(windowId)
  note right of WP: Determine target window: provided id → focused → main
  WP->>W: Resolve/ensure target window
  WP->>TP: getTabs(windowId)
  TP-->>WP: tabs[]
  alt Settings tab exists
    WP->>TP: activateTab(settingsTabId)
  else Create new
    WP->>TP: createTab({ url: "local://settings", activate: true })
  end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

A twitch of ears, a tap—cmd plus comma sings,
No tabs replaced, the Settings sprout their wings.
I hop through windows, neatly, one by one,
Find the gear, focus clear—then I’m done.
EventBus hums, the warren stays serene,
New paths, same burrow—swift and clean. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The PR description is only "fix #918" and does not follow the repository's required template; key sections are missing including a problem description, detailed solution, UI/UX changes, platform compatibility notes, and testing steps. As provided it lacks sufficient context for reviewers to assess scope, risk, and verification steps. This is insufficient per the project's PR template.	Resolution: Update the PR description to the repository template by describing the bug and reproduction steps, explaining the code changes and why they fix the issue, listing any UI/UX or platform compatibility notes, and adding testing steps or screenshots; link to issue #918 and mention the new openOrFocusSettingsTab API. After those details are added reviewers can more easily validate the change and its impact.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title "fix: add fix for setting page with shortcut" correctly signals the PR's intent to fix the settings-page behavior when triggered by the shortcut and aligns with the linked issue #918 and the changeset. It is slightly wordy and repetitive but remains related and understandable to reviewers. A more specific phrasing would improve clarity in changelogs and history.
Linked Issues Check	✅ Passed	The changes centralize GO_SETTINGS handling in the main process and add WindowPresenter.openOrFocusSettingsTab which searches for an existing settings tab or creates and activates a new one, directly addressing the bug where the settings shortcut replaced the conversation tab. The renderer now delegates the shortcut to the main process via eventBus and TabPresenter is used to avoid replacing conversation content, matching the expected behavior in issue #918. Based on the provided summaries, the implementation meets the primary coding requirements, though running the issue's reproduction steps across windows is recommended to confirm the fix in practice.
Out of Scope Changes Check	✅ Passed	All modified files (shortcutPresenter, windowPresenter, AppBar.vue, App.vue, and the presenter type definition) are directly related to rerouting the GO_SETTINGS action and implementing openOrFocusSettingsTab; I see no unrelated feature additions or extraneous refactors in the provided summaries. The changes appear focused on the linked issue's objective. Therefore the PR does not introduce out-of-scope changes based on the available summaries.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

📝 Generate Docstrings

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch bugfix/setting-view-shortcut

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d7ba68b and e6301b0.

📒 Files selected for processing (5)

src/main/presenter/shortcutPresenter.ts (1 hunks)
src/main/presenter/windowPresenter/index.ts (2 hunks)
src/renderer/shell/components/AppBar.vue (1 hunks)
src/renderer/src/App.vue (3 hunks)
src/shared/types/presenters/legacy.presenters.d.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (25)

**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/development-setup.mdc)

**/*.{js,jsx,ts,tsx}: 使用 OxLint 进行代码检查
Log和注释使用英文书写

**/*.{js,jsx,ts,tsx}: Use OxLint for JS/TS code; pre-commit hooks run lint-staged and typecheck
Use camelCase for variables and functions
Use PascalCase for types and classes
Use SCREAMING_SNAKE_CASE for constants

Files:

src/shared/types/presenters/legacy.presenters.d.ts
src/main/presenter/shortcutPresenter.ts
src/main/presenter/windowPresenter/index.ts

**/*.{ts,tsx}

📄 CodeRabbit inference engine (.cursor/rules/error-logging.mdc)

**/*.{ts,tsx}: 始终使用 try-catch 处理可能的错误
提供有意义的错误信息
记录详细的错误日志
优雅降级处理
日志应包含时间戳、日志级别、错误代码、错误描述、堆栈跟踪（如适用）、相关上下文信息
日志级别应包括 ERROR、WARN、INFO、DEBUG
不要吞掉错误
提供用户友好的错误信息
实现错误重试机制
避免记录敏感信息
使用结构化日志
设置适当的日志级别

Files:

src/shared/types/presenters/legacy.presenters.d.ts
src/main/presenter/shortcutPresenter.ts
src/main/presenter/windowPresenter/index.ts

src/shared/**/*.{ts,tsx,d.ts}

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

共享类型定义放在 shared 目录

Files:

src/shared/types/presenters/legacy.presenters.d.ts

**/*.{ts,tsx,js,vue}

📄 CodeRabbit inference engine (CLAUDE.md)

Use English for all logs and comments

Files:

src/shared/types/presenters/legacy.presenters.d.ts
src/main/presenter/shortcutPresenter.ts
src/renderer/src/App.vue
src/renderer/shell/components/AppBar.vue
src/main/presenter/windowPresenter/index.ts

**/*.{ts,tsx,vue}

📄 CodeRabbit inference engine (CLAUDE.md)

Enable and adhere to strict TypeScript typing (avoid implicit any, prefer precise types)

Files:

src/shared/types/presenters/legacy.presenters.d.ts
src/main/presenter/shortcutPresenter.ts
src/renderer/src/App.vue
src/renderer/shell/components/AppBar.vue
src/main/presenter/windowPresenter/index.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)

Store shared TypeScript types/utilities in src/shared/

Files:

src/shared/types/presenters/legacy.presenters.d.ts

**/*.{js,jsx,ts,tsx,vue}

📄 CodeRabbit inference engine (AGENTS.md)

Apply Prettier formatting: single quotes, no semicolons, max width 100

Files:

src/shared/types/presenters/legacy.presenters.d.ts
src/main/presenter/shortcutPresenter.ts
src/renderer/src/App.vue
src/renderer/shell/components/AppBar.vue
src/main/presenter/windowPresenter/index.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/shortcutPresenter.ts
src/main/presenter/windowPresenter/index.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/shortcutPresenter.ts
src/main/presenter/windowPresenter/index.ts

src/main/**/*.{ts,js,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

主进程代码放在 src/main

Files:

src/main/presenter/shortcutPresenter.ts
src/main/presenter/windowPresenter/index.ts

src/main/**

📄 CodeRabbit inference engine (AGENTS.md)

Place all Electron main-process code under src/main/

Files:

src/main/presenter/shortcutPresenter.ts
src/main/presenter/windowPresenter/index.ts

src/main/presenter/**

📄 CodeRabbit inference engine (AGENTS.md)

src/main/presenter/**: Organize main-process presenters under src/main/presenter/ (Window/Tab/Thread/Mcp/Config/LLMProvider)
Follow the Presenter pattern for main-process modules

Files:

src/main/presenter/shortcutPresenter.ts
src/main/presenter/windowPresenter/index.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/**/*.{vue,ts,js,tsx,jsx}

📄 CodeRabbit inference engine (.cursor/rules/project-structure.mdc)

渲染进程代码放在 src/renderer

Files:

src/renderer/src/App.vue
src/renderer/shell/components/AppBar.vue

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/**/*.vue

📄 CodeRabbit inference engine (.cursor/rules/vue-best-practices.mdc)

Use scoped styles to prevent CSS conflicts between components

Files:

src/renderer/src/App.vue

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/shell/components/AppBar.vue

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/shell/components/AppBar.vue

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/src/App.vue
src/renderer/shell/components/AppBar.vue

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/src/App.vue
src/renderer/shell/components/AppBar.vue

src/renderer/src/**

📄 CodeRabbit inference engine (AGENTS.md)

Put application code for the Vue app under src/renderer/src (components, stores, views, i18n, lib)

Files:

src/renderer/src/App.vue

src/renderer/src/**/*.{vue,ts}

📄 CodeRabbit inference engine (AGENTS.md)

All user-facing strings in the renderer must use vue-i18n keys defined in src/renderer/src/i18n

Files:

src/renderer/src/App.vue

src/renderer/**/*.vue

📄 CodeRabbit inference engine (AGENTS.md)

Name Vue components in PascalCase (e.g., ChatInput.vue)

Files:

src/renderer/src/App.vue
src/renderer/shell/components/AppBar.vue

src/renderer/shell/**

📄 CodeRabbit inference engine (AGENTS.md)

Keep shell UI code in src/renderer/shell/

Files:

src/renderer/shell/components/AppBar.vue

🧠 Learnings (4)

📚 Learning: 2025-07-21T01:45:54.229Z

Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/electron-best-practices.mdc:0-0
Timestamp: 2025-07-21T01:45:54.229Z
Learning: Applies to src/main/presenter/index.ts : The IPC messages from the main process to notify the view mainly rely on the EventBus index.ts to listen for events that need to be notified and then send them to the renderer through the mainWindow

Applied to files:

src/main/presenter/shortcutPresenter.ts
src/main/presenter/windowPresenter/index.ts

📚 Learning: 2025-09-16T07:06:30.939Z

Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: AGENTS.md:0-0
Timestamp: 2025-09-16T07:06:30.939Z
Learning: Applies to src/main/eventbus.ts : Use src/main/eventbus.ts as the central EventBus for app inter-process events

Applied to files:

src/main/presenter/shortcutPresenter.ts

📚 Learning: 2025-07-21T01:47:11.608Z

Learnt from: CR
PR: ThinkInAIXYZ/deepchat#0
File: .cursor/rules/project-structure.mdc:0-0
Timestamp: 2025-07-21T01:47:11.608Z
Learning: Applies to src/renderer/**/*.{vue,ts,js,tsx,jsx} : 渲染进程代码放在 `src/renderer`

Applied to files:

src/renderer/src/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/src/App.vue

🧬 Code graph analysis (2)

src/main/presenter/shortcutPresenter.ts (2)

src/main/eventbus.ts (1)

eventBus (151-151)

src/main/events.ts (1)

SHORTCUT_EVENTS (143-158)

src/main/presenter/windowPresenter/index.ts (4)

src/main/eventbus.ts (1)

eventBus (151-151)

src/main/events.ts (1)

SHORTCUT_EVENTS (143-158)

src/main/presenter/index.ts (1)

presenter (223-223)

src/main/presenter/tabPresenter.ts (1)

TabPresenter (15-989)

⏰ 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 (9)

src/shared/types/presenters/legacy.presenters.d.ts (1)

193-193: Good addition that aligns with the centralized Settings handling.

The new openOrFocusSettingsTab method properly complements the refactored GO_SETTINGS flow centralized in the main process. The signature is appropriate for cross-window navigation to Settings tabs.

src/renderer/src/App.vue (3)

167-167: Clean removal of renderer-side GO_SETTINGS handling.

The comment correctly indicates that GO_SETTINGS is now handled in the main process. This change aligns with the PR's objective to centralize Settings tab management and fix the tab replacement issue.

215-215: Consistent with the centralized approach.

The comment and removal of the GO_SETTINGS listener are consistent with the architectural change to handle Settings navigation in the main process rather than per-tab renderer.

301-301: Proper cleanup aligned with the removed functionality.

The removal of the GO_SETTINGS listener cleanup is appropriate since the listener registration was also removed.

src/main/presenter/shortcutPresenter.ts (1)

113-113: Correct centralization of GO_SETTINGS handling.

The change from presenter.windowPresenter.sendToActiveTab(focusedWindow.id, SHORTCUT_EVENTS.GO_SETTINGS) to eventBus.sendToMain(SHORTCUT_EVENTS.GO_SETTINGS, focusedWindow.id) properly centralizes the Settings navigation logic in the main process. This approach allows for better cross-window tab management and addresses the bug where Settings would replace conversation tabs.

src/main/presenter/windowPresenter/index.ts (3)

127-136: Well-implemented event listener for centralized GO_SETTINGS handling.

The event listener properly determines the target window using fallback logic (provided windowId → focused window → main window) and includes appropriate error handling. The implementation correctly delegates to the new openOrFocusSettingsTab method.

138-148: Good IPC handler for renderer-initiated Settings requests.

The IPC handler provides a bridge for renderer processes to request Settings navigation while maintaining the centralized approach. The window resolution logic and error handling are appropriate.

185-202: Well-designed Settings tab management method.

The openOrFocusSettingsTab method properly:

Validates the target window exists and is not destroyed

Searches for existing Settings tabs using appropriate URL patterns (local://settings or containing #/settings)

Switches to existing tab or creates a new one with correct activation

Handles the asynchronous nature of tab operations appropriately

The URL matching logic covers both direct settings URLs and hash-based routing patterns.

src/renderer/shell/components/AppBar.vue (1)

518-523: Simplified and improved Settings navigation.

The refactored openSettings function properly delegates to the centralized window presenter approach:

Uses window.api.getWindowId() to determine the current window

Calls the new windowPresenter.openOrFocusSettingsTab(windowId) method

Includes appropriate null check for windowId

This removes the complexity of manual tab searching and creation from the component, making it more maintainable while fixing the tab replacement issue described in the PR objectives.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

fix: add fix for setting page with shortcut

e6301b0

zerob13 merged commit 319ac6a into dev Sep 18, 2025
2 checks passed

zerob13 deleted the bugfix/setting-view-shortcut branch September 21, 2025 15:16

coderabbitai bot mentioned this pull request Oct 14, 2025

feat(window): add settings window #996

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: add fix for setting page with shortcut #919

fix: add fix for setting page with shortcut #919

Uh oh!

zerob13 commented Sep 18, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Sep 18, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix: add fix for setting page with shortcut #919

fix: add fix for setting page with shortcut #919

Uh oh!

Conversation

zerob13 commented Sep 18, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Sep 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Poem

Pre-merge checks and finishing touches

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zerob13 commented Sep 18, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Sep 18, 2025 •

edited

Loading