-
Notifications
You must be signed in to change notification settings - Fork 19.6k
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(animation): support multi-level drill-down for universal transition #17611
Conversation
… only when direction is 'nodirection'
Thanks for your contribution! Document changes are required in this PR. Please also make a PR to apache/echarts-doc for document changes and update the issue id in the PR description. When the doc PR is merged, the maintainers will remove the |
I will add more test cases to testify if everything works fine:
|
@@ -644,6 +765,7 @@ export function installUniversalTransition(registers: EChartsExtensionInstallReg | |||
|
|||
// TODO multiple to multiple series. | |||
if (globalStore.oldSeries && params.updatedSeries && params.optionChanged) { | |||
// TODO transitionOpt was used in an old implementation and can be removed now |
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.
Hi, @pissang.
As you mentioned earlier that transitionSeriesFromOpt
is an old implementation but not removed from code, should I remove those code related?
// If specified key dimension(itemGroupId by default). Use this same dimension from other data. | ||
// PENDING: If only use key dimension of newData. | ||
const keyDim = isOld | ||
? (oldKeyDim || newKeyDim) | ||
: (newKeyDim || oldKeyDim); |
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.
This fault-tolerance code is simply dropped. Because the code for getting key from dimension(encode) is brought forward, where accessing both oldKeyDim and newKeyDim is not convinient.
In my opinion, the code should better be dropped. Users should get universalTransition working only if they satisfy all needs to trigger a universalTransition as the doc says. If they do not provide a perfect option but our code helps handling what they miss to make universalTransition succeed, they might be confused once they find "a bad option also triggers universalTransition", since they have no idea that echarts developers write fault-tolerance code to help with their bad options.
Add another test case that shows the number of git commits in two github organizations Org X and Org Y: 2022-10-17.15.08.34.movIn this test case, I put all data into raw dataItems directly, not by a dataset. And it also works fine. |
@tyn1998 Looks great! |
Looks great to me. Thanks @tyn1998 for your excellent contribution! |
Congratulations! Your PR has been merged. Thanks for your contribution! 👍 |
Brief Information
Multiple level drill down support for the universalTransition feature.
This pull request is in the type of:
What does this PR do?
In this PR, a new property
childGroupId
comes up and together withgroupId
, they can build multiple "parent-child" relationships between options. The old way of only usinggroupId
to implement a one-level drilldown and other animations still works. I have reviewed all test cases related to universalTransition and everything remains fine because the new API does not bring any breaking change.It is necessary to decide whether
groupId
orchildGroupId
is used as key in the keyGetter funtion to make sure that universalTransition animation does right, so I figured out a way to handle this, please see the block comments in code.A test html file is added to illustrate how to use
childGroupId
withgroupId
to implement the effect of multiple level down drill.Also fixed #17309 by the way, so #17559 can be closed if this PR is merged (I just cherry picked the commit in that PR to this branch).
Fixed issues
Details
Before: What was the problem?
Only 1 level drill down is allowed.
After: How does it behave after the fixing?
Multiple level drill down is allowed:
2022-09-22.21.53.47.mov
Document Info
One of the following should be checked.
docs: update documents for multiple level drilldown -
childGroupId
echarts-doc#295update bar-drilldown with multiple level drilldown echarts-examples#55
Misc
ZRender Changes
Related test cases or examples to use the new APIs
N.A.
Others
Merging options
Other information
This small feature is an OPSS2022 task, thank you guys from echarts core team to give me so much help when I was developing it!