fix: quickopen auto focus previous active element #4272
Conversation
|
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
warning [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options. 变更概述演练此次变更主要涉及快速打开（Quick Open）服务中的焦点管理机制。在多个文件中，移除了关闭快速打开界面时自动聚焦编辑器的代码行。同时，在 变更
针对链接问题的评估
可能相关的 PR
建议评审人
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (5)
💤 Files with no reviewable changes (3)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (5)packages/quick-open/src/browser/prefix-quick-open.service.ts (5)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/quick-open/src/browser/quick-open.service.tsx (1)
Line range hint
159-169: 建议优化焦点恢复逻辑当前实现在取消操作时会恢复焦点，这是正确的。不过建议考虑以下优化：
- 可以抽取焦点管理逻辑到单独的方法
- 考虑添加焦点恢复失败的降级处理
建议重构为：
+ private restoreFocus(): void { + try { + if (this.previousActiveElement instanceof HTMLElement) { + this.previousActiveElement.focus(); + } + } catch (e) { + console.warn('Failed to restore focus:', e); + } finally { + this.previousActiveElement = undefined; + } + } public get widget(): QuickOpenWidget { // ... this._widget = this.injector.get(QuickOpenWidget, [ { onOk: () => { this.previousActiveElement = undefined; this.onClose(false); }, onCancel: () => { - if (this.previousActiveElement instanceof HTMLElement) { - this.previousActiveElement.focus(); - } - this.previousActiveElement = undefined; + this.restoreFocus(); this.onClose(true); }, // ... }, ]); // ... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/addons/src/browser/file-search.contribution.ts(0 hunks)packages/quick-open/src/browser/quick-open.command.service.ts(0 hunks)packages/quick-open/src/browser/quick-open.help.service.ts(0 hunks)packages/quick-open/src/browser/quick-open.service.tsx(2 hunks)
💤 Files with no reviewable changes (3)
- packages/quick-open/src/browser/quick-open.help.service.ts
- packages/quick-open/src/browser/quick-open.command.service.ts
- packages/addons/src/browser/file-search.contribution.ts
🔇 Additional comments (2)
packages/quick-open/src/browser/quick-open.service.tsx (2)
58-58: 类型定义更新合理！
将 previousActiveElement 的类型从 Element | undefined 扩展为 Element | undefined | null 是一个合理的改动，这样可以更准确地表达所有可能的状态。
106-106: 存储先前活动元素的时机正确！
在打开快速打开对话框时保存当前活动元素是解决焦点问题的关键。建议验证在不同场景下的焦点行为。
✅ Verification successful
焦点管理实现正确且符合代码库的最佳实践！
代码库中对 document.activeElement 的使用主要有以下几种场景：
- 在各种树形视图组件中检查焦点状态
- 在编辑器和调试相关组件中管理焦点
- 在核心浏览器服务中处理剪贴板和焦点追踪
当前实现在打开快速打开对话框时存储先前的活动元素是完全合理的，并且与代码库中其他地方的焦点管理模式保持一致。这种实现方式可以确保在对话框关闭后正确恢复焦点状态。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查其他地方是否有类似的焦点管理逻辑
rg "document\.activeElement" --type ts --type tsx
Length of output: 79
Script:
#!/bin/bash
# 检查其他地方是否有类似的焦点管理逻辑，修正文件类型
rg "document\.activeElement"
Length of output: 2621
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4272 +/- ##
=======================================
Coverage 54.27% 54.27%
=======================================
Files 1633 1633
Lines 99725 99723 -2
Branches 21633 21634 +1
=======================================
+ Hits 54122 54123 +1
+ Misses 37889 37886 -3
Partials 7714 7714
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
hacke2
force-pushed
the
fix/quick-auto-focus-pre-element
branch
from
53a0fcf to
34e72de
Compare
Types
Background or solution
fixed #4271
Changelog
fix: quickopen auto focus previous active element
Summary by CodeRabbit
新功能
Bug 修复