rusackas commented on code in PR #35859:
URL: https://github.com/apache/superset/pull/35859#discussion_r3455865773
##########
superset-frontend/plugins/legacy-plugin-chart-country-map/src/CountryMap.ts:
##########
@@ -175,34 +239,134 @@ function CountryMap(element: HTMLElement, props:
CountryMapProps) {
.classed('popup-at-bottom', y > (svgHeight * 2) / 3);
};
- const mouseenter = function mouseenter(this: SVGPathElement, d: GeoFeature) {
+ const mouseenter = function mouseenter(
+ this: SVGPathElement,
+ d: GeoFeature,
+ ): void {
// Darken color
let c: string = colorFn(d);
- if (c !== 'none') {
+ if (c) {
c = d3.rgb(c).darker().toString();
}
d3.select(this).style('fill', c);
- // Display information popup
- const result = data.filter(
- region => region.country_id === d.properties.ISO,
- );
- hoverPopup.style('display', 'block').html(
- `<div><strong>${getNameOfRegion(d)}</strong><br>${result.length > 0 ?
formatter(result[0].metric) : ''}</div>`,
- );
+ // Display information popup
+ const result = data.filter(r => r.country_id === d?.properties?.ISO);
+ hoverPopup
+ .style('display', 'block')
+ .html(
+ `<div><strong>${getNameOfRegion(d)}</strong><br>${result.length > 0 ?
formatter(result[0].metric) : ''}</div>`,
+ );
Review Comment:
Good catch, pushed a fix for this (451b9dbfc3) - escaped the region name and
the value through the `escapeHtml` helper that's already used elsewhere in this
file. The country geojsons are bundled assets so the real-world risk is low,
but no reason not to escape it.
##########
superset-frontend/plugins/legacy-plugin-chart-country-map/src/CountryMap.ts:
##########
@@ -112,46 +146,76 @@ function CountryMap(element: HTMLElement, props:
CountryMapProps) {
.attr('width', width)
.attr('height', height)
.attr('preserveAspectRatio', 'xMidYMid meet');
+
+ // Only set grab cursor if not in edit mode
+ if (!isEditMode) {
+ svg.style('cursor', 'grab');
+ }
const backgroundRect = svg
.append('rect')
.attr('class', 'background')
.attr('width', width)
.attr('height', height);
const g = svg.append('g');
const mapLayer = g.append('g').classed('map-layer', true);
+ // Add hover popup for tooltip
const hoverPopup = div.append('div').attr('class', 'hover-popup');
- let centered: GeoFeature | null;
-
- const clicked = function clicked(d: GeoFeature) {
- const hasCenter = d && centered !== d;
- let x: number;
- let y: number;
- let k: number;
- const halfWidth = width / 2;
- const halfHeight = height / 2;
-
- if (hasCenter) {
- const centroid = path.centroid(d);
- [x, y] = centroid;
- k = 4;
- centered = d;
- } else {
- x = halfWidth;
- y = halfHeight;
- k = 1;
- centered = null;
- }
+ // Track mouse position to distinguish clicks from drags
+ let mousedownPos: { x: number; y: number } | null = null;
- g.transition()
- .duration(750)
- .attr(
- 'transform',
-
`translate(${halfWidth},${halfHeight})scale(${k})translate(${-x},${-y})`,
- );
+ // Cross-filter support
+ const getCrossFilterDataMask = (
+ source: GeoFeature,
+ ): { dataMask: DataMask; isCurrentValueSelected: boolean } | undefined => {
+ if (!entity) return undefined;
+
+ const selected = filterState?.selectedValues || [];
+ const iso = source?.properties?.ISO;
+ if (!iso) return undefined;
+
+ const isSelected = selected.includes(iso);
+ const values = isSelected ? [] : [iso];
Review Comment:
I think this one is working as intended... there's no
Shift+Click/multi-select code in here (the author noted in the description
they'd drop it for consistency with the other charts), so this is plain
single-select cross-filtering: clicking a region sets the filter, clicking it
again clears it. The collapse to one-or-none is the desired behavior here
rather than a regression.
--
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]