rusackas commented on code in PR #37625:
URL: https://github.com/apache/superset/pull/37625#discussion_r2758097414
##########
superset-frontend/plugins/legacy-plugin-chart-country-map/src/CountryMap.ts:
##########
@@ -213,8 +228,8 @@ function CountryMap(element, props) {
if (map) {
drawMap(map);
} else {
- const url = countries[country];
- d3.json(url, (error, mapData) => {
+ const url = (countries as Record<string, string>)[country];
+ d3.json(url, (error: unknown, mapData: GeoData) => {
if (error) {
Review Comment:
Fixed in 79e7d816cc. Added a check for undefined URL before calling d3.json.
If the country isn't found in the countries map, we now display a user-friendly
error message instead of making a failed request.
##########
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:
Fixed in 79e7d816cc. Added proper undefined guard for d3Extent when
`colorScale === 'overall'`. Only sets yDomain if both rawExtent[0] and
rawExtent[1] are defined, otherwise leaves it undefined which lets HorizonRow
use its own default domain.
##########
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:
Fixed in 79e7d816cc. Changed from incorrect camelCase (`horizonColorScale`,
`seriesHeight`) to correct snake_case (`horizon_color_scale`, `series_height`)
to match the control panel form data keys.
##########
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:
Fixed in 79e7d816cc. Made `clusterer` a required prop since it's always
dereferenced with getClusters(). Removed the optional marker from the interface.
--
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]
