-
Notifications
You must be signed in to change notification settings - Fork 387
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: when context key cause different menu bars change at same time. only one menu bar refreshed #4182
Conversation
…only one menu bar refreshed
Walkthrough本次变更对 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MenubarServiceImpl
participant Menu
User->>MenubarServiceImpl: 触发菜单更改
MenubarServiceImpl->>MenubarServiceImpl: 更新 menuIds
MenubarServiceImpl->>MenubarServiceImpl: 去抖动处理
MenubarServiceImpl->>Menu: 调用 _rebuildSingleRootMenus
Menu-->>MenubarServiceImpl: 返回更新后的菜单结构
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint (1.23.1)
warning [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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: 0
🧹 Outside diff range and nitpick comments (1)
packages/core-browser/src/menu/next/menubar-service.ts (1)
65-78
: 事件处理逻辑改进得当,建议添加错误处理实现方案很好地解决了多个菜单栏同步刷新的问题:
- 使用 debounce 合理地控制了更新频率
- 通过 Set 收集变更的菜单 ID
- 批量处理所有累积的变更
建议添加 try-catch 来增强代码的健壮性:
)(() => { const changedMenuIds = [...menuIds.values()]; menuIds.clear(); + try { changedMenuIds.forEach((menuId) => { this._rebuildSingleRootMenus(menuId); }); + } catch (error) { + console.error('处理菜单更新时发生错误:', error); + this._onDidMenuBarChange.fire(); // 发生错误时触发完整刷新 + } }, this),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/core-browser/src/menu/next/menubar-service.ts
(1 hunks)
🔇 Additional comments (1)
packages/core-browser/src/menu/next/menubar-service.ts (1)
62-62
: 使用 Set 来跟踪菜单 ID 是个不错的选择!
使用 Set
数据结构可以有效地确保菜单 ID 的唯一性,这是一个很好的实现方式。
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4182 +/- ##
==========================================
- Coverage 54.06% 54.06% -0.01%
==========================================
Files 1612 1612
Lines 97917 97925 +8
Branches 20051 20043 -8
==========================================
Hits 52941 52941
- Misses 37365 37373 +8
Partials 7611 7611
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
LGTM
Types
Background or solution
复现步骤:
1、定义一个context key.设置两个不同的menubar下的menu item的when为该context key。
2、更改该context key的值。
现象:只有一个menubar被刷新。
期待结果:两个menubar应该都被刷新。
Changelog
Summary by CodeRabbit