rusackas commented on code in PR #37625:
URL: https://github.com/apache/superset/pull/37625#discussion_r2760087142


##########
superset-frontend/plugins/legacy-plugin-chart-horizon/src/HorizonChart.tsx:
##########
@@ -96,13 +97,13 @@ class HorizonChart extends PureComponent {
       offsetX,
     } = this.props;
 
-    let yDomain;
+    let yDomain: [number, number] | undefined;
     if (colorScale === 'overall') {
-      const allValues = data.reduce(
+      const allValues = data.reduce<DataValue[]>(
         (acc, current) => acc.concat(current.values),
         [],
       );
-      yDomain = d3Extent(allValues, d => d.y);
+      yDomain = d3Extent(allValues, d => d.y) as [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-horizon/src/transformProps.ts:
##########
@@ -16,7 +16,9 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-export default function transformProps(chartProps) {
+import { ChartProps } from '@superset-ui/core';
+
+export default function transformProps(chartProps: ChartProps) {
   const { height, width, formData, queriesData } = chartProps;
   const { horizonColorScale, seriesHeight } = formData;

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/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;

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]

Reply via email to