codeant-ai-for-open-source[bot] commented on code in PR #38695:
URL: https://github.com/apache/superset/pull/38695#discussion_r3555272789
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts:
##########
@@ -416,7 +428,12 @@ export function transformSeries(
symbolSize: markerSize,
label: {
show: !!showValue,
- position: isHorizontal ? 'right' : 'top',
+ position: (labelPosition === 'auto' || !labelPosition
+ ? isHorizontal
+ ? 'right'
+ : 'top'
+ : labelPosition) as any,
Review Comment:
**Suggestion:** The new auto-position fallback for horizontal series is set
to `right`, which places labels outside the bar and does not preserve the
intended horizontal default behavior for bar value labels. This breaks the
expected “Auto” semantics and can reintroduce confusing label placement in
existing horizontal charts. Use the horizontal auto default that matches the
intended prior behavior instead of forcing `right`. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
⚠️ Horizontal timeseries bar labels move outside bars unexpectedly.
⚠️ Mixed timeseries auto labels inconsistent with prior horizontal default.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Note the default labelPosition for Timeseries is set to 'auto' in
`DEFAULT_FORM_DATA`
at
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/constants.ts:50`,
and
`orientation` defaults to `Vertical` at line 53 but can be changed to
`Horizontal` via
form data.
2. In
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:220-279`,
form data is destructured to `labelPosition` and `orientation`, and
`isHorizontal` is
computed as `orientation === OrientationType.Horizontal` at line 43; these
values are then
passed into `transformSeries` at lines 46-84 with `labelPosition` forwarded
in the `opts`
object.
3. In
`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts:72-109`,
`transformSeries` receives `isHorizontal` (defaulting to false unless the
chart is
horizontal) and `labelPosition` (defaulting to 'auto'). Later, at lines
300-307, the
series label configuration is built: `position: (labelPosition === 'auto' ||
!labelPosition ? isHorizontal ? 'right' : 'top' : labelPosition) as any`, so
when
`labelPosition` is 'auto' and `isHorizontal` is true, the computed label
position becomes
`'right'`.
4. When a horizontal timeseries bar chart is rendered with "Show Values"
enabled and the
new "Label Position" control left at its default `Auto` (so `labelPosition =
'auto'`), the
chart options generated by `transformProps` and `transformSeries` include
`label.position:
'right'` for horizontal series instead of the prior horizontal default
(`inside` as
described in the PR), causing existing horizontal bar charts that relied on
orientation-aware defaults to now display labels outside the bars, breaking
the intended
"Auto preserves previous behavior" semantics. The same call pattern in
`superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:180-219`
and 23-59 passes `labelPosition` into `transformSeries`, so Mixed Timeseries
horizontal
bar series with Auto also get `right` rather than the previous inside
behavior.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=834cddb359a14780939744e652941c5f&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=834cddb359a14780939744e652941c5f&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:** 431:435
**Comment:**
*Logic Error: The new auto-position fallback for horizontal series is
set to `right`, which places labels outside the bar and does not preserve the
intended horizontal default behavior for bar value labels. This breaks the
expected “Auto” semantics and can reintroduce confusing label placement in
existing horizontal charts. Use the horizontal auto default that matches the
intended prior behavior instead of forcing `right`.
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=4d44e773ca7341c745089039a74105ca1ac1dea86e4eae9eebf12a0f059901e1&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38695&comment_hash=4d44e773ca7341c745089039a74105ca1ac1dea86e4eae9eebf12a0f059901e1&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]
