Re: [PR] feat(bar-chart): add option to color bars by primary axis when no dimensions are set [superset]

["bito-code-review[bot] (via GitHub)" <gi...@apache.org>]

bito-code-review[bot] commented on code in PR #37531:
URL: https://github.com/apache/superset/pull/37531#discussion_r2849131741


##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -438,6 +440,59 @@ export default function transformProps(
     }
   });
 
+  // Add x-axis color legend when colorByPrimaryAxis is enabled
+  if (colorByPrimaryAxis && groupBy.length === 0 && series.length > 0) {
+    // Hide original series from legend
+    series.forEach(s => {
+      s.legendHoverLink = false;
+    });
+
+    // Get x-axis values from the first series
+    const firstSeries = series[0];
+    if (firstSeries && Array.isArray(firstSeries.data)) {
+      const xAxisValues: (string | number)[] = [];
+
+      // Extract primary axis values (category axis)
+      // For horizontal charts the category is at index 1, for vertical at index 0
+      const primaryAxisIndex = isHorizontal ? 1 : 0;
+      (firstSeries.data as any[]).forEach(point => {
+        let xValue;
+        if (point && typeof point === 'object' && 'value' in point) {
+          const val = point.value;
+          xValue = Array.isArray(val) ? val[primaryAxisIndex] : val;
+        } else if (Array.isArray(point)) {
+          xValue = point[primaryAxisIndex];
+        } else {
+          xValue = point;
+        }
+        xAxisValues.push(xValue);
+      });
+
+      // Create hidden series for legend (using 'line' type to not affect bar width)
+      // Deduplicate x-axis values to avoid duplicate legend entries and unnecessary series
+      const uniqueXAxisValues = Array.from(
+        new Set(xAxisValues.map(v => String(v))),
+      );
+      uniqueXAxisValues.forEach(xValue => {
+        const colorKey = xValue;
+        series.push({
+          name: xValue,
+          type: 'line', // Use line type to not affect bar positioning
+          data: [], // Empty - doesn't render
+          itemStyle: {
+            color: colorScale(colorKey, sliceId),
+          },
+          lineStyle: {
+            color: colorScale(colorKey, sliceId),
+          },
+          silent: true,

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Missing Test Coverage</b></div>
   <div id="fix">
   
   The new colorByPrimaryAxis feature introduces complex logic for extracting x-axis values and modifying series/legend, but no tests were found in the existing test suite. Tests should assert the behavior directly, per project guidelines.
   </div>
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #093b63</i></small>
   </div>
   
   ---
   Should Bito avoid suggestions like this for future reviews? (<a href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
   - [ ] Yes, avoid them



-- 
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: notifications-unsubscribe@superset.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org