-
Notifications
You must be signed in to change notification settings - Fork 61
feat: expr #285
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
feat: expr #285
Conversation
Summary of ChangesHello @yvonneyx, 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! 本次拉取请求的核心目标是增强布局配置的灵活性和兼容性。通过引入表达式解析能力,解决了 Web Worker 环境下回调函数序列化受限的问题,并统一了节点尺寸和间距的处理逻辑,使得布局算法能够更强大、更易用。这些改进为用户提供了更精细的控制,并确保了不同运行环境下的稳定性。 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
这个 PR 引入了 @antv/expr 来支持将字符串表达式作为布局参数,解决了回调函数无法序列化导致 worker 失效的问题,这是一个很棒的改进。同时,将 nodeSize 和 nodeSpacing 等通用配置提升到 BaseLayoutOptions 中,使得 API 更加统一和清晰。整体代码风格良好,改动清晰。
555946d to
a5f5486
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v5 #285 +/- ##
==========================================
+ Coverage 85.44% 86.12% +0.68%
==========================================
Files 76 77 +1
Lines 5887 5938 +51
Branches 1411 1440 +29
==========================================
+ Hits 5030 5114 +84
+ Misses 856 823 -33
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| nodeStrength: 'node.data.ns', | ||
| edgeStrength: 'edge.data.es', | ||
| linkDistance: 'edge.data.len', | ||
| clusterNodeStrength: 'node.data.cs', |
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.
这个是 @antv/expr 支持的还是 layout 自己实现的
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.
antv/expr 支持
| test('should return node size plus spacing', () => { | ||
| const result = formatNodeSizeFn(20, 5, 10); | ||
| expect(result()).toBe(25); | ||
| expect(result()).toEqual([25, 25, 25]); |
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.
这三个值分别是啥意思
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.
【长、宽、高】
| * @param d - <zh/> 节点实例 | <en/> Node instance | ||
| */ | ||
| nodesepFunc?: (d?: NodeData) => number; | ||
| nodesepFunc?: Expr | ((node: NodeData) => number); |
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.
直接写出 string | ((node: NodeData) => number ?
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.
这里区分一下是 string 或者 type Expr=string
| * <en/> Minimum spacing between rings, used to adjust the radius | ||
| * @defaultValue 10 | ||
| */ | ||
| nodeSpacing?: number | ((d?: NodeData) => number); |
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.
这个参数删掉有影响吗
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.
提到通用配置里了
Aarebecca
left a comment
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.
文档里面可以要补充一下,一个是 Expr 的写法,还有就是可以访问的局部变量
11d117f to
cf9dda2
Compare
@antv/expr