-
Notifications
You must be signed in to change notification settings - Fork 602
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: [V2_combo] 混合图表-双折线图 #1331
Conversation
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.
Could you please add tests to make sure this change works as expected?
因本地 lint 问题(commit 前未执行钩子),更新了一部分格式错误,包括删除了部分无用代码,因此 Me-Momo 的部分 review 已过时,为保持整洁删除了一些过时 review(后来才发现还可以不删除直接隐藏)感谢 Me-Momo 的 review |
|
||
biax.render(); | ||
|
||
// TODO 补充用例 |
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.
好的,测试我再补充一下
xAxis: false, | ||
geometryConfigs: [ | ||
{ | ||
geometry: 'Line', |
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.
这个中 type 等产量,全部用小写 + 中划线的方式吧~
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.
type 是指?
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.
type 比如:图形类别,动画类型,布局算法类型等
// expect(pointGeometrys[0].attributes.size.values).toEqual([3]); | ||
// expect(pointGeometrys[0].attributes.shape.values).toEqual(['circle']); | ||
|
||
// Biax.destroy(); |
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.
单测打开~加上断言。
src/adaptor/geometries/line.ts
Outdated
@@ -3,25 +3,26 @@ import { Params } from '../../core/adaptor'; | |||
import { Options } from '../../types'; | |||
import { ShapeStyle } from '../../types/style'; | |||
|
|||
type lineOption = { |
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.
line -> Line
src/common/helper.ts
Outdated
|
||
/** | ||
* 在 Chart 中查找第一个指定 type 类型的 geometry | ||
* @param chart | ||
* @param type | ||
*/ | ||
export function findGeometry(chart: Chart, type: string): Geometry { | ||
export function findGeometry(chart: Chart | View, type: string): Geometry { |
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.
应该直接指定为 View 就可以了。传入 Chart 也是属于 View 的
Right = 'Right', | ||
} | ||
|
||
export enum BiaxGeometry { |
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.
Biax 表示双轴图是行业术语嘛?
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.
Biax 是双轴的意思 (bi axis)我看竞品都没有双轴图表这个概念,而是称之为带有双轴的折线图,因此新取了一个图表名。这个也欢迎给点意见,看看有没有更好的名称。或者 DualAxis 也可以? combo (混合图表)?
目前来看,双轴图应该是最复杂的图形了,保证质量,好好设计,不用为了时间点~ |
geometryConfig: geometryConfigs[1], | ||
}, | ||
}); | ||
return params; |
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.
既然是双图形了,建议还是两个 view 吧,然后分别设置数据和配置。
现在 chart 创建 view 的时候,会把 chart 自己的属性也带给 view,比如 data、tooltip 等配置,如果左边用 chart,右边用 view,感觉没有隔离开,后面可能会遇到一些 bug。
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.
好,我瞅下这块儿
分支 hold 太久,先合并一个版本,后续几个点:
|
* feat: add baseCombo & dualline * fix: 部分lint 错误 * fix: npm run fix * fix: 删除无用变量 * feat: push geogemtry 拆分方式 * feat: lint * fix: lint * fix: github test 失败 * feat: 双轴图打平 * feat: line * feat: 更改双轴图引用 * feat: test 用例 * fix: lint * feat: fix lint * fix: fix 不必要的提交 * fix: merge * fix: typescript * feat: 更改 cr 问题 Co-authored-by: aiyin.lzy <nadia.lzy@antfin.com>
* feat: add baseCombo & dualline * fix: 部分lint 错误 * fix: npm run fix * fix: 删除无用变量 * feat: push geogemtry 拆分方式 * feat: lint * fix: lint * fix: github test 失败 * feat: 双轴图打平 * feat: line * feat: 更改双轴图引用 * feat: test 用例 * fix: lint * feat: fix lint * fix: fix 不必要的提交 * fix: merge * fix: typescript * feat: 更改 cr 问题 Co-authored-by: aiyin.lzy <nadia.lzy@antfin.com>
更新:
混合双折线图:
API 所做更改:
可能有问题的地方