codeant-ai-for-open-source[bot] commented on code in PR #35859:
URL: https://github.com/apache/superset/pull/35859#discussion_r3465109132
##########
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];
+
+ return {
+ dataMask: {
+ extraFormData: {
+ filters: values.length
+ ? [{ col: entity, op: 'IN', val: values }]
+ : [],
+ },
+ filterState: {
+ value: values.length ? values : null,
+ selectedValues: values.length ? values : null,
+ },
+ },
+ isCurrentValueSelected: isSelected,
+ };
};
- backgroundRect.on('click', clicked);
+ // Handle right-click context menu
+ const handleContextMenu = (feature: GeoFeature): void => {
+ const pointerEvent = d3.event;
+
+ if (typeof onContextMenu === 'function') {
+ pointerEvent?.preventDefault();
+ }
+
+ const iso = feature?.properties?.ISO;
+ if (!iso || typeof onContextMenu !== 'function' || !entity) return;
+
+ const drillVal = iso;
+ const drillToDetailFilters = [
+ { col: entity, op: '==', val: drillVal, formattedVal: drillVal },
+ ];
+ const drillByFilters = [{ col: entity, op: '==', val: drillVal }];
+
+ onContextMenu(pointerEvent.clientX, pointerEvent.clientY, {
+ drillToDetail: drillToDetailFilters,
+ crossFilter: getCrossFilterDataMask(feature),
+ drillBy: { filters: drillByFilters, groupbyFieldName: entity },
Review Comment:
**Suggestion:** `groupbyFieldName` in drill-by payload must be the form-data
control key, not the selected column value. Passing `entity` here sends values
like `"country_code"` instead of `"entity"`, so drill-by cannot map the
selection back to the chart control and the action can fail or apply
incorrectly. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Country Map drill-by fails to update groupby control.
- ⚠️ Drill-by submenu appears but produces incorrect drilled charts.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a Country Map chart in Explore using the "ISO 3166-2 Codes"
control named
`entity`
(`superset-frontend/plugins/legacy-plugin-chart-country-map/src/controlPanel.ts:29-49`),
e.g. select a column `country_code`.
2. When the chart renders, `transformProps` passes `formData.entity` into
the `entity`
prop along with hooks
(`superset-frontend/plugins/legacy-plugin-chart-country-map/src/transformProps.ts:21-41,62-79`),
so inside `CountryMap` the `entity` variable holds the selected column name
like
`"country_code"`.
3. Right-click a region on the map; the D3 handler `handleContextMenu` in
`CountryMap.ts`
builds `drillByFilters` and calls `onContextMenu(pointerEvent.clientX,
pointerEvent.clientY, { drillToDetail, crossFilter, drillBy: { filters:
drillByFilters,
groupbyFieldName: entity } })` (`CountryMap.ts:196-218`), so
`groupbyFieldName` is set to
the column name (`"country_code"`) instead of the form control key
(`"entity"`).
4. The Chart layer receives these filters in
`ChartRenderer.handleOnContextMenu`
(`src/components/Chart/ChartRenderer.tsx:320-337`), forwards them to
`ChartContextMenu`,
which passes `filters.drillBy` into `DrillByModal` via `DrillBySubmenu`
(`ChartContextMenu.tsx:340-41). In
`DrillByModal.getFormDataChangesFromConfigs`,
`config.groupbyFieldName` is used as a key into `formData`
(`DrillByModal.tsx:220-36`);
because `groupbyFieldName` is `"country_code"` (a column label) and not
`"entity"` (the
real field name), `formData[config.groupbyFieldName]` is undefined and the
updated
formData gets a new `"country_code"` property instead of updating
`formData.entity`. This
breaks drill-by for Country Map (the new chart does not drill on the
selected dimension),
whereas other charts correctly pass control keys like `'groupby'`
(`plugin-chart-echarts/src/utils/eventHandlers.ts:87-92`,
`Timeseries/EchartsTimeseries.tsx:320-15`,
`Treemap/EchartsTreemap.tsx:140-13`,
`Graph/EchartsGraph.tsx:151-17`).
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=03cf95bca42146509f1c093bf8678320&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=03cf95bca42146509f1c093bf8678320&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/plugins/legacy-plugin-chart-country-map/src/CountryMap.ts
**Line:** 216:216
**Comment:**
*Api Mismatch: `groupbyFieldName` in drill-by payload must be the
form-data control key, not the selected column value. Passing `entity` here
sends values like `"country_code"` instead of `"entity"`, so drill-by cannot
map the selection back to the chart control and the action can fail or apply
incorrectly.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F35859&comment_hash=dc63b8aaad0008755145ba96065b1e493878def0a14663fa75dda9d1db8ee00e&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F35859&comment_hash=dc63b8aaad0008755145ba96065b1e493878def0a14663fa75dda9d1db8ee00e&reaction=dislike'>👎</a>
--
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]
