codeant-ai-for-open-source[bot] commented on code in PR #41350:
URL: https://github.com/apache/superset/pull/41350#discussion_r3578577494
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -594,17 +594,25 @@ export default function transformProps(
if (maxSecondary === undefined) maxSecondary = 1;
}
+ // A dashboard-level time grain override (e.g. via a filter or the temporal
+ // range control) is delivered in extraFormData and should take precedence
+ // over the chart's own time grain when formatting temporal axes/tooltips.
+ const resolvedTimeGrain =
+ formData.extraFormData?.time_grain_sqla ?? timeGrainSqla;
+
const tooltipFormatter =
xAxisDataType === GenericDataType.Temporal
- ? getTooltipTimeFormatter(tooltipTimeFormat)
+ ? getTooltipTimeFormatter(tooltipTimeFormat, resolvedTimeGrain)
: String;
const xAxisFormatter =
xAxisDataType === GenericDataType.Temporal
- ? getXAxisFormatter(xAxisTimeFormat, timeGrainSqla)
+ ? getXAxisFormatter(xAxisTimeFormat, resolvedTimeGrain)
: String;
const showMaxLabel =
- xAxisType === AxisType.Time && xAxisLabelRotation === 0 && !!timeGrainSqla;
+ xAxisType === AxisType.Time &&
+ xAxisLabelRotation === 0 &&
+ !!resolvedTimeGrain;
Review Comment:
**Suggestion:** The same `showMaxLabel` gating on `resolvedTimeGrain` in
mixed timeseries causes the max-label visibility fix to stop working when the
chart has a temporal axis but no explicit grain. This makes the last tick label
vulnerable to overlap suppression again; the condition should not depend on
grain availability. [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
❌ Mixed Chart time axes hide final tick label.
⚠️ Dashboard time-series readability degraded for no-grain charts.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Create a Mixed Chart using the ECharts MixedTimeseries plugin registered
in
`superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/index.ts:34-37`
(class
`EchartsTimeseriesChartPlugin` with `transformProps` wired in the
constructor at line 49).
2. Configure a temporal x-axis with `xAxisType === AxisType.Time` but no
time grain:
ensure `formData.timeGrainSqla` is unset and no
`extraFormData.time_grain_sqla` override
is applied, so `resolvedTimeGrain` computed at
`MixedTimeseries/transformProps.ts:600-602`
(`const resolvedTimeGrain = formData.extraFormData?.time_grain_sqla ??
timeGrainSqla;`)
evaluates to `undefined`.
3. When `transformProps()` runs, `showMaxLabel` is computed at
`MixedTimeseries/transformProps.ts:612-615` as `xAxisType === AxisType.Time
&&
xAxisLabelRotation === 0 && !!resolvedTimeGrain;`. With `resolvedTimeGrain`
falsy,
`showMaxLabel` becomes false, so `deduplicatedFormatter` falls back to
`xAxisFormatter`
and the axis config later applied at
`MixedTimeseries/transformProps.ts:718-721` omits the
`showMaxLabel: true` / `alignMaxLabel: 'right'` properties.
4. Render the chart and observe the x-axis on a non-rotated time axis: with
`hideOverlap`
still enabled per `MixedTimeseries/transformProps.ts:714-715` and no
`showMaxLabel`
safeguard, ECharts can again suppress the final tick label, reintroducing the
hidden/clipped last x-axis label behavior that the earlier max-label fix was
intended to
prevent. This regression only depends on the absence of a time grain, not on
any unusual
input.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2dd2488a03fd48fe9041999ee9882229&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=2dd2488a03fd48fe9041999ee9882229&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/MixedTimeseries/transformProps.ts
**Line:** 613:615
**Comment:**
*Incorrect Condition Logic: The same `showMaxLabel` gating on
`resolvedTimeGrain` in mixed timeseries causes the max-label visibility fix to
stop working when the chart has a temporal axis but no explicit grain. This
makes the last tick label vulnerable to overlap suppression again; the
condition should not depend on grain availability.
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%2F41350&comment_hash=82b877f0fe5d900a30cfeec35187412bd1c544ca5a599a0a5e50169d14de0b65&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41350&comment_hash=82b877f0fe5d900a30cfeec35187412bd1c544ca5a599a0a5e50169d14de0b65&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]
