fix: re-compute node style in collapseNode#7607
Conversation
Summary of ChangesHello @binxie33, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求旨在解决 G6 库中 Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
你好,感谢你对 G6 的贡献。
这次的 Pull Request 修复了 collapseNode 函数中节点样式没有被正确重新计算的问题,确保了在使用 badges 等依赖于节点状态的样式时能够正确显示。
我在代码中发现了一个可以优化的地方:computeStyle 和 computeChangesAndDrawData 的调用顺序。目前的顺序虽然可以工作,但存在一定的效率问题。将 computeStyle 提前,可以使逻辑更清晰,并可能提高性能。我在具体的代码行上留下了详细的评论。
总体来说,这是一个有价值的修复。合并后将改善节点收起功能的用户体验。
| this.context.animation!.clear(); | ||
| this.computeStyle('collapse'); |
There was a problem hiding this comment.
你好,感谢你的修复。这个改动通过调用 computeStyle 解决了样式更新不及时的问题。
不过,当前的执行顺序似乎可以优化。computeChangesAndDrawData (在第 713 行) 在 computeStyle 之前被调用,这意味着 data 的计算是基于旧的样式。虽然 animation.clear() 清除了可能基于旧样式创建的动画,但整个流程是:计算 -> 清除 -> 重新计算,效率较低。
更理想的顺序是先调用 computeStyle('collapse'),再调用 computeChangesAndDrawData。这样可以确保数据变更的计算总是基于最新的样式状态,逻辑更清晰,也可能让 this.context.animation!.clear() 不再需要。
由于评论范围的限制,我无法提供一个直接修改顺序的代码建议,但这在未来的重构中是值得考虑的一个点。
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v5 #7607 +/- ##
=======================================
Coverage 94.95% 94.95%
=======================================
Files 188 188
Lines 10014 10016 +2
Branches 2171 2171
=======================================
+ Hits 9509 9511 +2
Misses 505 505
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
现在 collapseNode 函数没有正确的更新style,导致如果使用badges来跟踪展开 / 收起的子节点数时,收起之后无法正确显示。
修复代码是直接复制expandNode函数里的对应代码。
修复后效果如下:

在测试代码里:behaviorExpandCollapseNode 添加badges如下可复现: