fix(charts): numerical column for the Point Radius field in mapbox (#36962)

superset-frontend/plugins/legacy-plugin-chart-map-box/src/ScatterPlotGlowOverlay.jsx

@@ -145,6 +145,26 @@ class ScatterPlotGlowOverlay extends PureComponent {
 
     const maxLabel = Math.max(...clusterLabelMap.filter(v => !Number.isNaN(v)));
 
+    // Calculate min/max radius values for Pixels mode scaling
+    let minRadiusValue = Infinity;
+    let maxRadiusValue = -Infinity;
+    if (pointRadiusUnit === 'Pixels') {
+      locations.forEach(location => {
+        // Accept both null and undefined as "no value" and coerce potential numeric strings
+        if (
+          !location.properties.cluster &&
+          location.properties.radius != null
+        ) {
+          const radiusValueRaw = location.properties.radius;
+          const radiusValue = Number(radiusValueRaw);
+          if (Number.isFinite(radiusValue)) {
+            minRadiusValue = Math.min(minRadiusValue, radiusValue);
+            maxRadiusValue = Math.max(maxRadiusValue, radiusValue);
+          }
+        }
+      });
+    }
+
     ctx.clearRect(0, 0, width, height);
     ctx.globalCompositeOperation = compositeOperation;
 
@@ -232,6 +252,50 @@ class ScatterPlotGlowOverlay extends PureComponent {
                   pointLatitude,
                   zoom,
                 );
+              } else if (pointRadiusUnit === 'Pixels') {
+                // Scale pixel values to a reasonable range (radius/6 to radius/3)
+                // This ensures points are visible and proportional to their values
+                const MIN_POINT_RADIUS = radius / 6;
+                const MAX_POINT_RADIUS = radius / 3;
+
+                if (
+                  Number.isFinite(minRadiusValue) &&
+                  Number.isFinite(maxRadiusValue) &&
+                  maxRadiusValue > minRadiusValue
+                ) {
+                  // Normalize the value to 0-1 range, then scale to pixel range
+                  const numericPointRadius = Number(pointRadius);
+                  if (!Number.isFinite(numericPointRadius)) {
+                    // fallback to minimum visible size when the value is not a finite number
+                    pointRadius = MIN_POINT_RADIUS;
+                  } else {
+                    const normalizedValueRaw =
+                      (numericPointRadius - minRadiusValue) /
+                      (maxRadiusValue - minRadiusValue);
+                    const normalizedValue = Math.max(
+                      0,
+                      Math.min(1, normalizedValueRaw),
+                    );
+                    pointRadius =
+                      MIN_POINT_RADIUS +
+                      normalizedValue * (MAX_POINT_RADIUS - MIN_POINT_RADIUS);
+                  }
+                  pointLabel = `${roundDecimal(radiusProperty, 2)}`;
+                } else if (
+                  Number.isFinite(minRadiusValue) &&
+                  minRadiusValue === maxRadiusValue
+                ) {
+                  // All values are the same, use a fixed medium size
+                  pointRadius = (MIN_POINT_RADIUS + MAX_POINT_RADIUS) / 2;
+                  pointLabel = `${roundDecimal(radiusProperty, 2)}`;
+                } else {
+                  // Use raw pixel values if they're already in a reasonable range
+                  pointRadius = Math.max(
+                    MIN_POINT_RADIUS,
+                    Math.min(pointRadius, MAX_POINT_RADIUS),
+                  );
+                  pointLabel = `${roundDecimal(radiusProperty, 2)}`;
+                }
               }
             }
 

superset-frontend/plugins/legacy-plugin-chart-map-box/src/transformProps.js

@@ -90,7 +90,9 @@ export default function transformProps(chartProps) {
       setControlValue('viewport_latitude', latitude);
       setControlValue('viewport_zoom', zoom);
     },
-    pointRadius: pointRadius === 'Auto' ? DEFAULT_POINT_RADIUS : pointRadius,
+    // Always use DEFAULT_POINT_RADIUS as the base radius for cluster sizing
+    // Individual point radii come from geoJSON properties.radius
+    pointRadius: DEFAULT_POINT_RADIUS,
     pointRadiusUnit,
     renderWhileDragging,
     rgb,

superset-frontend/plugins/legacy-plugin-chart-map-box/test/ScatterPlotGlowOverlay.test.tsx

