-
Notifications
You must be signed in to change notification settings - Fork 601
feat(api): changeData & update #1834
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
Conversation
😭 Deploy PR Preview e23bac0 failed. Build logs 🤖 By surge-preview |
This pull request introduces 3 alerts when merging bfbb9ca into f2bef05 - view on LGTM.com new alerts:
|
Pull Request Test Coverage Report for Build 341531366
💛 - Coveralls |
@lxfu1 已经 Review 了一遍,撸了一遍,核心的改动应该有:
这个 PR 的 CR 问题(需要解决,具体都有 CR 回复):
后续可能需要做的:
|
This pull request introduces 3 alerts and fixes 1 when merging b7e84c4 into f2bef05 - view on LGTM.com new alerts:
fixed alerts:
|
This pull request introduces 2 alerts and fixes 1 when merging fa6ae31 into f2bef05 - view on LGTM.com new alerts:
fixed alerts:
|
This pull request introduces 2 alerts and fixes 1 when merging 1d0fead into f2bef05 - view on LGTM.com new alerts:
fixed alerts:
|
This pull request introduces 2 alerts and fixes 1 when merging ad13129 into 8d7b4a9 - view on LGTM.com new alerts:
fixed alerts:
|
@lxfu1
所以需要在 G2Plot 完善 changeData API,并做到文档透出。在开发过程中遇到 update 的另外的问题。分别描述
changeData API
目前的 changeData 仅仅是对 data key 生效,而对于 percent 类型,双轴图的双 data 类型,直方图等经过数据 transform 的均不生效。
解决方案:
实现使用 g2 的 changeData,结果 g2 的 update 能力,可以实现动画,但是需要解决不同图表的 transform 逻辑。目前来看开发成本,回滚成本大。
依然使用 更新整体配置的方式,只不过只更新 data/percent 等数据相关的 key 值。具体伪代码:
update API 问题
在实现 changeData 中,发现 update 存在的问题。目前实现代码:
存在的问题:
解决办法:
因为这个也不算 bug,毕竟 lodash 都是这么做的,所以名字上重命名为: deepAssign 以作区分,方式误导开发者。