Skip to content
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

[Bug] incorrect dataGroupId for old data items in universalTransition #17309

Closed
tyn1998 opened this issue Jul 4, 2022 · 3 comments · Fixed by #17611 or #17559
Closed

[Bug] incorrect dataGroupId for old data items in universalTransition #17309

tyn1998 opened this issue Jul 4, 2022 · 3 comments · Fixed by #17611 or #17559
Labels
animation bug en This issue is in English resolved

Comments

@tyn1998
Copy link
Contributor

tyn1998 commented Jul 4, 2022

Version

5.3.0

Link to Minimal Reproduction

https://codepen.io/tyn1998/pen/GRxpgQq

Steps to Reproduce

  1. visit my codepen url above
  2. click "Animals" bar (universalTransition animation works)
  3. click "Back" (not works)
  4. comment line 12 dataGroupId: "", and repeat 2 and 3 they both will work

Current Behavior

Hi, Echarts core team!

I have read the related source code so I know where the problem occurs.

const dataGroupId = data.hostModel && (data.hostModel as SeriesModel).get('dataGroupId') as string;

When processing old data items, after executing this line, dataGroupId is wrongly assigned to a value that is belong to new data items, which is due to the factor of having notMerge default set to false when .setOption(newOption).

This bug will not be exposed when data item's groupId is specified.

Expected Behavior

Old data items should be given correct dataGroupId when item.groupId is not specified.

Environment

- OS: macOS Monterey
- Browser: Chrome 103
- Framework: None

Any additional comments?

N.A.

@tyn1998 tyn1998 added the bug label Jul 4, 2022
@echarts-bot echarts-bot bot added en This issue is in English pending We are not sure about whether this is a bug/new feature. labels Jul 4, 2022
@tyn1998
Copy link
Contributor Author

tyn1998 commented Sep 1, 2022

which is due to the factor of having notMerge default set to false when .setOption(newOption).

Can be supported by another case:

buttons: [{
text: 'Scatter',
onclick: function () {
chart.setOption(scatterOption, true);
}
}, {
text: 'Bar',
onclick: function () {
chart.setOption(barOption, true);
}
}],

where notMerge set to true so we can get correct dataGroupId for old data item.

@tyn1998
Copy link
Contributor Author

tyn1998 commented Sep 2, 2022

Complement:

if Echarts cannot find dataGroupId in series, it will try to find it in parent model:

echarts/src/model/Model.ts

Lines 234 to 239 in 7f3b2ba

if (obj == null && parentModel) {
obj = parentModel._doGet(
this.resolveParentPath(pathArr) as [string],
parentModel.parentModel
) as any;
}

@tyn1998
Copy link
Contributor Author

tyn1998 commented Oct 17, 2022

closed via #17559

@tyn1998 tyn1998 closed this as completed Oct 17, 2022
@plainheart plainheart added animation resolved and removed pending We are not sure about whether this is a bug/new feature. labels Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
animation bug en This issue is in English resolved
Projects
None yet
2 participants