@@ -0,0 +1,346 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { render } from '@testing-library/react';
+import ScatterPlotGlowOverlay from '../src/ScatterPlotGlowOverlay';
+
+// Mock react-map-gl's CanvasOverlay
+jest.mock('react-map-gl', () => ({
+  CanvasOverlay: ({ redraw }: { redraw: Function }) => {
+    // Store the redraw function so tests can call it
+    (global as any).mockRedraw = redraw;
+    return <div data-testid="canvas-overlay" />;
+  },
+}));
+
+// Mock utility functions
+jest.mock('../src/utils/luminanceFromRGB', () => ({
+  __esModule: true,
+  default: jest.fn(() => 150), // Return a value above the dark threshold
+}));
+
+// Test helpers
+const createMockCanvas = () => {
+  const ctx: any = {
+    clearRect: jest.fn(),
+    beginPath: jest.fn(),
+    arc: jest.fn(),
+    fill: jest.fn(),
+    fillText: jest.fn(),
+    measureText: jest.fn(() => ({ width: 10 })),
+    createRadialGradient: jest.fn(() => ({
+      addColorStop: jest.fn(),
+    })),
+    globalCompositeOperation: '',
+    fillStyle: '',
+    font: '',
+    textAlign: '',
+    textBaseline: '',
+    shadowBlur: 0,
+    shadowColor: '',
+  };
+
+  return ctx;
+};
+
+const createMockRedrawParams = (overrides = {}) => ({
+  width: 800,
+  height: 600,
+  ctx: createMockCanvas(),
+  isDragging: false,
+  project: (lngLat: [number, number]) => lngLat,
+  ...overrides,
+});
+
+const createLocation = (
+  coordinates: [number, number],
+  properties: Record<string, any>,
+) => ({
+  geometry: { coordinates },
+  properties,
+});
+
+const defaultProps = {
+  lngLatAccessor: (loc: any) => loc.geometry.coordinates,
+  dotRadius: 60,
+  rgb: ['', 255, 0, 0] as any,
+  globalOpacity: 1,
+};
+
+test('renders map with varying radius values in Pixels mode', () => {
+  const locations = [
+    createLocation([100, 100], { radius: 10, cluster: false }),
+    createLocation([200, 200], { radius: 50, cluster: false }),
+    createLocation([300, 300], { radius: 100, cluster: false }),
+  ];
+
+  expect(() => {
+    render(
+      <ScatterPlotGlowOverlay
+        {...defaultProps}
+        locations={locations}
+        pointRadiusUnit="Pixels"
+      />,
+    );
+    const redrawParams = createMockRedrawParams();
+    (global as any).mockRedraw(redrawParams);
+  }).not.toThrow();
+});
+
+test('handles dataset with uniform radius values', () => {
+  const locations = [
+    createLocation([100, 100], { radius: 50, cluster: false }),
+    createLocation([200, 200], { radius: 50, cluster: false }),
+    createLocation([300, 300], { radius: 50, cluster: false }),
+  ];
+
+  expect(() => {
+    render(
+      <ScatterPlotGlowOverlay
+        {...defaultProps}
+        locations={locations}
+        pointRadiusUnit="Pixels"
+      />,
+    );
+    const redrawParams = createMockRedrawParams();
+    (global as any).mockRedraw(redrawParams);
+  }).not.toThrow();
+});
+
+test('renders successfully when data contains non-finite values', () => {
+  const locations = [
+    createLocation([100, 100], { radius: 10, cluster: false }),
+    createLocation([200, 200], { radius: NaN, cluster: false }),
+    createLocation([300, 300], { radius: 100, cluster: false }),
+  ];
+
+  expect(() => {
+    render(
+      <ScatterPlotGlowOverlay
+        {...defaultProps}
+        locations={locations}
+        pointRadiusUnit="Pixels"
+      />,
+    );
+    const redrawParams = createMockRedrawParams();
+    (global as any).mockRedraw(redrawParams);
+  }).not.toThrow();
+});
+
+test('handles radius values provided as strings', () => {
+  const locations = [
+    createLocation([100, 100], { radius: '10', cluster: false }),
+    createLocation([200, 200], { radius: '50', cluster: false }),
+    createLocation([300, 300], { radius: '100', cluster: false }),
+  ];
+
+  expect(() => {
+    render(
+      <ScatterPlotGlowOverlay
+        {...defaultProps}
+        locations={locations}
+        pointRadiusUnit="Pixels"
+      />,
+    );
+    const redrawParams = createMockRedrawParams();
+    (global as any).mockRedraw(redrawParams);
+  }).not.toThrow();
+});
+
+test('renders points when radius values are missing', () => {
+  const locations = [
+    createLocation([100, 100], { radius: null, cluster: false }),
+    createLocation([200, 200], { radius: undefined, cluster: false }),
+    createLocation([300, 300], { cluster: false }),
+  ];
+
+  expect(() => {
+    render(
+      <ScatterPlotGlowOverlay
+        {...defaultProps}
+        locations={locations}
+        pointRadiusUnit="Pixels"
+      />,
+    );
+    const redrawParams = createMockRedrawParams();
+    (global as any).mockRedraw(redrawParams);
+  }).not.toThrow();
+});
+
+test('renders both cluster and non-cluster points correctly', () => {
+  const locations = [
+    createLocation([100, 100], { radius: 10, cluster: false }),
+    createLocation([200, 200], {
+      radius: 999,
+      cluster: true,
+      point_count: 5,
+      sum: 100,
+    }),
+    createLocation([300, 300], { radius: 100, cluster: false }),
+  ];
+
+  expect(() => {
+    render(
+      <ScatterPlotGlowOverlay
+        {...defaultProps}
+        locations={locations}
+        pointRadiusUnit="Pixels"
+      />,
+    );
+    const redrawParams = createMockRedrawParams();
+    (global as any).mockRedraw(redrawParams);
+  }).not.toThrow();
+});
+
+test('renders map with multiple points with different radius values', () => {
+  const locations = [
+    createLocation([100, 100], { radius: 10, cluster: false }),
+    createLocation([200, 200], { radius: 42.567, cluster: false }),
+    createLocation([300, 300], { radius: 100, cluster: false }),
+  ];
+
+  expect(() => {
+    render(
+      <ScatterPlotGlowOverlay
+        {...defaultProps}
+        locations={locations}
+        pointRadiusUnit="Pixels"
+      />,
+    );
+    const redrawParams = createMockRedrawParams();
+    (global as any).mockRedraw(redrawParams);
+  }).not.toThrow();
+});
+
+test('renders map with Kilometers mode', () => {
+  const locations = [
+    createLocation([100, 50], { radius: 10, cluster: false }),
+    createLocation([200, 50], { radius: 5, cluster: false }),
+  ];
+
+  expect(() => {
+    render(
+      <ScatterPlotGlowOverlay
+        {...defaultProps}
+        locations={locations}
+        pointRadiusUnit="Kilometers"
+        zoom={10}
+      />,
+    );
+    const redrawParams = createMockRedrawParams();
+    (global as any).mockRedraw(redrawParams);
+  }).not.toThrow();
+});
+
+test('renders map with Miles mode', () => {
+  const locations = [
+    createLocation([100, 50], { radius: 5, cluster: false }),
+    createLocation([200, 50], { radius: 10, cluster: false }),
+  ];
+
+  expect(() => {
+    render(
+      <ScatterPlotGlowOverlay
+        {...defaultProps}
+        locations={locations}
+        pointRadiusUnit="Miles"
+        zoom={10}
+      />,
+    );
+    const redrawParams = createMockRedrawParams();
+    (global as any).mockRedraw(redrawParams);
+  }).not.toThrow();
+});
+
+test('displays metric property labels on points', () => {
+  const locations = [
+    createLocation([100, 100], { radius: 50, metric: 123.456, cluster: false }),
+  ];
+
+  expect(() => {
+    render(
+      <ScatterPlotGlowOverlay
+        {...defaultProps}
+        locations={locations}
+        pointRadiusUnit="Pixels"
+      />,
+    );
+    const redrawParams = createMockRedrawParams();
+    (global as any).mockRedraw(redrawParams);
+  }).not.toThrow();
+});
+
+test('handles empty dataset without errors', () => {
+  expect(() => {
+    render(
+      <ScatterPlotGlowOverlay
+        {...defaultProps}
+        locations={[]}
+        pointRadiusUnit="Pixels"
+      />,
+    );
+    const redrawParams = createMockRedrawParams();
+    (global as any).mockRedraw(redrawParams);
+  }).not.toThrow();
+});
+
+test('handles extreme outlier radius values without breaking', () => {
+  const locations = [
+    createLocation([100, 100], { radius: 1, cluster: false }),
+    createLocation([200, 200], { radius: 50, cluster: false }),
+    createLocation([300, 300], { radius: 999999, cluster: false }),
+  ];
+
+  expect(() => {
+    render(
+      <ScatterPlotGlowOverlay
+        {...defaultProps}
+        locations={locations}
+        pointRadiusUnit="Pixels"
+      />,
+    );
+    const redrawParams = createMockRedrawParams();
+    (global as any).mockRedraw(redrawParams);
+  }).not.toThrow();
+});
+
+test('renders successfully with mixed extreme and negative radius values', () => {
+  const locations = [
+    createLocation([100, 100], { radius: 0.001, cluster: false }),
+    createLocation([150, 150], { radius: 5, cluster: false }),
+    createLocation([200, 200], { radius: 100, cluster: false }),
+    createLocation([250, 250], { radius: 50000, cluster: false }),
+    createLocation([300, 300], { radius: -10, cluster: false }),
+  ];
+
+  expect(() => {
+    render(
+      <ScatterPlotGlowOverlay
+        {...defaultProps}
+        locations={locations}
+        pointRadiusUnit="Pixels"
+      />,
+    );
+  }).not.toThrow();
+
+  expect(() => {
+    const redrawParams = createMockRedrawParams();
+    (global as any).mockRedraw(redrawParams);
+  }).not.toThrow();
+});