rusackas commented on code in PR #37625:
URL: https://github.com/apache/superset/pull/37625#discussion_r2758098752
##########
superset-frontend/plugins/legacy-plugin-chart-map-box/src/MapBox.tsx:
##########
@@ -29,24 +28,46 @@ const NOOP = () => {};
export const DEFAULT_MAX_ZOOM = 16;
export const DEFAULT_POINT_RADIUS = 60;
-const propTypes = {
- width: PropTypes.number,
- height: PropTypes.number,
- aggregatorName: PropTypes.string,
- clusterer: PropTypes.object,
- globalOpacity: PropTypes.number,
- hasCustomMetric: PropTypes.bool,
- mapStyle: PropTypes.string,
- mapboxApiKey: PropTypes.string.isRequired,
- onViewportChange: PropTypes.func,
- pointRadius: PropTypes.number,
- pointRadiusUnit: PropTypes.string,
- renderWhileDragging: PropTypes.bool,
- rgb: PropTypes.array,
- bounds: PropTypes.array,
-};
+interface Viewport {
+ longitude: number;
+ latitude: number;
+ zoom: number;
+ isDragging?: boolean;
+}
+
+interface Clusterer {
+ getClusters(bbox: number[], zoom: number): GeoJSONLocation[];
+}
+
+interface GeoJSONLocation {
+ geometry: {
+ coordinates: [number, number];
+ };
+ properties: Record<string, number | string | boolean | null | undefined>;
+}
-const defaultProps = {
+interface MapBoxProps {
+ width?: number;
+ height?: number;
+ aggregatorName?: string;
+ clusterer?: Clusterer;
+ globalOpacity?: number;
+ hasCustomMetric?: boolean;
+ mapStyle?: string;
+ mapboxApiKey: string;
+ onViewportChange?: (viewport: Viewport) => void;
+ pointRadius?: number;
+ pointRadiusUnit?: string;
+ renderWhileDragging?: boolean;
+ rgb?: (string | number)[];
+ bounds?: [[number, number], [number, number]];
Review Comment:
Fixed in 79e7d816cc. Made `bounds` a required prop since it's always
dereferenced for fitBounds and bbox computation. Removed the optional marker
from the interface.
##########
superset-frontend/plugins/legacy-plugin-chart-map-box/src/ScatterPlotGlowOverlay.tsx:
##########
@@ -186,7 +231,7 @@ class ScatterPlotGlowOverlay extends PureComponent {
if (location.properties.cluster) {
let clusterLabel = clusterLabelMap[i];
const scaledRadius = roundDecimal(
- (clusterLabel / maxLabel) ** 0.5 * radius,
+ ((clusterLabel as number) / maxLabel) ** 0.5 * radius,
Review Comment:
Fixed in 79e7d816cc. Added guards against zero/non-finite
maxLabel:\n```typescript\nconst filteredLabels = clusterLabelMap.filter(v =>
!Number.isNaN(v)) as number[];\nconst maxLabel = filteredLabels.length > 0 ?
Math.max(...filteredLabels) : 1;\nconst safeMaxLabel = maxLabel > 0 ? maxLabel
: 1;\n```
##########
superset-frontend/plugins/legacy-plugin-chart-map-box/src/ScatterPlotGlowOverlay.tsx:
##########
@@ -234,23 +279,28 @@ class ScatterPlotGlowOverlay extends PureComponent {
}
} else {
const defaultRadius = radius / 6;
- const radiusProperty = location.properties.radius;
- const pointMetric = location.properties.metric;
- let pointRadius =
- radiusProperty === null ? defaultRadius : radiusProperty;
- let pointLabel;
+ const radiusProperty = location.properties.radius as number | null;
+ const pointMetric = location.properties.metric as
+ | number
+ | string
+ | null;
+ let pointRadius: number =
+ radiusProperty === null
+ ? defaultRadius
+ : (radiusProperty as number);
+ let pointLabel: string | number | undefined;
if (radiusProperty !== null) {
Review Comment:
Fixed in 79e7d816cc. Changed from `=== null` to `== null` to properly handle
both null and undefined values, ensuring the code doesn't enter the conversion
block when the radius property is absent.
--
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]
