[GitHub] [incubator-echarts] chfw commented on a change in pull request #13833: feat: The histogram supports intra group sorting and includes stacking states

[GitBox <gi...@apache.org>]

chfw commented on a change in pull request #13833:
URL: https://github.com/apache/incubator-echarts/pull/13833#discussion_r546963847



##########
File path: src/layout/barGrid.ts
##########
@@ -464,26 +515,77 @@ export function layout(seriesType: string, ecModel: GlobalModel) {
 
         lastStackCoords[stackId] = lastStackCoords[stackId] || [];
 
+        layoutInfo.groupOrder = seriesModel.get('groupOrder');
+        columnOffsetList[seriesIndex] = columnOffset;
+
         data.setLayout({
             bandWidth: columnLayoutInfo.bandWidth,
             offset: columnOffset,
             size: columnWidth
         });
-
         const valueDim = data.mapDimension(valueAxis.dim);
         const baseDim = data.mapDimension(baseAxis.dim);
         const stacked = isDimensionStacked(data, valueDim /*, baseDim*/);
         const isValueAxisH = valueAxis.isHorizontal();
 
         const valueAxisStart = getValueAxisStart(baseAxis, valueAxis, stacked);
-
+        let dataToPointList: number[];
         for (let idx = 0, len = data.count(); idx < len; idx++) {
-            const value = data.get(valueDim, idx);
+            const value = data.get(valueDim, idx) as number;
             const baseValue = data.get(baseDim, idx) as number;
-
+            if (isNaN(value) || isNaN(baseValue)) {
+                //When the value is NaN, it is still set to maintain the original code logic
+                data.setItemLayout(idx, {
+                    x: undefined,
+                    y: undefined,
+                    width: undefined,
+                    height: undefined
+                });
+                continue;
+            }
+            if (layoutDataList[baseValue] === undefined) {
+                layoutDataList[baseValue] = [];
+            }
             const sign = value >= 0 ? 'p' : 'n' as 'p' | 'n';
+            if (isValueAxisH) {
+                dataToPointList = [value, baseValue];
+            } else {
+                dataToPointList = [baseValue, value];
+            }
+            layoutDataList[baseValue].push({
+                idx: idx,
+                value: value,
+                baseValue: baseValue,
+                seriesModelData: data,
+                columnWidth: columnLayoutInfo.width,
+                coord: cartesian.dataToPoint(dataToPointList),
+                stacked: stacked,
+                stackId: stackId,
+                sign: sign,
+                valueAxisStart: valueAxisStart,
+                isValueAxisH: isValueAxisH,
+                barMinHeight: barMinHeight
+            })
+        }
+    });
+    if (layoutInfo.groupOrder !== undefined) {
+        orderLayoutData(layoutInfo);
+    }
+    zrUtil.each(layoutDataList, function (layoutDataListItem) {
+        zrUtil.each(layoutDataListItem, function (dataItem, itemIndex) {

Review comment:
       I pondered how would you unit test this long and anonymous function.




----------------------------------------------------------------
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.

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