codeant-ai-for-open-source[bot] commented on code in PR #40577:
URL: https://github.com/apache/superset/pull/40577#discussion_r3333845674
##########
superset-frontend/plugins/plugin-chart-echarts/src/Bubble/transformProps.ts:
##########
@@ -213,7 +213,8 @@ export default function transformProps(chartProps:
EchartsBubbleChartProps) {
type: 'dashed',
},
},
- interval: xAxisLabelInterval,
+ interval:
+ xAxisLabelInterval === '0' ? 0 : xAxisLabelInterval,
Review Comment:
**Suggestion:** `interval` is being added at the `xAxis` level instead of
inside `xAxis.axisLabel`, so this does not control label density like the
control intends. On Bubble charts this ends up wiring the `"auto"` string into
axis tick interval config (numeric axis), which is an API mismatch and can
leave the "All" label option ineffective or produce incorrect tick behavior.
Move this setting under `axisLabel.interval` (or avoid applying it for
value/log axes if unsupported). [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Bubble chart X-axis label interval control has no effect.
- ⚠️ Bubble chart may send invalid interval config to ECharts.
- ⚠️ Inconsistent behavior versus Timeseries and MixedTimeseries charts.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open or create a Bubble chart (ECharts Bubble plugin defined in
`superset-frontend/plugins/plugin-chart-echarts/src/Bubble/index.ts:33-66`)
in the
Superset Explore view so that `EchartsBubbleChartPlugin` is used with
`transformProps`
from `Bubble/transformProps.ts`.
2. In the Bubble chart control panel, under the "X Axis" section
(`superset-frontend/plugins/plugin-chart-echarts/src/Bubble/controlPanel.tsx:121-138`),
change "X Axis Label Interval" from the default "Auto" to "All"; this uses
the shared
`xAxisLabelInterval` control from
`superset-frontend/plugins/plugin-chart-echarts/src/controls.tsx:23-37`,
which sets
`formData.xAxisLabelInterval` to the string `'0'`.
3. When the chart runs, `transformProps` destructures `xAxisLabelInterval`
from `formData`
(`superset-frontend/plugins/plugin-chart-echarts/src/Bubble/transformProps.ts:110-16`)
and
builds `echartOptions` such that `xAxis` is defined with `axisLabel: {
formatter, rotate:
xAxisLabelRotation }` and a top-level `interval` field (`transformProps.ts`
around lines
203-217) set to `xAxisLabelInterval === '0' ? 0 : xAxisLabelInterval`.
4. The resulting ECharts option has `xAxis.interval` receiving either
`'auto'` or `0` on a
value/log axis (Bubble `xAxisType` is `AxisType.Log` or `AxisType.Value` at
`transformProps.ts:27`), while `axisLabel.interval` remains unset; ECharts
expects label
density to be controlled via `xAxis.axisLabel.interval` (as done correctly
in Timeseries
and MixedTimeseries at
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:887-911`
and `src/MixedTimeseries/transformProps.ts:12-21`), so on Bubble charts the
"X Axis Label
Interval" control no longer affects label spacing and the `'auto'` string is
wired into an
unsupported `xAxis.interval` property, leading to the "All" option being
ineffective and
potentially incorrect tick behavior.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d66cea51d55a4f1285bbe6312949ddec&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=d66cea51d55a4f1285bbe6312949ddec&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/plugin-chart-echarts/src/Bubble/transformProps.ts
**Line:** 216:217
**Comment:**
*Api Mismatch: `interval` is being added at the `xAxis` level instead
of inside `xAxis.axisLabel`, so this does not control label density like the
control intends. On Bubble charts this ends up wiring the `"auto"` string into
axis tick interval config (numeric axis), which is an API mismatch and can
leave the "All" label option ineffective or produce incorrect tick behavior.
Move this setting under `axisLabel.interval` (or avoid applying it for
value/log axes if unsupported).
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%2F40577&comment_hash=3a9f61868208c5537443c113cf4f80dc13b74a4103fafbf842f2f0cd469cbe99&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40577&comment_hash=3a9f61868208c5537443c113cf4f80dc13b74a4103fafbf842f2f0cd469cbe99&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]
