codeant-ai-for-open-source[bot] commented on code in PR #38695:
URL: https://github.com/apache/superset/pull/38695#discussion_r3531220603
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts:
##########
@@ -416,7 +419,13 @@ export function transformSeries(
symbolSize: markerSize,
label: {
show: !!showValue,
- position: isHorizontal ? 'right' : 'top',
+ position:
+ labelPosition === 'auto' || !labelPosition
+ ? isHorizontal
+ ? 'inside'
+ : 'top'
+ : labelPosition,
+ overflow: 'truncate',
Review Comment:
**Suggestion:** The new configurable label position is applied only at
series level, but negative bar points are still force-overridden to `outside`
by `transformNegativeLabelsPosition`, so user-selected positions (for example
`inside`) are silently ignored on negative values. Respect explicit non-`auto`
selections by skipping the negative-value override (or by making that helper
use the selected position) when a manual label position is provided.
[incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Timeseries bar charts ignore manual label position on negatives.
- ⚠️ Mixed timeseries bars show inconsistent labels for negative values.
- ⚠️ New label-position control appears unreliable for negative data.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. In the frontend, the user selects a manual label position (e.g. `Inside`)
in the UI
control defined at
`superset-frontend/plugins/plugin-chart-echarts/src/controls.tsx:143-152`
(`labelPositionControl` with `name: 'label_position'`), which is visible
only when
`show_value` is enabled (visibility at `controls.tsx:21-22`).
2. That control populates `formData.labelPosition` via the
`EchartsTimeseriesFormData`
type in
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/types.ts:54-60,108-110`,
and `transformProps` reads `labelPosition` from formData and passes it into
`transformSeries` at
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:239-243`
(destructuring) and `transformProps.ts:505-511` (opts object with `showValue,
labelPosition, onlyTotal, ...`).
3. Inside `transformSeries` in
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts`,
the
series-level label position is set from that `labelPosition` at
`transformers.ts:421-428`,
where `label.position` is computed as `labelPosition === 'auto' ||
!labelPosition ?
(isHorizontal ? 'inside' : 'top') : labelPosition`, so any non-`auto`
selection (e.g.
`'inside'`) is intended to be used for all points.
4. For unstacked bar series with negative values, the same `transformSeries`
call rewrites
the `data` array via `transformNegativeLabelsPosition(series, isHorizontal)`
at
`transformers.ts:45-47,385-387`, and that helper
(`transformNegativeLabelsPosition`
defined at `transformers.ts:146-31`) maps each negative-value point to an
object with
`label: { position: 'outside' }`, which in ECharts overrides the series-level
`label.position`; as a result, for any Timeseries or MixedTimeseries bar
chart with
negative values and `show_value=true`, manually choosing `Inside` (or any
non-`auto`
position) is silently ignored on negative bars, which continue to render with
`label.position: 'outside'`.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=05a93a1795b84eceb131bb91ab9bda47&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=05a93a1795b84eceb131bb91ab9bda47&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/Timeseries/transformers.ts
**Line:** 423:428
**Comment:**
*Incomplete Implementation: The new configurable label position is
applied only at series level, but negative bar points are still
force-overridden to `outside` by `transformNegativeLabelsPosition`, so
user-selected positions (for example `inside`) are silently ignored on negative
values. Respect explicit non-`auto` selections by skipping the negative-value
override (or by making that helper use the selected position) when a manual
label position is provided.
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%2F38695&comment_hash=8dee4f336ed9a217d53198dac60c281dc96a4bfcfd0514c9711c688a5241ec7f&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38695&comment_hash=8dee4f336ed9a217d53198dac60c281dc96a4bfcfd0514c9711c688a5241ec7f&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]
