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


##########
superset-frontend/plugins/legacy-plugin-chart-country-map/src/CountryMap.js:
##########
@@ -41,10 +41,65 @@ const propTypes = {
   linearColorScheme: PropTypes.string,
   mapBaseUrl: PropTypes.string,
   numberFormat: PropTypes.string,
+  customColorRules: PropTypes.array,

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Prop name mismatch in PropTypes</b></div>
   <div id="fix">
   
   The PropTypes definition adds 'customColorRules' as an array prop, but the 
component destructures 'customColorScale' from props. This mismatch could 
prevent the prop from being properly validated or used, potentially leading to 
incorrect color scaling behavior if the prop is passed.
   </div>
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #d66de9</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



##########
superset-frontend/plugins/legacy-plugin-chart-country-map/src/CountryMap.js:
##########
@@ -53,22 +108,127 @@ function CountryMap(element, props) {
     country,
     linearColorScheme,
     numberFormat,
+    customColorScale = [],
     colorScheme,
     sliceId,
   } = props;
 
   const container = element;
   const format = getNumberFormatter(numberFormat);
-  const linearColorScale = getSequentialSchemeRegistry()
-    .get(linearColorScheme)
-    .createLinearScale(d3Extent(data, v => v.metric));
-  const colorScale = CategoricalColorNamespace.getScale(colorScheme);
+  const normalizedScale = normalizeScale(customColorScale);
+  const normalizedScaleWithColors = Array.isArray(normalizedScale)
+    ? normalizedScale.map(e => {
+        if (!e || typeof e !== 'object') return e;
+        return { ...e, color: normalizeColorKeyword(e.color) };
+      })
+    : [];
+
+  const parsedData = Array.isArray(data)
+    ? data.map(r => ({ ...r, metric: safeNumber(r.metric) }))
+    : [];
+
+  const numericValues = parsedData
+    .map(r => r.metric)
+    .filter(v => Number.isFinite(v));
+
+  let minValue = 0;
+  let maxValue = 1;
+  if (numericValues.length > 0) {
+    const extent = d3Extent(numericValues);
+    minValue = extent[0];
+    maxValue = extent[1];
+  }
+  const valueRange = maxValue - minValue;
+  const valueRangeNonZero = valueRange === 0 ? 1 : valueRange;
+
+  let percentColorScale = null;
+  if (
+    Array.isArray(normalizedScaleWithColors) &&
+    normalizedScaleWithColors.length >= 2
+  ) {
+    const sorted = normalizedScaleWithColors
+      .filter(
+        e => e && typeof e.percent === 'number' && typeof e.color === 'string',
+      )
+      .slice()
+      .sort((a, b) => a.percent - b.percent);
+
+    if (sorted.length >= 2) {
+      const domainPerc = sorted.map(e => e.percent);
+      const rangeColors = sorted.map(e => e.color);
+      percentColorScale = d3.scale
+        .linear()
+        .domain(domainPerc)
+        .range(rangeColors)
+        .clamp(true)
+        // Remove interpolation to avoid blending between steps - always 
return lower boundary
+        .interpolate(function (a, b) {
+          return function () {
+            return a;
+          };
+        });

Review Comment:
   <div>
   
   
   <div id="suggestion">
   <div id="issue"><b>Incorrect Color Scale Implementation</b></div>
   <div id="fix">
   
   The percentColorScale uses d3.scale.linear with a custom interpolate 
function that always returns the lower boundary color, which prevents the last 
color in the range from ever being used. For step-based color scales with 
explicit thresholds, d3.scale.threshold should be used instead, as it properly 
maps intervals to discrete colors without this limitation.
   </div>
   
   
   </div>
   
   
   
   
   <small><i>Code Review Run #d66de9</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: [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