qf-jonathan commented on code in PR #37208:
URL: https://github.com/apache/superset/pull/37208#discussion_r2705013903


##########
superset-frontend/plugins/plugin-chart-echarts/src/Heatmap/transformProps.ts:
##########
@@ -291,8 +292,13 @@ export default function transformProps(
           );
           return [];
         }
-        return [[xIndex, yIndex, metricValue] as [number, number, any]];
-      }),
+        // Include rank as 4th dimension when normalized is enabled
+        // This allows visualMap to use dimension: 3 to color by rank 
percentile
+        if (normalized) {

Review Comment:
   @eschutho I've added error logging and skip rows that don't contain the 
`rank` dimension when `normalized` is enabled.
   
   On the other hand, falling back to a non-normalized chart (e.g. switching 
`visualMap.dimension` to 2) would also require changing `colorColumn` and 
recalculating the `min`/`max` bounds; otherwise, it would introduce new 
rendering issues. This would require additional changes.
   
   Personally, I don't think a fallback chart is desirable. If the user enables 
"Normalized", silently rendering a non-normalized chart would be misleading and 
would degrade the UX.
   
   The `rank` dimension is always expected. If it's missing at any point, it 
should be treated as a programming error and fixed at the source rather than 
hidden by a fallback. Please let me know what you think.



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

Reply via email to