🐛 fix: enhance the multi-display window opening experience #8176
Conversation
|
@Wxh16144 is attempting to deploy a commit to the LobeHub Community Team on Vercel. A member of the Team first needs to authorize it. |
Reviewer's GuideThis PR introduces a parentIdentifier property and centers newly opened windows on the same display as their parent, ensuring sub-windows (like settings or devtools) appear on the correct monitor when first shown. Sequence diagram for opening a new windowsequenceDiagram
participant User as actor
participant ChatWindow as "Chat Window"
participant SettingsBrowser as "Settings Browser"
participant BrowserManager
participant ScreenAPI as "Electron Screen API"
User->>ChatWindow: Clicks 'Open Settings'
ChatWindow->>SettingsBrowser: show()
activate SettingsBrowser
SettingsBrowser->>SettingsBrowser: determineWindowPosition()
SettingsBrowser->>BrowserManager: retrieveByIdentifier("chat")
BrowserManager-->>SettingsBrowser: returns parent window instance
SettingsBrowser->>ScreenAPI: getDisplayNearestPoint(parent window bounds)
ScreenAPI-->>SettingsBrowser: returns correct display
SettingsBrowser->>SettingsBrowser: setPosition(newX, newY)
SettingsBrowser->>SettingsBrowser: browserWindow.show()
deactivate SettingsBrowser
Updated class diagram for Browser and BrowserWindowOptsclassDiagram
class BrowserWindowOpts {
+identifier: string
+keepAlive: boolean
+parentIdentifier: string
+path: string
+showOnInit: boolean
+title: string
}
class Browser {
-options: BrowserWindowOpts
+show()
-determineWindowPosition()
}
Browser "1" -- "1" BrowserWindowOpts : uses
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Thank you for raising your pull request and contributing to our Community |
There was a problem hiding this comment.
Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.
There was a problem hiding this comment.
Hey @Wxh16144 - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `apps/desktop/src/main/core/Browser.ts:163` </location>
<code_context>
+ const { parentIdentifier } = this.options;
+
+ if (parentIdentifier) {
+ // todo: fix ts type
+ const parentWin = this.app.browserManager.retrieveByIdentifier(parentIdentifier as any);
+ if (parentWin) {
</code_context>
<issue_to_address>
Avoid casting to any by updating retrieveByIdentifier typing
Consider updating retrieveByIdentifier or BrowserManager to accept a string and return a typed Browser instance, rather than casting parentIdentifier to any.
Suggested implementation:
```typescript
if (parentIdentifier) {
const parentWin = this.app.browserManager.retrieveByIdentifier(parentIdentifier);
if (parentWin) {
```
You must also update the `retrieveByIdentifier` method in the `BrowserManager` class (likely in `apps/desktop/src/main/core/BrowserManager.ts` or similar) to have a signature like:
```ts
retrieveByIdentifier(identifier: string): Browser | undefined
```
and ensure it returns a `Browser` instance or `undefined` as appropriate. This will allow you to remove the `as any` cast and have proper type safety.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const { parentIdentifier } = this.options; | ||
|
|
||
| if (parentIdentifier) { | ||
| // todo: fix ts type |
There was a problem hiding this comment.
suggestion: Avoid casting to any by updating retrieveByIdentifier typing
Consider updating retrieveByIdentifier or BrowserManager to accept a string and return a typed Browser instance, rather than casting parentIdentifier to any.
Suggested implementation:
if (parentIdentifier) {
const parentWin = this.app.browserManager.retrieveByIdentifier(parentIdentifier);
if (parentWin) {You must also update the retrieveByIdentifier method in the BrowserManager class (likely in apps/desktop/src/main/core/BrowserManager.ts or similar) to have a signature like:
retrieveByIdentifier(identifier: string): Browser | undefinedand ensure it returns a Browser instance or undefined as appropriate. This will allow you to remove the as any cast and have proper type safety.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8176 +/- ##
=========================================
Coverage 87.84% 87.84%
=========================================
Files 836 836
Lines 62148 62148
Branches 5663 3932 -1731
=========================================
Hits 54596 54596
Misses 7552 7552
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
❤️ Great PR @Wxh16144 ❤️ The growth of project is inseparable from user feedback and contribution, thanks for your contribution! If you are interesting with the lobehub developer community, please join our discord and then dm @arvinxx or @canisminor1990. They will invite you to our private developer channel. We are talking about the lobe-chat development or sharing ai newsletter around the world. |
|
⏳ Processing in progress |
### [Version 1.94.11](v1.94.10...v1.94.11) <sup>Released on **2025-06-17**</sup> #### 🐛 Bug Fixes - **misc**: Enhance the multi-display window opening experience. <br/> <details> <summary><kbd>Improvements and Fixes</kbd></summary> #### What's fixed * **misc**: Enhance the multi-display window opening experience, closes [#8176](#8176) ([b132e66](b132e66)) </details> <div align="right"> [](#readme-top) </div>
|
🎉 This PR is included in version 1.94.11 🎉 The release is available on: Your semantic-release bot 📦🚀 |
### [Version 1.94.4](v1.94.3...v1.94.4) <sup>Released on **2025-06-18**</sup> #### 🐛 Bug Fixes - **misc**: Enhance the multi-display window opening experience. <br/> <details> <summary><kbd>Improvements and Fixes</kbd></summary> #### What's fixed * **misc**: Enhance the multi-display window opening experience, closes [lobehub#8176](https://github.com/jaworldwideorg/OneJA-Bot/issues/8176) ([b132e66](b132e66)) </details> <div align="right"> [](#readme-top) </div>
### [Version 1.94.11](lobehub/lobe-chat@v1.94.10...v1.94.11) <sup>Released on **2025-06-17**</sup> #### 🐛 Bug Fixes - **misc**: Enhance the multi-display window opening experience. <br/> <details> <summary><kbd>Improvements and Fixes</kbd></summary> #### What's fixed * **misc**: Enhance the multi-display window opening experience, closes [lobehub#8176](lobehub#8176) ([b132e66](lobehub@b132e66)) </details> <div align="right"> [](#readme-top) </div>
### [Version 1.94.11](lobehub/lobe-chat@v1.94.10...v1.94.11) <sup>Released on **2025-06-17**</sup> #### 🐛 Bug Fixes - **misc**: Enhance the multi-display window opening experience. <br/> <details> <summary><kbd>Improvements and Fixes</kbd></summary> #### What's fixed * **misc**: Enhance the multi-display window opening experience, closes [lobehub#8176](lobehub#8176) ([7a638f7](lobehub@7a638f7)) </details> <div align="right"> [](#readme-top) </div>
💻 变更类型 | Change Type
🔀 变更说明 | Description of Change
有多个显示器屏幕下，应用程序首次启动后，将应用拖到副屏后，首次打开 setting 窗口位置会重新从主屏幕显示。这个 PR 主要解决这个体验问题。
📝 补充信息 | Additional Information
Summary by Sourcery
Improve multi-display window opening experience by centering child windows (e.g., settings, devtools) on the same monitor as their parent.
Enhancements: