You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@echarts.apache.org by su...@apache.org on 2020/06/08 14:23:00 UTC
[incubator-echarts] 03/03: fix: fix custom series merge children
strategy (force no empty children even thought merging children).
This is an automated email from the ASF dual-hosted git repository.
sushuang pushed a commit to branch custom-series-enhance
in repository https://gitbox.apache.org/repos/asf/incubator-echarts.git
commit 3a6ffe9f5f9c390e3c28e2e819f085c5f14981a0
Author: 100pah <su...@gmail.com>
AuthorDate: Mon Jun 8 22:15:55 2020 +0800
fix: fix custom series merge children strategy (force no empty children even thought merging children).
---
src/chart/custom.ts | 82 +++++++++++-----------
src/util/graphic.ts | 2 +-
test/custom-transition.html | 164 ++++++++++++++++++++++++++++++++++++++++----
3 files changed, 196 insertions(+), 52 deletions(-)
diff --git a/src/chart/custom.ts b/src/chart/custom.ts
index 9125b97..9485c0e 100644
--- a/src/chart/custom.ts
+++ b/src/chart/custom.ts
@@ -432,18 +432,18 @@ class CustomSeriesView extends ChartView {
// roam or data zoom according to `actionType`.
data.diff(oldData)
.add(function (newIdx) {
- createOrUpdateItemEl(
+ createOrUpdateItem(
null, newIdx, renderItem(newIdx, payload), customSeries, group, data
);
})
.update(function (newIdx, oldIdx) {
- createOrUpdateItemEl(
+ createOrUpdateItem(
oldData.getItemGraphicEl(oldIdx),
newIdx, renderItem(newIdx, payload), customSeries, group, data
);
})
.remove(function (oldIdx) {
- removeItemEl(oldData.getItemGraphicEl(oldIdx), customSeries, group);
+ doRemoveEl(oldData.getItemGraphicEl(oldIdx), customSeries, group);
})
.execute();
@@ -486,7 +486,7 @@ class CustomSeriesView extends ChartView {
}
}
for (let idx = params.start; idx < params.end; idx++) {
- const el = createOrUpdateItemEl(null, idx, renderItem(idx, payload), customSeries, this.group, data);
+ const el = createOrUpdateItem(null, idx, renderItem(idx, payload), customSeries, this.group, data);
el.traverse(setIncrementalAndHoverLayer);
}
}
@@ -590,6 +590,13 @@ function createEl(elOption: CustomElementOption): Element {
* Some "object-like" config like `textConfig`, `textContent`, `style` which are not needed for
* every elment, so we replace them only when user specify them. And the that is a total replace.
*
+ * TODO: there is no hint of 'isFirst' to users. So the performance enhancement can not be
+ * performed yet. Consider the case:
+ * (1) setOption to "mergeChildren" with a smaller children count
+ * (2) Use dataZoom to make an item disappear.
+ * (3) User dataZoom to make the item display again. At that time, renderItem need to return the
+ * full option rather than partial option to recreate the element.
+ *
* ----------------------------------------------
* [STRATEGY_NULL] `hasOwnProperty` or `== null`:
*
@@ -1348,6 +1355,14 @@ function makeRenderItem(
return itemStyle;
}
+ function applyExtraAfter(itemStyle: ZRStyleProps, extra: ZRStyleProps): void {
+ for (const key in extra) {
+ if (hasOwn(extra, key)) {
+ (itemStyle as any)[key] = (extra as any)[key];
+ }
+ }
+ }
+
function preFetchFromExtra(extra: ZRStyleProps, itemStyle: ItemStyleProps): void {
// A trick to retrieve those props firstly, which are used to
// apply auto inside fill/stroke in `convertToEC4StyleForCustomSerise`.
@@ -1425,7 +1440,7 @@ function wrapEncodeDef(data: List<CustomSeriesModel>): Dictionary<number[]> {
return encodeDef;
}
-function createOrUpdateItemEl(
+function createOrUpdateItem(
el: Element,
dataIndex: number,
elOption: CustomElementOption,
@@ -1433,13 +1448,25 @@ function createOrUpdateItemEl(
group: ViewRootGroup,
data: List<CustomSeriesModel>
): Element {
- el = doCreateOrUpdate(el, dataIndex, elOption, seriesModel, group, true);
+ // [Rule]
+ // If `renderItem` returns `null`/`undefined`/`false`, remove the previous el if existing.
+ // (It seems that violate the "merge" principle, but most of users probably intuitively
+ // regard "return;" as "show nothing element whatever", so make a exception to meet the
+ // most cases.)
+ // The rule or "merge" see [STRATEGY_MERGE].
+
+ // If `elOption` is `null`/`undefined`/`false` (when `renderItem` returns nothing).
+ if (!elOption) {
+ el && group.remove(el);
+ return;
+ }
+ el = doCreateOrUpdateEl(el, dataIndex, elOption, seriesModel, group, true);
el && data.setItemGraphicEl(dataIndex, el);
return el;
}
-function doCreateOrUpdate(
+function doCreateOrUpdateEl(
el: Element,
dataIndex: number,
elOption: CustomElementOption,
@@ -1448,20 +1475,10 @@ function doCreateOrUpdate(
isRoot: boolean
): Element {
- // [Rule]
- // If `renderItem` returns `null`/`undefined`/`false`, remove the previous el if existing.
- // (It seems that violate the "merge" principle, but most of users probably intuitively
- // regard "return;" as "show nothing element whatever", so make a exception to meet the
- // most cases.)
- // The rule or "merge" see [STRATEGY_MERGE].
-
- // If `elOption` is `null`/`undefined`/`false` (when `renderItem` returns nothing).
- if (!elOption) {
- el && group.remove(el);
- return;
+ if (__DEV__) {
+ assert(elOption, 'should not have an null/undefined element setting');
}
- elOption = elOption || {} as CustomElementOption;
let toBeReplacedIdx = -1;
if (el && doesElNeedRecreate(el, elOption)) {
@@ -1525,7 +1542,6 @@ function doesElNeedRecreate(el: Element, elOption: CustomElementOption): boolean
const elOptionShape = (elOption as CustomZRPathOption).shape;
const elOptionStyle = elOption.style;
return (
- // || elOption.$merge === false
// If `elOptionType` is `null`, follow the merge principle.
(elOptionType != null
&& elOptionType !== elInner.customGraphicType
@@ -1729,8 +1745,7 @@ function retrieveStyleOptionOnState(
//
// For implementation simpleness, do not provide a direct way to remove sinlge
// child (otherwise the total indicies of the children array have to be modified).
-// User can remove a single child by set its `ignore` as `true` or replace
-// it by another element, where its `$merge` can be set as `true` if necessary.
+// User can remove a single child by set its `ignore` as `true`.
function mergeChildren(
el: graphicUtil.Group,
dataIndex: number,
@@ -1767,7 +1782,7 @@ function mergeChildren(
// might be better performance.
let index = 0;
for (; index < newLen; index++) {
- newChildren[index] && doCreateOrUpdate(
+ newChildren[index] && doCreateOrUpdateEl(
el.childAt(index),
dataIndex,
newChildren[index],
@@ -1776,11 +1791,8 @@ function mergeChildren(
false
);
}
- if (__DEV__) {
- assert(
- !notMerge || el.childCount() === index,
- 'MUST NOT contain empty item in children array when `group.$mergeChildren` is `false`.'
- );
+ for (let i = el.childCount() - 1; i >= index; i--) {
+ doRemoveEl(el.childAt(i), seriesModel, el);
}
}
@@ -1819,7 +1831,7 @@ function processAddUpdate(
const childOption = newIndex != null ? context.newChildren[newIndex] : null;
const child = oldIndex != null ? context.oldChildren[oldIndex] : null;
- doCreateOrUpdate(
+ doCreateOrUpdateEl(
child,
context.dataIndex,
childOption,
@@ -1829,21 +1841,13 @@ function processAddUpdate(
);
}
-function applyExtraAfter(itemStyle: ZRStyleProps, extra: ZRStyleProps): void {
- for (const key in extra) {
- if (hasOwn(extra, key)) {
- (itemStyle as any)[key] = (extra as any)[key];
- }
- }
-}
-
function processRemove(this: DataDiffer<DiffGroupContext>, oldIndex: number): void {
const context = this.context;
const child = context.oldChildren[oldIndex];
- child && context.group.remove(child);
+ doRemoveEl(child, context.seriesModel, context.group);
}
-function removeItemEl(
+function doRemoveEl(
el: Element,
seriesModel: CustomSeriesModel,
group: ViewRootGroup
diff --git a/src/util/graphic.ts b/src/util/graphic.ts
index 21d6aa2..c97b438 100644
--- a/src/util/graphic.ts
+++ b/src/util/graphic.ts
@@ -1117,7 +1117,7 @@ function animateOrSetProps<Props>(
}
else {
el.stopAnimation();
- el.attr(props);
+ !isFrom && el.attr(props);
cb && cb();
}
}
diff --git a/test/custom-transition.html b/test/custom-transition.html
index a51d31d..cd9c865 100644
--- a/test/custom-transition.html
+++ b/test/custom-transition.html
@@ -37,9 +37,11 @@ under the License.
<!-- <div id="texture-bar-texture-maker"></div> -->
+
<div id="spiral-fixed-extent"></div>
<div id="spiral-dynamic-extent"></div>
<div id="texture-bar-by-clipPath"></div>
+ <div id="no-animation"></div>
<div id="enter-animation-and-merge"></div>
<div id="enter-animation2"></div>
<div id="enter-animation-clipPath"></div>
@@ -51,7 +53,6 @@ under the License.
-
<!--
<script>
require(['echarts'], function (echarts) {
@@ -799,6 +800,133 @@ under the License.
+
+
+
+
+
+ <script>
+ require(['echarts'], function (echarts) {
+
+ var option = {
+ animation: false,
+ xAxis: {
+ max: 600,
+ },
+ yAxis: {
+ },
+ dataZoom: [
+ { type: 'slider', start: 10, end: 60 },
+ { type: 'inside', start: 10, end: 60 }
+ ],
+ series: [{
+ type: 'custom',
+ renderItem: function (params, api) {
+ var pos = api.coord([api.value(0), api.value(1)]);
+ return {
+ type: 'group',
+ x: pos[0],
+ y: pos[1],
+ children: [{
+ type: 'rect',
+ shape: {
+ x: -50,
+ y: 50,
+ width: 100,
+ height: 50,
+ r: 10
+ },
+ style: {
+ fill: 'blue',
+ }
+ }, {
+ type: 'circle',
+ shape: {
+ cx: -50,
+ cy: 50,
+ r: 30
+ },
+ style: {
+ fill: 'green',
+ },
+ textConfig: {
+ position: 'bottom'
+ },
+ textContent: {
+ style: {
+ text: 'xxxx',
+ fill: 'black',
+ }
+ }
+ }]
+ };
+ },
+ data: [[221, 333], [129, 312]]
+ }]
+ };
+
+ var chart = testHelper.create(echarts, 'no-animation', {
+ title: [
+ 'No-animation',
+ '(1) Move dataZoom, position should have no transition animation but move normally.',
+ '(2) Use dataZoom hide a data item, and then show it.',
+ '(3) click button to setOption merge: ',
+ ' circle **disappears** and rect become **red border black bg**',
+ '(4) **Repeat (2)** after merged, should be correct.'
+ ],
+ height: 300,
+ option: option,
+ buttons: [{
+ text: 'go',
+ onclick: function () {
+ chart.dispatchAction({type: 'dataZoom', start: 10, end: 60});
+ }
+ }, {
+ text: 'click me to merge children',
+ onclick: function () {
+ chart.setOption({
+ series: {
+ type: 'custom',
+ renderItem: function (params, api) {
+ var pos = api.coord([api.value(0), api.value(1)]);
+ return {
+ type: 'group',
+ x: pos[0],
+ y: pos[1],
+ children: [{
+ type: 'rect',
+ shape: {
+ x: -50,
+ y: 50,
+ width: 100,
+ height: 50,
+ r: 10
+ },
+ style: {
+ stroke: 'red',
+ lineWidth: 5
+ }
+ }]
+ };
+ }
+ }
+ });
+ }
+ }]
+ });
+ });
+ </script>
+
+
+
+
+
+
+
+
+
+
+
<script>
require(['echarts'], function (echarts) {
@@ -808,12 +936,13 @@ under the License.
animationDuration: animationDuration,
animationDurationUpdate: animationDurationUpdate,
xAxis: {
+ max: 600
},
yAxis: {
},
dataZoom: [
- { type: 'slider' },
- { type: 'inside' }
+ { type: 'slider', start: 10, end: 60 },
+ { type: 'inside', start: 10, end: 60 }
],
series: [{
type: 'custom',
@@ -834,7 +963,7 @@ under the License.
},
style: {
fill: 'blue',
- $enterFrom: { opacity: 0 }
+ // $enterFrom: { opacity: 0 }
}
}, {
type: 'circle',
@@ -845,7 +974,7 @@ under the License.
},
style: {
fill: 'green',
- $enterFrom: { opacity: 0 }
+ // $enterFrom: { opacity: 0 }
},
textConfig: {
position: 'bottom'
@@ -854,44 +983,55 @@ under the License.
style: {
text: 'xxxx',
fill: 'black',
- $enterFrom: { opacity: 0 }
+ // $enterFrom: { opacity: 0 }
}
}
}]
};
},
- data: [[121, 333], [29, 312]]
+ data: [[221, 333], [129, 312]]
}]
};
var chart = testHelper.create(echarts, 'enter-animation-and-merge', {
title: [
+ 'Transition animation:',
'(1) Move dataZoom, position should have transition animation.',
'(2) Use dataZoom hide a data item, and then show it, ensure the **fade in** animation not be interupted.',
- '(3) click button to setOption merge.',
- '(4) Repeat (2), should be correct.'
+ '(3) click button to setOption merge: ',
+ ' circle **disappears** and rect become **red border black bg**',
+ '(4) **Repeat (2)** after merged, should be correct.'
],
height: 300,
option: option,
buttons: [{
- text: 'replace style: border become red, and background become black',
+ text: 'click me to merge children',
onclick: function () {
chart.setOption({
series: {
type: 'custom',
renderItem: function (params, api) {
+ var pos = api.coord([api.value(0), api.value(1)]);
return {
type: 'group',
+ x: pos[0],
+ y: pos[1],
children: [{
type: 'rect',
+ shape: {
+ x: -50,
+ y: 50,
+ width: 100,
+ height: 50,
+ r: 10
+ },
style: {
stroke: 'red',
lineWidth: 5
}
}]
};
- },
- data: [[121, 333], [29, 312]]
+ }
}
});
}
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org