codeant-ai-for-open-source[bot] commented on code in PR #43299:
URL: https://github.com/apache/superset/pull/43299#discussion_r3803113836
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformers.ts:
##########
@@ -795,8 +795,11 @@ export function getPadding(
zoomable && !isHorizontal
? TIMESERIES_CONSTANTS.gridOffsetBottomZoomable + xAxisOffset
: TIMESERIES_CONSTANTS.gridOffsetBottom + xAxisOffset,
+ // The title margin is only reserved when a Y-axis title is actually
+ // rendered. Without this guard every chart pays for the default
+ // margin, which eats a large share of the plot area on narrow charts.
left:
- yAxisTitlePosition === 'Left'
+ yAxisTitlePosition === 'Left' && addYAxisTitleOffset
? TIMESERIES_CONSTANTS.gridOffsetLeft +
(Number(yAxisTitleMargin) || 0)
: TIMESERIES_CONSTANTS.gridOffsetLeft,
Review Comment:
**Suggestion:** The new comment claims the title margin is reserved only
when a title is rendered, but the `top` branch still adds `yAxisTitleMargin`
whenever `yAxisTitlePosition` is `Top`, even when `addYAxisTitleOffset` is
false. Timeseries and MixedTimeseries therefore continue reserving the default
top margin for charts without a Y-axis title. Gate the top margin with the same
rendered-title condition, or narrow the comment so it only describes the left
branch. [comment mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Timeseries charts waste top plot space with empty titles.
- ⚠️ MixedTimeseries charts retain unnecessary top padding.
- ⚠️ Narrow charts lose usable vertical plotting area.
```
</details>
[](https://github.com/CodeAnt-AI/skills/blob/main/skills/codeant-resolve-pr-comments/SKILL.md)
<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:** 798:805
**Comment:**
*Comment Mismatch: The new comment claims the title margin is reserved
only when a title is rendered, but the `top` branch still adds
`yAxisTitleMargin` whenever `yAxisTitlePosition` is `Top`, even when
`addYAxisTitleOffset` is false. Timeseries and MixedTimeseries therefore
continue reserving the default top margin for charts without a Y-axis title.
Gate the top margin with the same rendered-title condition, or narrow the
comment so it only describes the left branch.
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%2F43299&comment_hash=67a2d01568cc0674d03aa50813e7d3a15b992459e36cbc4bf8a03fa7c3a2a7c3&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43299&comment_hash=67a2d01568cc0674d03aa50813e7d3a15b992459e36cbc4bf8a03fa7c3a2a7c3&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]
