-
Notifications
You must be signed in to change notification settings - Fork 416
fix: not displayed when widget.disposed #4494
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
/next |
Walkthrough在 Changes
Sequence Diagram(s)sequenceDiagram
participant CT as CommentsThread
participant W as Widget
participant E as 编辑器
E->>CT: 请求显示 widget
CT->>W: 检查 widget.disposed 状态
alt widget 未处置
W->>W: 执行 show()
else widget 已处置
CT-->>E: 跳过显示
end
Possibly related PRs
Suggested labels
Suggested reviewers
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
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:
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 (1)
packages/comments/src/browser/comments-thread.ts (1)
256-258
: 正确地增加了 widget.disposed 检查,避免了对已销毁组件的操作!这个修改很好地解决了原有代码可能调用已销毁 widget 的 show 方法的问题。在调用 show 方法前检查 widget 是否已被销毁,可以有效避免潜在的错误。
不过,有一个小优化点:既然已经检查了
!widget.disposed
,那么后面使用widget?.show()
中的可选链操作符?.
就显得多余了,因为我们已经确定 widget 存在且未被销毁。可以直接使用widget.show()
。if (!widget.disposed) { - widget?.show(); + widget.show(); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/comments/src/browser/comments-thread.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: 🚀🚀🚀 Next Version for pull request
- GitHub Check: ubuntu-latest, Node.js 20.x
- GitHub Check: unittest (macos-latest, 18.x, node)
- GitHub Check: unittest (macos-latest, 18.x, jsdom)
- GitHub Check: unittest (ubuntu-latest, 18.x, jsdom)
- GitHub Check: unittest (ubuntu-latest, 18.x, node)
- GitHub Check: build (macos-latest, 20.x)
- GitHub Check: build (ubuntu-latest, 20.x)
- GitHub Check: build-windows
🎉 PR Next publish successful! 3.8.3-next-1742817599.0 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4494 +/- ##
==========================================
- Coverage 53.13% 53.13% -0.01%
==========================================
Files 1665 1665
Lines 102640 102641 +1
Branches 22215 22224 +9
==========================================
Hits 54535 54535
Misses 40019 40019
- Partials 8086 8087 +1
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:
|
@wangxiaojuan 这个问题有复现路径吗? |
Types
Background or solution
Changelog
fix: 修复widget为disposed后不展示
Summary by CodeRabbit