100pah commented on code in PR #17349:
URL: https://github.com/apache/echarts/pull/17349#discussion_r918582647
##########
src/chart/custom/CustomView.ts:
##########
@@ -1316,14 +1316,21 @@ function mergeChildren(
// might be better performance.
let index = 0;
for (; index < newLen; index++) {
- newChildren[index] && doCreateOrUpdateEl(
- api,
- el.childAt(index),
- dataIndex,
- newChildren[index] as CustomElementOption,
- seriesModel,
- el
- );
+ const newChild = newChildren[index];
+ if (newChild) {
+ doCreateOrUpdateEl(
+ api,
+ el.childAt(index),
+ dataIndex,
+ newChild as CustomElementOption,
+ seriesModel,
+ el
+ );
+ }
+ else {
+ // The element is being null after updating, remove the old element
+ el.remove(el.childAt(index));
Review Comment:
Basically I agree this change, which might make it more intuitive. BTW,
using "merge" by default rather than replace because the performance seams to
be better.
But this is a breaking change, we need to note it as "breaking change" in
release note.
And if modifying this behavior, the comment of this function are also needed
to be updated.
> // (1) By default, `elOption.$mergeChildren` is `'byIndex'`, which
indicates that
// the existing children will not be removed, and enables the feature
that
// update some of the props of some of the children simply by construct
// the returned children of `renderItem` like:
// `var children = group.children = []; children[3] = {opacity: 0.5};`
Probably need to explain the usage like: `null` means delete and empty
object `{}` means keep nothing change.
Do we need to add some doc to explain that in
<https://echarts.apache.org/en/option.html#series-custom.renderItem.return_group>?
This doc of `$mergeChildren` is missing because I'm not sure whether to
provide this option and whether to name it as `$mergeChildren` yet.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]