-
Notifications
You must be signed in to change notification settings - Fork 424
fix: get close tab shortcut from key-binding-registry #4606
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
base: main
Are you sure you want to change the base?
Conversation
pengxingjian seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Walkthrough本次更改优化了编辑器标签页关闭按钮的提示信息,动态获取并显示“editor.close”命令的实际快捷键,而不再根据平台硬编码为“⌘W”或“Ctrl+W”。其余组件和服务逻辑保持不变。 Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EditorTabCloseComponent
participant KeybindingRegistry
User->>EditorTabCloseComponent: 鼠标悬停关闭按钮
EditorTabCloseComponent->>KeybindingRegistry: 查询"editor.close"快捷键
KeybindingRegistry-->>EditorTabCloseComponent: 返回快捷键信息
EditorTabCloseComponent-->>User: 显示包含实际快捷键的提示信息
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
yarn install v1.22.22 ✨ 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. 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
🧹 Nitpick comments (2)
packages/design/src/browser/override/editor-tab.service.tsx (2)
23-26
: 实现逻辑正确,但可以进行优化动态获取快捷键的实现逻辑正确,成功替代了硬编码的平台特定快捷键。不过有几个改进建议:
- 第25行可以使用可选链操作符来简化代码
- 逻辑清晰,正确处理了回退情况
应用以下优化:
- const keyBinds = keybindingRegistry.getKeybindingsForCommand('editor.close'); - const keyBinding = keyBinds && keyBinds[0]; + const keyBinding = keybindingRegistry.getKeybindingsForCommand('editor.close')?.[0];
27-27
: 考虑 useMemo 依赖数组
useMemo
的依赖数组为空,但计算依赖于shortCut
变量。如果运行时可能动态更新快捷键绑定,建议将shortCut
添加到依赖数组中。- const title = useMemo(() => formatLocalize('editor.closeTab.title', shortCut), []); + const title = useMemo(() => formatLocalize('editor.closeTab.title', shortCut), [shortCut]);如果快捷键绑定在运行时是静态的,当前实现也是可以接受的。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/design/src/browser/override/editor-tab.service.tsx
(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
packages/design/src/browser/override/editor-tab.service.tsx (1)
Learnt from: bytemain
PR: opensumi/core#4088
File: packages/ai-native/src/browser/widget/inline-stream-diff/live-preview.decoration.tsx:73-74
Timestamp: 2024-10-12T07:43:08.790Z
Learning: 在 `LivePreviewDiffDecorationModel` 类(位于 `packages/ai-native/src/browser/widget/inline-stream-diff/live-preview.decoration.tsx`)中,通过依赖注入(`@Autowired`)从 module 的 `providers` 声明中获取的依赖(如 `_onPartialEditEvent`),其销毁逻辑由 `ClientApp` 的生命周期管理,不需要手动调用 `dispose()` 进行销毁。
🪛 Biome (1.9.4)
packages/design/src/browser/override/editor-tab.service.tsx
[error] 25-25: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (1)
packages/design/src/browser/override/editor-tab.service.tsx (1)
4-4
: 导入变更正确正确添加了
KeybindingRegistry
导入,这是实现动态获取快捷键功能所必需的。
LGTM。 |
Types
Background or solution
fix #4604
Changelog
Summary by CodeRabbit