codeant-ai-for-open-source[bot] commented on code in PR #43669:
URL: https://github.com/apache/superset/pull/43669#discussion_r3885616587
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -1205,46 +1207,24 @@ export default function transformProps(
// When showMaxLabel is true, ECharts may render a label at the axis
// boundary that formats identically to the last data-point tick (e.g.
- // "2005" appears twice with Year grain). Wrap the formatter to suppress
- // consecutive duplicate labels.
+ // "2005" appears twice with Year grain), and hideOverlap must stay off so
+ // that forced boundary label is never suppressed (#39899). Wrap the
+ // formatter to suppress consecutive duplicate labels and to thin out
+ // labels that would otherwise visually collide, since hideOverlap can no
+ // longer do that for us.
const showMaxLabel =
xAxisType === AxisType.Time &&
xAxisLabelRotation === 0 &&
!!resolvedTimeGrain;
const deduplicatedFormatter = showMaxLabel
- ? (() => {
- let lastLabel: string | undefined;
- let lastValue: number | undefined;
- const wrapper = (value: number | string) => {
- // ECharts formats the labels in repeated ascending passes. Reset the
- // dedup state when the sequence restarts so a forced boundary label
- // (e.g. the min date) isn't blanked by the previous pass's last
label
- // when both format identically (e.g. a May-to-May range).
- if (
- typeof value === 'number' &&
- lastValue !== undefined &&
- value <= lastValue
- ) {
- lastLabel = undefined;
- }
- if (typeof value === 'number') {
- lastValue = value;
- }
- const label =
- typeof xAxisFormatter === 'function'
- ? (xAxisFormatter as Function)(value)
- : String(value);
- if (label === lastLabel) {
- return '';
- }
- lastLabel = label;
- return label;
- };
- if (typeof xAxisFormatter === 'function' && 'id' in xAxisFormatter) {
- (wrapper as any).id = (xAxisFormatter as any).id;
- }
- return wrapper;
- })()
+ ? createSpacedXAxisFormatter(
+ xAxisFormatter,
+ ...getXAxisDomain(
+ [rebasedData as Record<string, unknown>[]],
+ xAxisLabel,
+ ),
+ Math.max(width - 2 * TIMESERIES_CONSTANTS.gridOffsetLeft, 0),
+ )
Review Comment:
**Suggestion:** The spacing calculation assumes the plot width is the full
chart width minus only the fixed 20px left and right offsets. The actual
ECharts plot is further reduced by y-axis title margins, legend placement, and
other padding, so this overestimates available label space and can leave labels
visually overlapping when `hideOverlap` is disabled. Base the calculation on
the effective grid width. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Dense labels can overlap with left-side axis titles.
- ⚠️ Timeseries charts use incorrect available x-axis width.
- ⚠️ MixedTimeseries has the same approximation at lines 672-682.
```
</details>
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=e2bfd9bbdcb74104b28fd39aca5b6cde&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=e2bfd9bbdcb74104b28fd39aca5b6cde&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<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/transformProps.ts
**Line:** 1219:1227
**Comment:**
*Logic Error: The spacing calculation assumes the plot width is the
full chart width minus only the fixed 20px left and right offsets. The actual
ECharts plot is further reduced by y-axis title margins, legend placement, and
other padding, so this overestimates available label space and can leave labels
visually overlapping when `hideOverlap` is disabled. Base the calculation on
the effective grid width.
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%2F43669&comment_hash=513d92a0a1fb761d2d6f6913ae6634d7f5ab7bca4d5b6b1e868feb0782e8714d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43669&comment_hash=513d92a0a1fb761d2d6f6913ae6634d7f5ab7bca4d5b6b1e868feb0782e8714d&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]
