codeant-ai-for-open-source[bot] commented on code in PR #40181:
URL: https://github.com/apache/superset/pull/40181#discussion_r3252402188
##########
superset-frontend/plugins/plugin-chart-echarts/src/Treemap/constants.ts:
##########
@@ -21,8 +21,8 @@ import { TreePathInfo } from '../types';
export const COLOR_SATURATION = [0.7, 0.4];
export const LABEL_FONTSIZE = 11;
-export const BORDER_WIDTH = 2;
-export const GAP_WIDTH = 2;
+export const BORDER_WIDTH = 0;
+export const GAP_WIDTH = 0;
Review Comment:
**Suggestion:** Setting global treemap border/gap constants to zero only
fixes the default node style, but filtered (non-selected) leaf nodes still get
a hardcoded `borderWidth: 2` in `transformProps`, so gaps reappear during
cross-filter interactions. Apply the same zero-gap policy in that filtered-node
branch as well (and set `gapWidth` there) to keep tiles flush in all states.
[incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Treemap cross-filtered leaves show gaps despite zero-gap constants.
- ⚠️ Visual inconsistency between default and filtered treemap node styles.
- ⚠️ Undercuts PR goal of removing treemap node spacing.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Render a Treemap chart using the ECharts Treemap plugin, which wires
`transformProps`
from
`superset-frontend/plugins/plugin-chart-echarts/src/Treemap/transformProps.ts:107-243`
via
the plugin registration in
`superset-frontend/plugins/plugin-chart-echarts/src/Treemap/index.ts:34-82`.
2. Observe that for the root node and all normal child nodes,
`transformProps` constructs
`itemStyle` with `borderWidth: BORDER_WIDTH` and `gapWidth: GAP_WIDTH` (see
`transformProps.ts:187-196` for leaf defaults and
`transformProps.ts:228-237` for the root
node), and `BORDER_WIDTH`/`GAP_WIDTH` are both `0` as defined in
`superset-frontend/plugins/plugin-chart-echarts/src/Treemap/constants.ts:22-25`,
so tiles
render flush with no gaps in the default state.
3. Trigger a cross-filtered or externally-filtered state so that
`filterState.selectedValues` is populated and does not include at least one
leaf's
`joinedName` key (built at `transformProps.ts:186-205`); for such
non-selected leaves,
execution enters the filtered branch at `transformProps.ts:207-218`, where
`itemStyle` is
overridden to `{ colorAlpha: OpacityEnum.SemiTransparent, color:
theme.colorText,
borderColor: theme.colorBgBase, borderWidth: 2 }` without applying
`gapWidth`.
4. Inspect the rendered treemap (or log the produced ECharts option) and
note that
filtered, non-selected leaf nodes now have `borderWidth: 2` while other
nodes use
`borderWidth: 0` and `gapWidth: 0`, causing visible gaps/borders to reappear
around those
filtered tiles despite the global zero-gap constants in
`constants.ts:24-25`, confirming
that the zero-gap policy is not applied consistently across all rendering
paths.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fplugins%2Fplugin-chart-echarts%2Fsrc%2FTreemap%2Fconstants.ts%0A%2A%2ALine%3A%2A%2A%2024%3A25%0A%2A%2AComment%3A%2A%2A%0A%09%2AIncomplete%20Implementation%3A%20Setting%20global%20treemap%20border%2Fgap%20constants%20to%20zero%20only%20fixes%20the%20default%20node%20style%2C%20but%20filtered%20%28non-selected%29%20leaf%20nodes%20still%20get%20a%20hardcoded%20%60borderWidth%3A%202%60%20in%20%60transformProps%60%2C%20so%20gaps%20reappear%20during%20cross-filter%20interactions.%20Apply%20the%20same%20zero-gap%20policy%20in%20that%20filtered-node%20branch%20as%20well%20%28and%20set%20%60gapWidth%60%20there%29%20to%20keep%20tiles%20flush%20in%20all%20states.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20imp
lement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20superset-frontend%2Fplugins%2Fplugin-chart-echarts%2Fsrc%2FTreemap%2Fconstants.ts%0A%2A%2ALine%3A%2A%2A%2024%3A25%0A%2A%2AComment%3A%2A%2A%0A%09%2AIncomplete%20Implementation%3A%20Setting%20global%20treemap%20border%2Fgap%20constants%20to%20zero%20only%20fixes%20the%20default%20node%20style%2C%20but%20filtered%20%28non-selected%29%20leaf%20nodes%20still%20get%20a%20hardcoded%20%60borderWidth%3A%202%60%20in%20%60transformProps%60%2C%20so%20
gaps%20reappear%20during%20cross-filter%20interactions.%20Apply%20the%20same%20zero-gap%20policy%20in%20that%20filtered-node%20branch%20as%20well%20%28and%20set%20%60gapWidth%60%20there%29%20to%20keep%20tiles%20flush%20in%20all%20states.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
*(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/plugin-chart-echarts/src/Treemap/constants.ts
**Line:** 24:25
**Comment:**
*Incomplete Implementation: Setting global treemap border/gap constants
to zero only fixes the default node style, but filtered (non-selected) leaf
nodes still get a hardcoded `borderWidth: 2` in `transformProps`, so gaps
reappear during cross-filter interactions. Apply the same zero-gap policy in
that filtered-node branch as well (and set `gapWidth` there) to keep tiles
flush in all states.
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%2F40181&comment_hash=199e23c70d79bcf85ebc522fc536e38f1c1bbb75f17c3dfc8a137f0a6a83dedc&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40181&comment_hash=199e23c70d79bcf85ebc522fc536e38f1c1bbb75f17c3dfc8a137f0a6a83dedc&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]
