codeant-ai-for-open-source[bot] commented on code in PR #37105:
URL: https://github.com/apache/superset/pull/37105#discussion_r2687405086
##########
superset-frontend/plugins/legacy-plugin-chart-country-map/src/ReactCountryMap.jsx:
##########
@@ -70,5 +70,10 @@ export default styled(CountryMap)`
cursor: pointer;
stroke: ${theme.colorSplit};
}
+
+ .hover-popup.popup-at-bottom {
Review Comment:
**Suggestion:** Selector is not namespaced to the map container — the rule
".hover-popup.popup-at-bottom" can match elements outside this component or
conflict with other components; scope it the same way other rules are (prefix
with the map root class) to avoid leakage and specificity surprises. [possible
bug]
**Severity Level:** Critical 🚨
```suggestion
.superset-legacy-chart-country-map .hover-popup.popup-at-bottom {
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The current rule is global within this styled block and could match tooltips
outside this component.
Prefixing with the map root class is consistent with the rest of the file
(see other rules using
`.superset-legacy-chart-country-map .hover-popup`) and prevents style
leakage or accidental overrides.
The improved change is minimal, correct, and safe.
</details>
<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/ReactCountryMap.jsx
**Line:** 74:74
**Comment:**
*Possible Bug: Selector is not namespaced to the map container — the
rule ".hover-popup.popup-at-bottom" can match elements outside this component
or conflict with other components; scope it the same way other rules are
(prefix with the map root class) to avoid leakage and specificity surprises.
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.
```
</details>
##########
superset-frontend/plugins/legacy-plugin-chart-country-map/src/CountryMap.js:
##########
@@ -134,6 +134,16 @@ function CountryMap(element, props) {
return '';
};
+ const svgHeight = svg.node().getBoundingClientRect().height;
+ const updatePopupPosition = () => {
Review Comment:
**Suggestion:** Logic bug: `svgHeight` is computed once at initialization
and then reused for all subsequent pointer events; if the SVG/container is
resized (responsive layouts, zoom, or when the parent changes size) the
threshold used to decide "bottom third" becomes stale and the tooltip will be
mis-positioned. Move the computation of the SVG height inside
`updatePopupPosition` so the height is recomputed on each invocation. [logic
error]
**Severity Level:** Minor ⚠️
```suggestion
const updatePopupPosition = () => {
const svgHeight = svg.node().getBoundingClientRect().height;
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
This is a valid bug: svgHeight is captured once at init and can become stale
in responsive cases, making the bottom-third test incorrect. Recomputing height
inside updatePopupPosition (or on resize) fixes the logic without breaking
other behavior.
</details>
<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.js
**Line:** 137:138
**Comment:**
*Logic Error: Logic bug: `svgHeight` is computed once at initialization
and then reused for all subsequent pointer events; if the SVG/container is
resized (responsive layouts, zoom, or when the parent changes size) the
threshold used to decide "bottom third" becomes stale and the tooltip will be
mis-positioned. Move the computation of the SVG height inside
`updatePopupPosition` so the height is recomputed on each invocation.
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.
```
</details>
--
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]
