rusackas commented on code in PR #37625:
URL: https://github.com/apache/superset/pull/37625#discussion_r2760088455
##########
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:
This is a direct TypeScript migration of existing legacy plugin code. The
suggestion identifies a pre-existing edge case that should be addressed in a
separate bug fix PR, not in a migration PR.
##########
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:
This is a direct TypeScript migration of existing legacy plugin code. The
suggestion identifies a pre-existing edge case that should be addressed in a
separate bug fix PR, not in a migration PR.
##########
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:
This is a direct TypeScript migration of existing legacy plugin code. The
suggestion identifies a pre-existing edge case that should be addressed in a
separate bug fix PR, not in a migration PR.
--
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]