-
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
| ๆไปถ่ทฏๅพ | ๅๆดๆ่ฆ |
|------------------------------------------------------------------|--------------------------------------------|
| packages/design/src/browser/override/editor-tab.service.tsx | ๅจๆ่ทๅโeditor.closeโๅฝไปค็ๅฟซๆท้ฎ๏ผๅนถๅจๅ
ณ้ญๆ ็ญพ้กตๆ้ฎ็ๆ็คบไธญๆพ็คบ๏ผๆฟๆขๅๆ็็กฌ็ผ็ ๆนๅผ๏ผ่ฐๆดๆ็คบๅผนๅบไฝ็ฝฎใ|
## Sequence Diagram(s)
```mermaid
sequenceDiagram
participant User
participant EditorTabCloseComponent
participant KeybindingRegistry
User->>EditorTabCloseComponent: ้ผ ๆ ๆฌๅๅ
ณ้ญๆ้ฎ
EditorTabCloseComponent->>KeybindingRegistry: ๆฅ่ฏข"editor.close"ๅฟซๆท้ฎ
KeybindingRegistry-->>EditorTabCloseComponent: ่ฟๅๅฟซๆท้ฎไฟกๆฏ
EditorTabCloseComponent-->>User: ๆพ็คบๅ
ๅซๅฎ้
ๅฟซๆท้ฎ็ๆ็คบไฟกๆฏ Estimated code review effort1 (~5 minutes)
|
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ใ |
done |
Codecov ReportAll modified and coverable lines are covered by tests โ
Additional details and impacted files@@ Coverage Diff @@
## main #4606 +/- ##
========================================
Coverage 52.61% 52.61%
========================================
Files 1687 1687
Lines 104341 104341
Branches 22668 22816 +148
========================================
Hits 54899 54899
Misses 41081 41081
Partials 8361 8361
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:
|
![]() |
Types
Background or solution
fix #4604
Changelog
Summary by CodeRabbit