You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@echarts.apache.org by GitBox <gi...@apache.org> on 2022/08/22 02:52:33 UTC
[GitHub] [echarts] pissang commented on a diff in pull request #17461: feat(funnel): add funnel new styles #14863
pissang commented on code in PR #17461:
URL: https://github.com/apache/echarts/pull/17461#discussion_r950959256
##########
src/chart/funnel/funnelLayout.ts:
##########
@@ -28,9 +28,9 @@ import { isFunction } from 'zrender/src/core/util';
function getViewRect(seriesModel: FunnelSeriesModel, api: ExtensionAPI) {
return layout.getLayoutRect(
seriesModel.getBoxLayoutParams(), {
- width: api.getWidth(),
- height: api.getHeight()
- }
+ width: api.getWidth(),
Review Comment:
wrong indentation here
##########
src/chart/funnel/FunnelSeries.ts:
##########
@@ -45,7 +45,7 @@ import SeriesData from '../../data/SeriesData';
type FunnelLabelOption = Omit<SeriesLabelOption, 'position'> & {
position?: LabelOption['position']
- | 'outer' | 'inner' | 'center' | 'rightTop' | 'rightBottom' | 'leftTop' | 'leftBottom'
+ | 'outer' | 'inner' | 'center' | 'rightTop' | 'rightBottom' | 'leftTop' | 'leftBottom'
Review Comment:
Please avoid this kind of code style change
##########
src/chart/funnel/funnelLayout.ts:
##########
@@ -337,55 +427,156 @@ export default function funnelLayout(ecModel: GlobalModel, api: ExtensionAPI) {
indices = indices.reverse();
}
- for (let i = 0; i < indices.length; i++) {
- const idx = indices[i];
- const nextIdx = indices[i + 1];
- const itemModel = data.getItemModel<FunnelDataItemOption>(idx);
+ const valueSum = valueArr.reduce((pre, cur) => pre + cur);
- if (orient === 'horizontal') {
- let width = itemModel.get(['itemStyle', 'width']);
- if (width == null) {
- width = itemSize;
+ const getSideLen = function (sideLen: number | string, idx?: number): number {
+ if (dynamicHeight) {
+ // in dy height, user can't set itemHeight or itemWidth
+ const val = data.get(valueDim, idx) as number || 0;
+ sideLen = linearMap(val, [0, valueSum], sizeExtent, true);
+ sideLen = sort === 'ascending' ? -sideLen : sideLen;
+ return sideLen;
+ }
+
+ if (sideLen == null) {
+ sideLen = itemSize;
+ }
+ else {
+ if (orient === 'horizontal') {
+ sideLen = parsePercent(sideLen, viewWidth);
}
else {
- width = parsePercent(width, viewWidth);
- if (sort === 'ascending') {
- width = -width;
- }
+ sideLen = parsePercent(sideLen, viewHeight);
}
- const start = getLinePoints(idx, x);
- const end = getLinePoints(nextIdx, x + width);
+ if (sort === 'ascending') {
+ sideLen = -sideLen;
+ }
+ }
+ return sideLen;
+ };
- x += width + gap;
+ const exitShape = seriesModel.get('exitShape');
+ let resSize = sizeExtent[1];
+ let maxSize = sizeExtent[1];
+ let exitWidth: string | number = seriesModel.get('exitWidth');
- data.setItemLayout(idx, {
- points: start.concat(end.slice().reverse())
- });
+ if (exitWidth) {
+ const percentReg = /^\w{1,2}\.{0,}\w{0,}%$/;
Review Comment:
`parsePercent` can be used here
##########
src/chart/funnel/funnelLayout.ts:
##########
@@ -324,6 +374,46 @@ export default function funnelLayout(ecModel: GlobalModel, api: ExtensionAPI) {
];
};
+ const getLinePointsBySize = function (offset: number, itemSize: number) {
+ if (orient === 'horizontal') {
Review Comment:
Similar to the comments above. the code can be more tight here.
##########
src/chart/funnel/funnelLayout.ts:
##########
@@ -277,9 +322,14 @@ export default function funnelLayout(ecModel: GlobalModel, api: ExtensionAPI) {
}
const funnelAlign = seriesModel.get('funnelAlign');
- let gap = seriesModel.get('gap');
- const viewSize = orient === 'horizontal' ? viewWidth : viewHeight;
- let itemSize = (viewSize - gap * (data.count() - 1)) / data.count();
+ let viewSize: number;
+ if (dynamicHeight) {
+ viewSize = orient === 'vertical' ? viewWidth : viewHeight;
Review Comment:
It's better to do the same check 'horizontal' so the behavior will be more consistent when the value is neither `vertical` nor `horizontal`
##########
test/funnel.html:
##########
@@ -19,141 +18,196 @@
-->
<html>
- <head>
- <meta charset="utf-8">
- <script src="lib/simpleRequire.js"></script>
- <script src="lib/config.js"></script>
- <script src="lib/dat.gui.min.js"></script>
- </head>
- <body>
- <style>
- html, body, #main {
- width: 100%;
- height: 100%;
- margin: 0;
- }
- </style>
- <div id="main"></div>
- <script>
- require([
- 'echarts'
- ], function (echarts) {
+<head>
+ <meta charset="utf-8">
Review Comment:
Please add create a new test case here. Changing the original test case will break the automatic test
##########
src/chart/funnel/funnelLayout.ts:
##########
@@ -337,55 +427,156 @@ export default function funnelLayout(ecModel: GlobalModel, api: ExtensionAPI) {
indices = indices.reverse();
}
- for (let i = 0; i < indices.length; i++) {
- const idx = indices[i];
- const nextIdx = indices[i + 1];
- const itemModel = data.getItemModel<FunnelDataItemOption>(idx);
+ const valueSum = valueArr.reduce((pre, cur) => pre + cur);
- if (orient === 'horizontal') {
- let width = itemModel.get(['itemStyle', 'width']);
- if (width == null) {
- width = itemSize;
+ const getSideLen = function (sideLen: number | string, idx?: number): number {
+ if (dynamicHeight) {
+ // in dy height, user can't set itemHeight or itemWidth
+ const val = data.get(valueDim, idx) as number || 0;
+ sideLen = linearMap(val, [0, valueSum], sizeExtent, true);
+ sideLen = sort === 'ascending' ? -sideLen : sideLen;
+ return sideLen;
+ }
+
+ if (sideLen == null) {
+ sideLen = itemSize;
+ }
+ else {
+ if (orient === 'horizontal') {
+ sideLen = parsePercent(sideLen, viewWidth);
}
else {
- width = parsePercent(width, viewWidth);
- if (sort === 'ascending') {
- width = -width;
- }
+ sideLen = parsePercent(sideLen, viewHeight);
}
- const start = getLinePoints(idx, x);
- const end = getLinePoints(nextIdx, x + width);
+ if (sort === 'ascending') {
+ sideLen = -sideLen;
+ }
+ }
+ return sideLen;
+ };
- x += width + gap;
+ const exitShape = seriesModel.get('exitShape');
+ let resSize = sizeExtent[1];
+ let maxSize = sizeExtent[1];
+ let exitWidth: string | number = seriesModel.get('exitWidth');
- data.setItemLayout(idx, {
- points: start.concat(end.slice().reverse())
- });
+ if (exitWidth) {
+ const percentReg = /^\w{1,2}\.{0,}\w{0,}%$/;
+ if (percentReg.test(exitWidth)) {
+ exitWidth = parseInt(exitWidth, 10);
+ resSize = maxSize = 100 * maxSize / (100 - exitWidth);
}
else {
- let height = itemModel.get(['itemStyle', 'height']);
- if (height == null) {
- height = itemSize;
- }
- else {
- height = parsePercent(height, viewHeight);
- if (sort === 'ascending') {
- height = -height;
+ throw new Error(`the exitWidth must be in percentage format and cannot be greater than 99%,
+ but you set it as '${exitWidth}'`);
+ }
+ }
+
+ const showRate = seriesModel.get('showRate');
+ let firstVal: number;
+ const setLayoutPoints =
+ // The subsequent funnel shape modification will be done in this func.
+ // We don’t need to concern direction when we use this function to set points.
+ function (
+ index: number,
+ idx: number,
+ nextIdx: number,
+ sideLen: number,
+ pos: number
+ ): void {
+ if (dynamicHeight) {
+ const start = getLinePointsBySize(pos, resSize / maxSize * viewSize);
+ let end;
+
+ if (index === indices.length - 1 && exitShape === 'rect') {
+ end = getLinePointsBySize(pos + sideLen, resSize / maxSize * viewSize);
+ }
+ else {
+ resSize += sort === 'ascending' ? sideLen : -sideLen;
+ end = getLinePointsBySize(pos + sideLen, resSize / maxSize * viewSize);
}
+
+ data.setItemLayout(idx, {
+ points: start.concat(end.slice().reverse())
+ });
+ return;
}
+ else if (showRate) {
+ const dataStart = getLinePoints(idx, pos);
+ let dataEnd;
+ if (exitWidth !== undefined && index === indices.length - 1) {
+ const val = data.get(valueDim, idx) as number || 0;
+ const itemSize = linearMap(val, [min, max], sizeExtent, true);
+ const exitSize = itemSize * (exitWidth as number) / 100;
+ dataEnd = getLinePointsBySize(pos + sideLen / 2, exitSize);
+ }
+ else {
+ dataEnd = getLinePoints(idx, pos + sideLen / 2);
+ }
+ const rateStart = getLinePoints(idx, pos + sideLen / 2);
+ const rateEnd = getLinePoints(nextIdx, pos + sideLen);
- const start = getLinePoints(idx, y);
- const end = getLinePoints(nextIdx, y + height);
+ const val = data.get(valueDim, idx) as number || 0;
+ const nextVal = data.get(valueDim, nextIdx) as number || 0;
+ let rate: number | string = nextVal / val;
+ rate = 'Rate ' + (rate * 100).toFixed(0) + '%';
+ if (index === 0) {
+ firstVal = val;
+ }
+ else if (index === indices.length - 1) {
+ const lastVal = val;
+ rate = lastVal / firstVal;
+ rate = 'Overall rate ' + (rate * 100).toFixed(0) + '%';
Review Comment:
rate string should not be fixed here. It needs to be configurable by the `formatter` option like the existing `label.formatter` option
##########
src/chart/funnel/funnelLayout.ts:
##########
@@ -246,26 +246,71 @@ function labelLayout(data: SeriesData) {
});
}
+function rateLabelLayout(data: SeriesData) {
+ data.each(function (idx) {
+ const layout = data.getItemLayout(idx);
+ const points = layout.ratePoints;
+
+ const isLabelInside = true;
+
+ const textX = (points[0][0] + points[1][0] + points[2][0] + points[3][0]) / 4;
+ const textY = (points[0][1] + points[1][1] + points[2][1] + points[3][1]) / 4;
+ const textAlign = 'center';
+
+ const linePoints = [
+ [textX, textY], [textX, textY]
+ ];
+
+ layout.rateLabel = {
+ linePoints: linePoints,
+ x: textX,
+ y: textY,
+ verticalAlign: 'middle',
+ textAlign: textAlign,
+ inside: isLabelInside
+ };
+ });
+}
+
export default function funnelLayout(ecModel: GlobalModel, api: ExtensionAPI) {
ecModel.eachSeriesByType('funnel', function (seriesModel: FunnelSeriesModel) {
const data = seriesModel.getData();
const valueDim = data.mapDimension('value');
+ const valueArr = data.mapArray(valueDim, function (val: number) {
+ return val;
+ });
const sort = seriesModel.get('sort');
const viewRect = getViewRect(seriesModel, api);
const orient = seriesModel.get('orient');
+ const dynamicHeight = seriesModel.get('dynamicHeight');
const viewWidth = viewRect.width;
const viewHeight = viewRect.height;
let indices = getSortedIndices(data, sort);
let x = viewRect.x;
let y = viewRect.y;
- const sizeExtent = orient === 'horizontal' ? [
- parsePercent(seriesModel.get('minSize'), viewHeight),
- parsePercent(seriesModel.get('maxSize'), viewHeight)
- ] : [
+ let gap = seriesModel.get('gap');
+ const gapSum = gap * (data.count() - 1);
+ let sizeExtent: Array<number> = null;
+ if (dynamicHeight) {
+ sizeExtent = orient === 'horizontal' ? [
Review Comment:
The code can be more tight here.
```ts
const isHorizontal = orient === 'horizontal';
const size = isHorizontal ? xxxx : dynamicHeight ? xxx : xxx;
sizeExtent = [
parsePercent(seriesModel.get( 'minSize'), size),
parsePercent(seriesModel.get( 'maxSize'), size),
]
```
##########
src/chart/funnel/funnelLayout.ts:
##########
@@ -337,55 +427,156 @@ export default function funnelLayout(ecModel: GlobalModel, api: ExtensionAPI) {
indices = indices.reverse();
}
- for (let i = 0; i < indices.length; i++) {
- const idx = indices[i];
- const nextIdx = indices[i + 1];
- const itemModel = data.getItemModel<FunnelDataItemOption>(idx);
+ const valueSum = valueArr.reduce((pre, cur) => pre + cur);
- if (orient === 'horizontal') {
- let width = itemModel.get(['itemStyle', 'width']);
- if (width == null) {
- width = itemSize;
+ const getSideLen = function (sideLen: number | string, idx?: number): number {
+ if (dynamicHeight) {
+ // in dy height, user can't set itemHeight or itemWidth
+ const val = data.get(valueDim, idx) as number || 0;
+ sideLen = linearMap(val, [0, valueSum], sizeExtent, true);
+ sideLen = sort === 'ascending' ? -sideLen : sideLen;
+ return sideLen;
+ }
+
+ if (sideLen == null) {
+ sideLen = itemSize;
+ }
+ else {
+ if (orient === 'horizontal') {
+ sideLen = parsePercent(sideLen, viewWidth);
}
else {
- width = parsePercent(width, viewWidth);
- if (sort === 'ascending') {
- width = -width;
- }
+ sideLen = parsePercent(sideLen, viewHeight);
}
- const start = getLinePoints(idx, x);
- const end = getLinePoints(nextIdx, x + width);
+ if (sort === 'ascending') {
+ sideLen = -sideLen;
+ }
+ }
+ return sideLen;
+ };
- x += width + gap;
+ const exitShape = seriesModel.get('exitShape');
+ let resSize = sizeExtent[1];
+ let maxSize = sizeExtent[1];
+ let exitWidth: string | number = seriesModel.get('exitWidth');
- data.setItemLayout(idx, {
- points: start.concat(end.slice().reverse())
- });
+ if (exitWidth) {
+ const percentReg = /^\w{1,2}\.{0,}\w{0,}%$/;
+ if (percentReg.test(exitWidth)) {
+ exitWidth = parseInt(exitWidth, 10);
+ resSize = maxSize = 100 * maxSize / (100 - exitWidth);
}
else {
- let height = itemModel.get(['itemStyle', 'height']);
- if (height == null) {
- height = itemSize;
- }
- else {
- height = parsePercent(height, viewHeight);
- if (sort === 'ascending') {
- height = -height;
+ throw new Error(`the exitWidth must be in percentage format and cannot be greater than 99%,
+ but you set it as '${exitWidth}'`);
+ }
+ }
+
+ const showRate = seriesModel.get('showRate');
+ let firstVal: number;
+ const setLayoutPoints =
+ // The subsequent funnel shape modification will be done in this func.
+ // We don’t need to concern direction when we use this function to set points.
+ function (
+ index: number,
+ idx: number,
+ nextIdx: number,
+ sideLen: number,
+ pos: number
+ ): void {
+ if (dynamicHeight) {
+ const start = getLinePointsBySize(pos, resSize / maxSize * viewSize);
+ let end;
+
+ if (index === indices.length - 1 && exitShape === 'rect') {
+ end = getLinePointsBySize(pos + sideLen, resSize / maxSize * viewSize);
+ }
+ else {
+ resSize += sort === 'ascending' ? sideLen : -sideLen;
+ end = getLinePointsBySize(pos + sideLen, resSize / maxSize * viewSize);
}
+
+ data.setItemLayout(idx, {
+ points: start.concat(end.slice().reverse())
+ });
+ return;
}
+ else if (showRate) {
+ const dataStart = getLinePoints(idx, pos);
+ let dataEnd;
+ if (exitWidth !== undefined && index === indices.length - 1) {
+ const val = data.get(valueDim, idx) as number || 0;
Review Comment:
`number || 0` is still `number`
##########
src/chart/funnel/funnelLayout.ts:
##########
@@ -337,55 +427,156 @@ export default function funnelLayout(ecModel: GlobalModel, api: ExtensionAPI) {
indices = indices.reverse();
}
- for (let i = 0; i < indices.length; i++) {
- const idx = indices[i];
- const nextIdx = indices[i + 1];
- const itemModel = data.getItemModel<FunnelDataItemOption>(idx);
+ const valueSum = valueArr.reduce((pre, cur) => pre + cur);
- if (orient === 'horizontal') {
- let width = itemModel.get(['itemStyle', 'width']);
- if (width == null) {
- width = itemSize;
+ const getSideLen = function (sideLen: number | string, idx?: number): number {
+ if (dynamicHeight) {
+ // in dy height, user can't set itemHeight or itemWidth
+ const val = data.get(valueDim, idx) as number || 0;
+ sideLen = linearMap(val, [0, valueSum], sizeExtent, true);
+ sideLen = sort === 'ascending' ? -sideLen : sideLen;
+ return sideLen;
+ }
+
+ if (sideLen == null) {
+ sideLen = itemSize;
+ }
+ else {
+ if (orient === 'horizontal') {
+ sideLen = parsePercent(sideLen, viewWidth);
}
else {
- width = parsePercent(width, viewWidth);
- if (sort === 'ascending') {
- width = -width;
- }
+ sideLen = parsePercent(sideLen, viewHeight);
}
- const start = getLinePoints(idx, x);
- const end = getLinePoints(nextIdx, x + width);
+ if (sort === 'ascending') {
+ sideLen = -sideLen;
+ }
+ }
+ return sideLen;
+ };
- x += width + gap;
+ const exitShape = seriesModel.get('exitShape');
+ let resSize = sizeExtent[1];
+ let maxSize = sizeExtent[1];
+ let exitWidth: string | number = seriesModel.get('exitWidth');
- data.setItemLayout(idx, {
- points: start.concat(end.slice().reverse())
- });
+ if (exitWidth) {
+ const percentReg = /^\w{1,2}\.{0,}\w{0,}%$/;
+ if (percentReg.test(exitWidth)) {
+ exitWidth = parseInt(exitWidth, 10);
+ resSize = maxSize = 100 * maxSize / (100 - exitWidth);
}
else {
- let height = itemModel.get(['itemStyle', 'height']);
- if (height == null) {
- height = itemSize;
- }
- else {
- height = parsePercent(height, viewHeight);
- if (sort === 'ascending') {
- height = -height;
+ throw new Error(`the exitWidth must be in percentage format and cannot be greater than 99%,
+ but you set it as '${exitWidth}'`);
+ }
+ }
+
+ const showRate = seriesModel.get('showRate');
+ let firstVal: number;
+ const setLayoutPoints =
+ // The subsequent funnel shape modification will be done in this func.
+ // We don’t need to concern direction when we use this function to set points.
+ function (
+ index: number,
+ idx: number,
+ nextIdx: number,
+ sideLen: number,
+ pos: number
+ ): void {
+ if (dynamicHeight) {
+ const start = getLinePointsBySize(pos, resSize / maxSize * viewSize);
+ let end;
+
+ if (index === indices.length - 1 && exitShape === 'rect') {
+ end = getLinePointsBySize(pos + sideLen, resSize / maxSize * viewSize);
+ }
+ else {
+ resSize += sort === 'ascending' ? sideLen : -sideLen;
+ end = getLinePointsBySize(pos + sideLen, resSize / maxSize * viewSize);
}
+
+ data.setItemLayout(idx, {
+ points: start.concat(end.slice().reverse())
+ });
+ return;
}
+ else if (showRate) {
+ const dataStart = getLinePoints(idx, pos);
+ let dataEnd;
+ if (exitWidth !== undefined && index === indices.length - 1) {
+ const val = data.get(valueDim, idx) as number || 0;
+ const itemSize = linearMap(val, [min, max], sizeExtent, true);
+ const exitSize = itemSize * (exitWidth as number) / 100;
+ dataEnd = getLinePointsBySize(pos + sideLen / 2, exitSize);
+ }
+ else {
+ dataEnd = getLinePoints(idx, pos + sideLen / 2);
+ }
+ const rateStart = getLinePoints(idx, pos + sideLen / 2);
+ const rateEnd = getLinePoints(nextIdx, pos + sideLen);
- const start = getLinePoints(idx, y);
- const end = getLinePoints(nextIdx, y + height);
+ const val = data.get(valueDim, idx) as number || 0;
+ const nextVal = data.get(valueDim, nextIdx) as number || 0;
+ let rate: number | string = nextVal / val;
+ rate = 'Rate ' + (rate * 100).toFixed(0) + '%';
+ if (index === 0) {
+ firstVal = val;
+ }
+ else if (index === indices.length - 1) {
+ const lastVal = val;
+ rate = lastVal / firstVal;
+ rate = 'Overall rate ' + (rate * 100).toFixed(0) + '%';
+ }
- y += height + gap;
+ data.setItemLayout(idx, {
+ points: dataStart.concat(dataEnd.slice().reverse()),
+ ratePoints: rateStart.concat(rateEnd.slice().reverse()),
+ isLastPiece: index === indices.length - 1,
+ rate
+ });
+ return;
+ }
+ const start = getLinePoints(idx, pos);
+ let end;
+
+ if (index === indices.length - 1 && exitShape === 'rect') {
+ end = getLinePoints(idx, pos + sideLen);
Review Comment:
Like the comments above. The code can be tighter here.
```ts
end = getLinePoints( index === indices.length - 1 && exitShape === 'rect' ? idx : nextIdx, pos + sideLen);
```
--
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: commits-unsubscribe@echarts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@echarts.apache.org
For additional commands, e-mail: commits-help@echarts.apache.org