You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "prathamesh04 (via GitHub)" <gi...@apache.org> on 2026/07/22 06:27:25 UTC
[PR] fix: last date label hidden on time series x-axis (#39899) [superset]
prathamesh04 opened a new pull request, #42299:
URL: https://github.com/apache/superset/pull/42299
## Description
When xAxisLabelRotation is 0 on a time axis, both `hideOverlap` and `showMaxLabel` were set, but ECharts' `hideOverlap: true` can still suppress the forced max label.
This fix sets `hideOverlap: false` when `showMaxLabel` is active so the last date label is never hidden by ECharts' overlap detection.
## Changes
- `transformProps.ts` (Timeseries): Set `hideOverlap: false` when `showMaxLabel` is true
- `transformProps.ts` (MixedTimeseries): Same fix applied
- `transformers.test.ts`: Updated existing test and added regression test for #39899
## How to test
1. Create a time series chart with many data points
2. Set x-axis label rotation to 0 (no rotation)
3. Observe that the last date label on the x-axis is now visible
4. Also verify labels don't overlap when there are many data points
## Screenshots
N/A
## Checklist
- [x] Tests pass
- [x] Lint passes
- [x] Regression test added
Fixes #39899
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #42299:
URL: https://github.com/apache/superset/pull/42299#issuecomment-5121397334
@prathamesh04 I left an open question on my last review:
> MixedTimeseries/transformProps.ts gets the same hideOverlap/showMaxLabel change but only Timeseries picked up a test for it, mind adding one there too?
Feel like tackling that now, while it's on the radar?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "prathamesh04 (via GitHub)" <gi...@apache.org>.
prathamesh04 commented on PR #42299:
URL: https://github.com/apache/superset/pull/42299#issuecomment-5125936333
Added a MixedTimeseries test at the end of `transformProps.test.ts` that asserts `showMaxLabel: true`, `alignMaxLabel: 'right'`, and `hideOverlap: false` on a time axis with 0° rotation, matching the Timeseries test you reviewed. Ready for re-review, thanks @rusackas!
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "prathamesh04 (via GitHub)" <gi...@apache.org>.
prathamesh04 commented on PR #42299:
URL: https://github.com/apache/superset/pull/42299#issuecomment-5043437796
cc @rusackas — ready for review. Added video testing in the PR description.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #42299:
URL: https://github.com/apache/superset/pull/42299#issuecomment-5045682709
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/42299?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `75.00000%` with `1 line` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 65.13%. Comparing base ([`bef03fb`](https://app.codecov.io/gh/apache/superset/commit/bef03fb8508338f3d73ff4588f9cc91d25221de1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`e5a32e8`](https://app.codecov.io/gh/apache/superset/commit/e5a32e8da4b7b991d5056ecae4562cc5bdaf1fa2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 137 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/42299?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...hart-echarts/src/MixedTimeseries/transformProps.ts](https://app.codecov.io/gh/apache/superset/pull/42299?src=pr&el=tree&filepath=superset-frontend%2Fplugins%2Fplugin-chart-echarts%2Fsrc%2FMixedTimeseries%2FtransformProps.ts&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGx1Z2lucy9wbHVnaW4tY2hhcnQtZWNoYXJ0cy9zcmMvTWl4ZWRUaW1lc2VyaWVzL3RyYW5zZm9ybVByb3BzLnRz) | 50.00% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/42299?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #42299 +/- ##
==========================================
- Coverage 65.13% 65.13% -0.01%
==========================================
Files 2753 2753
Lines 154584 154586 +2
Branches 35463 35465 +2
==========================================
+ Hits 100695 100696 +1
- Misses 51975 51976 +1
Partials 1914 1914
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/42299/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/42299/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `70.73% <75.00%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/42299?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42299:
URL: https://github.com/apache/superset/pull/42299#issuecomment-5173445202
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42299:
URL: https://github.com/apache/superset/pull/42299#issuecomment-5042628105
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42299:
URL: https://github.com/apache/superset/pull/42299#issuecomment-5044963913
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42299:
URL: https://github.com/apache/superset/pull/42299#issuecomment-5127308634
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #42299:
URL: https://github.com/apache/superset/pull/42299#discussion_r3708031726
##########
superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts:
##########
@@ -1013,3 +1020,421 @@ test('temporal x coltype wires the time formatter and Time axis', () => {
expect(typeof label).toBe('string');
expect(label).not.toMatch(/NaN/);
});
+
+test('tooltip time grain wiring: dashboard-level extraFormData time grain overrides the chart-level grain in the tooltip', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ // The chart itself is configured with a Day grain...
+ timeGrainSqla: TimeGranularity.DAY,
+ // ...but a dashboard-level filter/override resolves to Month.
+ extraFormData: { time_grain_sqla: TimeGranularity.MONTH },
+ },
+ queriesData: [temporalQueryData, temporalQueryData],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const tooltipFormatter = (echartOptions as unknown as TooltipFormatterOptions)
+ .tooltip.formatter;
+
+ const result = tooltipFormatter({
+ value: [ts, 100],
+ seriesName: 'metric',
+ });
+
+ // Month grain (the dashboard override) should win, so the tooltip title
+ // reads "Jan 2021" rather than the Day-grain "2021-01-07".
+ expect(result).toContain('Jan');
+ expect(result).toContain('2021');
+ expect(result).not.toContain('2021-01-07');
+});
+
+test('tooltip time grain wiring: chart-level time grain drives the tooltip when there is no dashboard override', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ timeGrainSqla: TimeGranularity.YEAR,
+ },
+ queriesData: [temporalQueryData, temporalQueryData],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const tooltipFormatter = (echartOptions as unknown as TooltipFormatterOptions)
+ .tooltip.formatter;
+
+ const result = tooltipFormatter({
+ value: [ts, 100],
+ seriesName: 'metric',
+ });
+
+ expect(result).toContain('2021');
+ expect(result).not.toContain('2021-01-07');
+});
+
+const createTemporalMixedChartProps = (timeFormat: string) => {
+ const rows = [
+ { __timestamp: Date.UTC(2003, 4, 1), metric: 10 },
+ { __timestamp: Date.UTC(2004, 0, 1), metric: 20 },
+ { __timestamp: Date.UTC(2005, 4, 1), metric: 30 },
+ ];
+ const q = createTestQueryData(rows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+ return createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [q, q],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ timeGrainSqla: TimeGranularity.MONTH,
+ xAxisTimeFormat: timeFormat,
+ },
+ queriesData: [q, q],
+ });
+};
+
+test('x-axis forces showMinLabel for time grains so the beginning date stays visible (mixed)', () => {
+ const xAxis = transformProps(createTemporalMixedChartProps('smart_date'))
+ .echartOptions.xAxis as any;
+ expect(xAxis.axisLabel.showMinLabel).toBe(true);
+});
+
+test('x-axis dedup keeps the forced min label when the endpoints format identically (mixed)', () => {
+ // May→May range renders "May" at both boundaries; the dedup must reset per
+ // ECharts pass so the forced min label survives the second pass.
+ const { formatter } = (
+ transformProps(createTemporalMixedChartProps('%b')).echartOptions
+ .xAxis as any
+ ).axisLabel;
+ const min = Date.UTC(2003, 4, 1);
+ const mid = Date.UTC(2004, 0, 1);
+ const max = Date.UTC(2005, 4, 1);
+
+ formatter(min);
+ formatter(mid);
+ formatter(max);
+
+ expect(formatter(min)).toBe('May');
+});
Review Comment:
The dedup wrapper resets lastLabel whenever value <= lastValue, so a new ascending pass (min again) clears the stale max-pass state before formatting. This test passes.
##########
superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts:
##########
@@ -1013,3 +1020,421 @@ test('temporal x coltype wires the time formatter and Time axis', () => {
expect(typeof label).toBe('string');
expect(label).not.toMatch(/NaN/);
});
+
+test('tooltip time grain wiring: dashboard-level extraFormData time grain overrides the chart-level grain in the tooltip', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ // The chart itself is configured with a Day grain...
+ timeGrainSqla: TimeGranularity.DAY,
+ // ...but a dashboard-level filter/override resolves to Month.
+ extraFormData: { time_grain_sqla: TimeGranularity.MONTH },
+ },
+ queriesData: [temporalQueryData, temporalQueryData],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const tooltipFormatter = (echartOptions as unknown as TooltipFormatterOptions)
+ .tooltip.formatter;
+
+ const result = tooltipFormatter({
+ value: [ts, 100],
+ seriesName: 'metric',
+ });
+
+ // Month grain (the dashboard override) should win, so the tooltip title
+ // reads "Jan 2021" rather than the Day-grain "2021-01-07".
+ expect(result).toContain('Jan');
+ expect(result).toContain('2021');
+ expect(result).not.toContain('2021-01-07');
+});
+
+test('tooltip time grain wiring: chart-level time grain drives the tooltip when there is no dashboard override', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ timeGrainSqla: TimeGranularity.YEAR,
+ },
+ queriesData: [temporalQueryData, temporalQueryData],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const tooltipFormatter = (echartOptions as unknown as TooltipFormatterOptions)
+ .tooltip.formatter;
+
+ const result = tooltipFormatter({
+ value: [ts, 100],
+ seriesName: 'metric',
+ });
+
+ expect(result).toContain('2021');
+ expect(result).not.toContain('2021-01-07');
+});
+
+const createTemporalMixedChartProps = (timeFormat: string) => {
+ const rows = [
+ { __timestamp: Date.UTC(2003, 4, 1), metric: 10 },
+ { __timestamp: Date.UTC(2004, 0, 1), metric: 20 },
+ { __timestamp: Date.UTC(2005, 4, 1), metric: 30 },
+ ];
+ const q = createTestQueryData(rows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+ return createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [q, q],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ timeGrainSqla: TimeGranularity.MONTH,
+ xAxisTimeFormat: timeFormat,
+ },
+ queriesData: [q, q],
+ });
+};
+
+test('x-axis forces showMinLabel for time grains so the beginning date stays visible (mixed)', () => {
+ const xAxis = transformProps(createTemporalMixedChartProps('smart_date'))
+ .echartOptions.xAxis as any;
+ expect(xAxis.axisLabel.showMinLabel).toBe(true);
+});
+
+test('x-axis dedup keeps the forced min label when the endpoints format identically (mixed)', () => {
+ // May→May range renders "May" at both boundaries; the dedup must reset per
+ // ECharts pass so the forced min label survives the second pass.
+ const { formatter } = (
+ transformProps(createTemporalMixedChartProps('%b')).echartOptions
+ .xAxis as any
+ ).axisLabel;
+ const min = Date.UTC(2003, 4, 1);
+ const mid = Date.UTC(2004, 0, 1);
+ const max = Date.UTC(2005, 4, 1);
+
+ formatter(min);
+ formatter(mid);
+ formatter(max);
+
+ expect(formatter(min)).toBe('May');
+});
+
+test('#39899 - x-axis dates do not overlap and last label stays visible at 0° rotation (mixed)', () => {
+ // When showMaxLabel is active on a time axis with 0° rotation,
+ // hideOverlap must be off so ECharts cannot suppress the forced
+ // max label (the end-of-axis date).
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [
+ createTestQueryData(
+ [
+ {
+ __timestamp: Date.UTC(2026, 0, 1),
+ sum__num: 100,
+ },
+ {
+ __timestamp: Date.UTC(2026, 6, 1),
+ sum__num: 200,
+ },
+ ],
+ {
+ colnames: ['__timestamp', 'sum__num'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], sum__num: ['sum__num'] },
+ },
+ ),
+ createTestQueryData(
+ [
+ {
+ __timestamp: Date.UTC(2026, 0, 1),
+ sum__num: 100,
+ },
+ {
+ __timestamp: Date.UTC(2026, 6, 1),
+ sum__num: 200,
+ },
+ ],
+ {
+ colnames: ['__timestamp', 'sum__num'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], sum__num: ['sum__num'] },
+ },
+ ),
+ ],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['sum__num'],
+ metricsB: ['sum__num'],
+ groupby: [],
+ groupbyB: [],
+ xAxisLabelRotation: 0,
+ },
+ queriesData: [
+ createTestQueryData(
+ [
+ {
+ __timestamp: Date.UTC(2026, 0, 1),
+ sum__num: 100,
+ },
+ {
+ __timestamp: Date.UTC(2026, 6, 1),
+ sum__num: 200,
+ },
+ ],
+ {
+ colnames: ['__timestamp', 'sum__num'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], sum__num: ['sum__num'] },
+ },
+ ),
+ createTestQueryData(
+ [
+ {
+ __timestamp: Date.UTC(2026, 0, 1),
+ sum__num: 100,
+ },
+ {
+ __timestamp: Date.UTC(2026, 6, 1),
+ sum__num: 200,
+ },
+ ],
+ {
+ colnames: ['__timestamp', 'sum__num'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], sum__num: ['sum__num'] },
+ },
+ ),
+ ],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const { axisLabel } = echartOptions.xAxis as Record<string, any>;
+
+ expect(axisLabel.showMaxLabel).toBe(true);
+ expect(axisLabel.alignMaxLabel).toBe('right');
+ expect(axisLabel.hideOverlap).toBe(false);
+});
+
+test('regression #37921: multi-metric Query A with groupby does not duplicate first metric in series names', () => {
+ // Regression test for https://github.com/apache/superset/issues/37921
+ // ("Residual" follow-up to #37055).
+ //
+ // When Query A has multiple metrics + at least one Group By dimension,
+ // the display-name builder in transformProps.ts used to prepend the FIRST
+ // metric's display name to every series that didn't literally contain it:
+ // name: `${MetricDisplayNameA}, ${entryName}`
+ // For series belonging to the *second* metric, this produced a
+ // cross-contaminated label like `score_one, score_two, A` — the
+ // user-visible "first metric duplicated" symptom in the legend / tooltip.
+ // The fix derives each series' metric from its label-map tuple instead.
+ const multiMetricRows = [
+ {
+ 'score_one, A': 1,
+ 'score_one, B': 2,
+ 'score_two, A': 3,
+ 'score_two, B': 4,
+ ds: 599616000000,
+ },
+ {
+ 'score_one, A': 5,
+ 'score_one, B': 6,
+ 'score_two, A': 7,
+ 'score_two, B': 8,
+ ds: 599916000000,
+ },
+ ];
+ const multiMetricLabelMap = {
+ ds: ['ds'],
+ 'score_one, A': ['score_one', 'A'],
+ 'score_one, B': ['score_one', 'B'],
+ 'score_two, A': ['score_two', 'A'],
+ 'score_two, B': ['score_two', 'B'],
+ };
+
+ const queryAData = createTestQueryData(multiMetricRows, {
+ label_map: multiMetricLabelMap,
+ });
+ // Query B keeps the existing single-metric shape — the bug is on
+ // Query A's path so we just need a valid Query B alongside.
+ const queryBData = createTestQueryData(defaultQueryRows, {
+ label_map: defaultLabelMap,
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [queryAData, queryBData],
+ formData: {
+ ...formData,
+ metrics: ['score_one', 'score_two'],
+ groupby: ['category'],
+ },
+ queriesData: [queryAData, queryBData],
+ });
+ const transformed = transformProps(chartProps);
+
+ const queryASeriesNames = (transformed.echartOptions.series as any[])
+ .map((s: any) => String(s.name))
+ .filter((n: string) => n.includes('score_'));
+
+ // Each (metric, dim_value) combo from Query A should appear exactly once
+ // with the *correct* metric prefix — not the first-metric-prepended-to-
+ // everything-else form. Comparing the sorted array (rather than using
+ // separate toContain assertions) also catches a regression that emits
+ // duplicate series for the same name.
+ expect([...queryASeriesNames].sort()).toEqual(
+ ['score_one, A', 'score_one, B', 'score_two, A', 'score_two, B'].sort(),
+ );
Review Comment:
The series-name builder derives each series' metric from its label-map tuple, not from the first metric globally, so Query A's score_two series correctly gets 'score_two, A' rather than the concatenated form. This test passes.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe merged PR #42299:
URL: https://github.com/apache/superset/pull/42299
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on PR #42299:
URL: https://github.com/apache/superset/pull/42299#issuecomment-5339679818
## EnxDev's Review Agent — apache/superset#42299 · HEAD c90cb13c
**comment** — the root cause is correctly identified and the regression guards are real, but `hideOverlap: false` lands on the *default* chart config, and the `interval: 'auto'` argument used to dismiss that on both threads doesn't hold for `type: 'time'` axes.
Verified the premise first: in echarts 6.1.0 `AxisBuilder.layOutAxisTickLabel` calls `fixMinMaxLabelShow` and *then* `hideOverlap`, so a forced max label can indeed be dropped after being forced. The bug in #39899 is real and this diff targets the actual mechanism.
### 🔴 Functional
- **`superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:1259`** (same at **`MixedTimeseries/transformProps.ts:770`**) · _Medium_ — `showMaxLabel` is not a narrow case: it's `Time axis && rotation === 0 && grain`, and `xAxisLabelRotation` defaults to `0` (`src/defaults.ts`), so every temporal chart with a resolved grain — the default configuration — loses label-collision handling. The counter-argument on both threads (`interval: 'auto'` still thins ticks) doesn't apply to this axis type: echarts routes non-category axes through `makeRealNumberLabels`, which never reads `axisLabel.interval` (`coord/axisTickLabelBuilder.js`, `createAxisLabels` branches on `axis.type === 'category'`). Time-axis tick count is `span / splitNumber` (`scale/Time.js`, `calcNiceForTimeScale`) and is width-independent, and Superset sets `splitNumber` only on the yAxis (`transformProps.ts:1317`). Echarts' own comment at the `hideOverlap` call site — "This bit fixes the la
bel overlap issue for the time chart" (echarts#14266) — is about exactly this case. Net effect: a narrow dashboard tile with a day grain renders ~6-10 date labels into ~300px with nothing thinning them. The yAxis already has the right pattern here (`splitNumber` tiered off available pixels) — deriving an xAxis `splitNumber` from `width`, or gating the disable on width, fixes the boundary label without dropping collision handling globally. **regression test:** transform at `width: 300` with a day grain and assert the guard survives (`hideOverlap` true, or a reduced xAxis `splitNumber`); today nothing fails when the axis goes unthinned.
One mitigation worth knowing, since it narrows what the diff actually buys: `fixMinMaxLabelShow` already hides the *neighbour* of a forced min/max label when they intersect, so the boundary collision was handled. What the disable adds is protection against the max label losing to a non-adjacent label — paid for with every intermediate collision.
### 🟡 Should-fix
- **`test/Timeseries/transformers.test.ts:351`** — flipping this assertion removes the last test that pinned `hideOverlap: true` on a timeseries x-axis; the surviving `!(Time && rotation !== 0)` branch is now unpinned repo-wide. Add a case for time axis + 0° rotation + no time grain → `hideOverlap` true, otherwise the next edit collapses the ternary silently.
### 🔵 Nits
- `test/MixedTimeseries/transformProps.test.ts:1167` — the same two-row fixture is constructed four times (`defaultQueriesData` ×2, `queriesData` ×2); hoist it to one const.
- `test/Timeseries/transformers.test.ts:343` — the name still claims "x-axis dates do not overlap" while the body now asserts overlap detection is off. Rename to what it guards.
- `MixedTimeseries/transformProps.ts:770` — the Timeseries counterpart carries the rationale comment; this one has none.
### 🙌 Praise
- `test/MixedTimeseries/transformProps.test.ts:1167` — it genuinely fails without the fix (the old value at 0° rotation was `true`), and the note about needing `timeGrainSqla` set to actually reach the branch is what keeps a test from silently no-opping. Addresses the review request for mixed coverage.
CI is green apart from the netlify docs preview (not run). Title is conventional-commits clean; no migration, auth, or SQL surface.
<!-- enxdev-review-agent:c90cb13c -->
_Reviewed by EnxDev's Review Agent — @EnxDev · HEAD c90cb13c._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #42299:
URL: https://github.com/apache/superset/pull/42299#issuecomment-5042641491
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 3860da923fbe4ad6aa4b9abf07d20088eabce2c4 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a6062cddd9f0a000894d7b4 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-42299--superset-docs-preview.netlify.app](https://deploy-preview-42299--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DcpNCoAgEEDhu8y6aIiwnxt0jNEZSbASdWFEd282j7f4Xkj5PlOFDaADm-lyh74PbajhlCI5SOkbtVD6SKVqrESlLCnez86KDRk0o2Pm1SMh4rJOPNsJvh8) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-42299--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #42299:
URL: https://github.com/apache/superset/pull/42299#issuecomment-5135274044
Hopefully the conflict isn't my fault, but Copilot found a lot of little things with the test that could use some scrutiny, if you don't mind.
Thanks for the help seeing this through!
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #42299:
URL: https://github.com/apache/superset/pull/42299#discussion_r3708030473
##########
superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts:
##########
@@ -1013,3 +1020,421 @@ test('temporal x coltype wires the time formatter and Time axis', () => {
expect(typeof label).toBe('string');
expect(label).not.toMatch(/NaN/);
});
+
+test('tooltip time grain wiring: dashboard-level extraFormData time grain overrides the chart-level grain in the tooltip', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ // The chart itself is configured with a Day grain...
+ timeGrainSqla: TimeGranularity.DAY,
+ // ...but a dashboard-level filter/override resolves to Month.
+ extraFormData: { time_grain_sqla: TimeGranularity.MONTH },
Review Comment:
Ran it. The formatter does read resolvedTimeGrain (extraFormData.time_grain_sqla falls back to timeGrainSqla, both feed getTooltipTimeFormatter), and this test passes against the real implementation. False positive.
##########
superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts:
##########
@@ -1013,3 +1020,421 @@ test('temporal x coltype wires the time formatter and Time axis', () => {
expect(typeof label).toBe('string');
expect(label).not.toMatch(/NaN/);
});
+
+test('tooltip time grain wiring: dashboard-level extraFormData time grain overrides the chart-level grain in the tooltip', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ // The chart itself is configured with a Day grain...
+ timeGrainSqla: TimeGranularity.DAY,
+ // ...but a dashboard-level filter/override resolves to Month.
+ extraFormData: { time_grain_sqla: TimeGranularity.MONTH },
+ },
+ queriesData: [temporalQueryData, temporalQueryData],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const tooltipFormatter = (echartOptions as unknown as TooltipFormatterOptions)
+ .tooltip.formatter;
+
+ const result = tooltipFormatter({
+ value: [ts, 100],
+ seriesName: 'metric',
+ });
+
+ // Month grain (the dashboard override) should win, so the tooltip title
+ // reads "Jan 2021" rather than the Day-grain "2021-01-07".
+ expect(result).toContain('Jan');
+ expect(result).toContain('2021');
+ expect(result).not.toContain('2021-01-07');
+});
+
+test('tooltip time grain wiring: chart-level time grain drives the tooltip when there is no dashboard override', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ timeGrainSqla: TimeGranularity.YEAR,
+ },
+ queriesData: [temporalQueryData, temporalQueryData],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const tooltipFormatter = (echartOptions as unknown as TooltipFormatterOptions)
+ .tooltip.formatter;
+
+ const result = tooltipFormatter({
+ value: [ts, 100],
+ seriesName: 'metric',
+ });
+
+ expect(result).toContain('2021');
+ expect(result).not.toContain('2021-01-07');
+});
+
+const createTemporalMixedChartProps = (timeFormat: string) => {
+ const rows = [
+ { __timestamp: Date.UTC(2003, 4, 1), metric: 10 },
+ { __timestamp: Date.UTC(2004, 0, 1), metric: 20 },
+ { __timestamp: Date.UTC(2005, 4, 1), metric: 30 },
+ ];
+ const q = createTestQueryData(rows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+ return createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [q, q],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ timeGrainSqla: TimeGranularity.MONTH,
+ xAxisTimeFormat: timeFormat,
+ },
+ queriesData: [q, q],
+ });
+};
+
+test('x-axis forces showMinLabel for time grains so the beginning date stays visible (mixed)', () => {
+ const xAxis = transformProps(createTemporalMixedChartProps('smart_date'))
+ .echartOptions.xAxis as any;
+ expect(xAxis.axisLabel.showMinLabel).toBe(true);
+});
Review Comment:
showMinLabel does get set, it's spread onto axisLabel alongside showMaxLabel/alignMaxLabel/alignMinLabel whenever showMaxLabel is true. This test passes against the real implementation.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "prathamesh04 (via GitHub)" <gi...@apache.org>.
prathamesh04 commented on PR #42299:
URL: https://github.com/apache/superset/pull/42299#issuecomment-5113137229
Hi @rusackas — this PR has been approved for a while now. Could we get this merged? Thanks!
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42299:
URL: https://github.com/apache/superset/pull/42299#discussion_r3679678245
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -728,7 +728,9 @@ export default function transformProps(
nameGap: convertInteger(xAxisTitleMargin),
nameLocation: 'middle',
axisLabel: {
- hideOverlap: !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0),
+ hideOverlap: showMaxLabel
+ ? false
+ : !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0),
Review Comment:
**Suggestion:** Mixed-timeseries charts now disable ECharts' overlap suppression whenever `showMaxLabel` is active, including dense charts with many combined ticks from both queries. This can render every intermediate label on top of the others and make the x-axis unreadable; the endpoint should be preserved without globally disabling density handling. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Dense Mixed Timeseries x-axes become unreadable.
- ⚠️ Intermediate daily labels can overlap on narrow charts.
- ⚠️ Endpoint visibility is preserved at the cost of label density.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d0f4b3dacd5844a8afffa689903aba4d&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=d0f4b3dacd5844a8afffa689903aba4d&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:** 731:733
**Comment:**
*Logic Error: Mixed-timeseries charts now disable ECharts' overlap suppression whenever `showMaxLabel` is active, including dense charts with many combined ticks from both queries. This can render every intermediate label on top of the others and make the x-axis unreadable; the endpoint should be preserved without globally disabling density handling.
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%2F42299&comment_hash=f9233bc1a3d32180b075bbb54b570353c91008956716023c0b2177450a481a27&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42299&comment_hash=f9233bc1a3d32180b075bbb54b570353c91008956716023c0b2177450a481a27&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -906,10 +906,12 @@ export default function transformProps(
// When rotation is applied on time axes, hideOverlap can
// aggressively hide the last label. Rotated labels already
// have less overlap, so disabling hideOverlap is safe.
- // At 0° rotation, keep hideOverlap to prevent long labels
- // from overlapping each other, with showMaxLabel to ensure
- // the last data point label stays visible (#37181).
- hideOverlap: !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0),
+ // At 0° rotation, also disable hideOverlap when showMaxLabel
+ // is active so the forced boundary label is never suppressed
+ // by ECharts' overlap detection (#39899).
+ hideOverlap: showMaxLabel
+ ? false
+ : !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0),
Review Comment:
**Suggestion:** Setting `hideOverlap` to `false` for every non-rotated time axis with a time grain disables ECharts' only label-density suppression. Dense daily or monthly ranges on narrow charts will render all intermediate labels, causing severe overlap and unreadable x-axis text. Preserve overlap suppression for intermediate labels while ensuring the forced endpoint label remains visible. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Dense Timeseries x-axes can become unreadable.
- ⚠️ Daily and monthly labels collide on narrow charts.
- ⚠️ Label-density behavior regresses for common date ranges.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=bfba196a0e114ad2a43620aa264ea2f0&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=bfba196a0e114ad2a43620aa264ea2f0&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/transformProps.ts
**Line:** 912:914
**Comment:**
*Logic Error: Setting `hideOverlap` to `false` for every non-rotated time axis with a time grain disables ECharts' only label-density suppression. Dense daily or monthly ranges on narrow charts will render all intermediate labels, causing severe overlap and unreadable x-axis text. Preserve overlap suppression for intermediate labels while ensuring the forced endpoint label remains visible.
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%2F42299&comment_hash=60a575fe1c60b2c57cdd4d90599e58f5a7bcf264db7ff1344ef84305453abab9&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42299&comment_hash=60a575fe1c60b2c57cdd4d90599e58f5a7bcf264db7ff1344ef84305453abab9&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts:
##########
@@ -1013,3 +1020,421 @@ test('temporal x coltype wires the time formatter and Time axis', () => {
expect(typeof label).toBe('string');
expect(label).not.toMatch(/NaN/);
});
+
+test('tooltip time grain wiring: dashboard-level extraFormData time grain overrides the chart-level grain in the tooltip', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ // The chart itself is configured with a Day grain...
+ timeGrainSqla: TimeGranularity.DAY,
+ // ...but a dashboard-level filter/override resolves to Month.
+ extraFormData: { time_grain_sqla: TimeGranularity.MONTH },
+ },
+ queriesData: [temporalQueryData, temporalQueryData],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const tooltipFormatter = (echartOptions as unknown as TooltipFormatterOptions)
+ .tooltip.formatter;
+
+ const result = tooltipFormatter({
+ value: [ts, 100],
+ seriesName: 'metric',
+ });
+
+ // Month grain (the dashboard override) should win, so the tooltip title
+ // reads "Jan 2021" rather than the Day-grain "2021-01-07".
+ expect(result).toContain('Jan');
+ expect(result).toContain('2021');
+ expect(result).not.toContain('2021-01-07');
+});
+
+test('tooltip time grain wiring: chart-level time grain drives the tooltip when there is no dashboard override', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ timeGrainSqla: TimeGranularity.YEAR,
+ },
+ queriesData: [temporalQueryData, temporalQueryData],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const tooltipFormatter = (echartOptions as unknown as TooltipFormatterOptions)
+ .tooltip.formatter;
+
+ const result = tooltipFormatter({
+ value: [ts, 100],
+ seriesName: 'metric',
+ });
+
+ expect(result).toContain('2021');
+ expect(result).not.toContain('2021-01-07');
+});
+
+const createTemporalMixedChartProps = (timeFormat: string) => {
+ const rows = [
+ { __timestamp: Date.UTC(2003, 4, 1), metric: 10 },
+ { __timestamp: Date.UTC(2004, 0, 1), metric: 20 },
+ { __timestamp: Date.UTC(2005, 4, 1), metric: 30 },
+ ];
+ const q = createTestQueryData(rows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+ return createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [q, q],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ timeGrainSqla: TimeGranularity.MONTH,
+ xAxisTimeFormat: timeFormat,
+ },
+ queriesData: [q, q],
+ });
+};
+
+test('x-axis forces showMinLabel for time grains so the beginning date stays visible (mixed)', () => {
+ const xAxis = transformProps(createTemporalMixedChartProps('smart_date'))
+ .echartOptions.xAxis as any;
+ expect(xAxis.axisLabel.showMinLabel).toBe(true);
+});
+
+test('x-axis dedup keeps the forced min label when the endpoints format identically (mixed)', () => {
+ // May→May range renders "May" at both boundaries; the dedup must reset per
+ // ECharts pass so the forced min label survives the second pass.
+ const { formatter } = (
+ transformProps(createTemporalMixedChartProps('%b')).echartOptions
+ .xAxis as any
+ ).axisLabel;
+ const min = Date.UTC(2003, 4, 1);
+ const mid = Date.UTC(2004, 0, 1);
+ const max = Date.UTC(2005, 4, 1);
+
+ formatter(min);
+ formatter(mid);
+ formatter(max);
+
+ expect(formatter(min)).toBe('May');
+});
+
+test('#39899 - x-axis dates do not overlap and last label stays visible at 0° rotation (mixed)', () => {
+ // When showMaxLabel is active on a time axis with 0° rotation,
+ // hideOverlap must be off so ECharts cannot suppress the forced
+ // max label (the end-of-axis date).
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [
+ createTestQueryData(
+ [
+ {
+ __timestamp: Date.UTC(2026, 0, 1),
+ sum__num: 100,
+ },
+ {
+ __timestamp: Date.UTC(2026, 6, 1),
+ sum__num: 200,
+ },
+ ],
+ {
+ colnames: ['__timestamp', 'sum__num'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], sum__num: ['sum__num'] },
+ },
+ ),
+ createTestQueryData(
+ [
+ {
+ __timestamp: Date.UTC(2026, 0, 1),
+ sum__num: 100,
+ },
+ {
+ __timestamp: Date.UTC(2026, 6, 1),
+ sum__num: 200,
+ },
+ ],
+ {
+ colnames: ['__timestamp', 'sum__num'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], sum__num: ['sum__num'] },
+ },
+ ),
+ ],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['sum__num'],
+ metricsB: ['sum__num'],
+ groupby: [],
+ groupbyB: [],
+ xAxisLabelRotation: 0,
+ },
+ queriesData: [
+ createTestQueryData(
+ [
+ {
+ __timestamp: Date.UTC(2026, 0, 1),
+ sum__num: 100,
+ },
+ {
+ __timestamp: Date.UTC(2026, 6, 1),
+ sum__num: 200,
+ },
+ ],
+ {
+ colnames: ['__timestamp', 'sum__num'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], sum__num: ['sum__num'] },
+ },
+ ),
+ createTestQueryData(
+ [
+ {
+ __timestamp: Date.UTC(2026, 0, 1),
+ sum__num: 100,
+ },
+ {
+ __timestamp: Date.UTC(2026, 6, 1),
+ sum__num: 200,
+ },
+ ],
+ {
+ colnames: ['__timestamp', 'sum__num'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], sum__num: ['sum__num'] },
+ },
+ ),
+ ],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const { axisLabel } = echartOptions.xAxis as Record<string, any>;
+
+ expect(axisLabel.showMaxLabel).toBe(true);
+ expect(axisLabel.alignMaxLabel).toBe('right');
+ expect(axisLabel.hideOverlap).toBe(false);
+});
+
+test('regression #37921: multi-metric Query A with groupby does not duplicate first metric in series names', () => {
+ // Regression test for https://github.com/apache/superset/issues/37921
+ // ("Residual" follow-up to #37055).
+ //
+ // When Query A has multiple metrics + at least one Group By dimension,
+ // the display-name builder in transformProps.ts used to prepend the FIRST
+ // metric's display name to every series that didn't literally contain it:
+ // name: `${MetricDisplayNameA}, ${entryName}`
+ // For series belonging to the *second* metric, this produced a
+ // cross-contaminated label like `score_one, score_two, A` — the
+ // user-visible "first metric duplicated" symptom in the legend / tooltip.
+ // The fix derives each series' metric from its label-map tuple instead.
+ const multiMetricRows = [
+ {
+ 'score_one, A': 1,
+ 'score_one, B': 2,
+ 'score_two, A': 3,
+ 'score_two, B': 4,
+ ds: 599616000000,
+ },
+ {
+ 'score_one, A': 5,
+ 'score_one, B': 6,
+ 'score_two, A': 7,
+ 'score_two, B': 8,
+ ds: 599916000000,
+ },
+ ];
+ const multiMetricLabelMap = {
+ ds: ['ds'],
+ 'score_one, A': ['score_one', 'A'],
+ 'score_one, B': ['score_one', 'B'],
+ 'score_two, A': ['score_two', 'A'],
+ 'score_two, B': ['score_two', 'B'],
+ };
+
+ const queryAData = createTestQueryData(multiMetricRows, {
+ label_map: multiMetricLabelMap,
+ });
+ // Query B keeps the existing single-metric shape — the bug is on
+ // Query A's path so we just need a valid Query B alongside.
+ const queryBData = createTestQueryData(defaultQueryRows, {
+ label_map: defaultLabelMap,
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [queryAData, queryBData],
+ formData: {
+ ...formData,
+ metrics: ['score_one', 'score_two'],
+ groupby: ['category'],
+ },
+ queriesData: [queryAData, queryBData],
+ });
+ const transformed = transformProps(chartProps);
+
+ const queryASeriesNames = (transformed.echartOptions.series as any[])
+ .map((s: any) => String(s.name))
+ .filter((n: string) => n.includes('score_'));
+
+ // Each (metric, dim_value) combo from Query A should appear exactly once
+ // with the *correct* metric prefix — not the first-metric-prepended-to-
+ // everything-else form. Comparing the sorted array (rather than using
+ // separate toContain assertions) also catches a regression that emits
+ // duplicate series for the same name.
+ expect([...queryASeriesNames].sort()).toEqual(
+ ['score_one, A', 'score_one, B', 'score_two, A', 'score_two, B'].sort(),
+ );
Review Comment:
**Suggestion:** The expected series names encode a metric-specific naming fix that is not present in the mixed-timeseries transform. Query A still derives `metricPart` from `MetricDisplayNameA` (the first metric), so `score_two, A` becomes `score_one, score_two, A`; the newly added expectation will fail until the production series-name builder is changed or the test is removed from this PR. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Mixed Timeseries transform tests fail in CI.
- ⚠️ Query A legend names contain incorrect metric prefixes.
- ⚠️ Tooltips expose cross-contaminated metric labels.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4b7e1b51fdc24bb2a4be5c627bffa307&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=4b7e1b51fdc24bb2a4be5c627bffa307&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/test/MixedTimeseries/transformProps.test.ts
**Line:** 1334:1336
**Comment:**
*Api Mismatch: The expected series names encode a metric-specific naming fix that is not present in the mixed-timeseries transform. Query A still derives `metricPart` from `MetricDisplayNameA` (the first metric), so `score_two, A` becomes `score_one, score_two, A`; the newly added expectation will fail until the production series-name builder is changed or the test is removed from this PR.
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%2F42299&comment_hash=19f346b4d43c3dcb3e1f028a47d5d344f09cbe429afca36fc775a436ee0e376c&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42299&comment_hash=19f346b4d43c3dcb3e1f028a47d5d344f09cbe429afca36fc775a436ee0e376c&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts:
##########
@@ -1013,3 +1020,421 @@ test('temporal x coltype wires the time formatter and Time axis', () => {
expect(typeof label).toBe('string');
expect(label).not.toMatch(/NaN/);
});
+
+test('tooltip time grain wiring: dashboard-level extraFormData time grain overrides the chart-level grain in the tooltip', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ // The chart itself is configured with a Day grain...
+ timeGrainSqla: TimeGranularity.DAY,
+ // ...but a dashboard-level filter/override resolves to Month.
+ extraFormData: { time_grain_sqla: TimeGranularity.MONTH },
+ },
+ queriesData: [temporalQueryData, temporalQueryData],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const tooltipFormatter = (echartOptions as unknown as TooltipFormatterOptions)
+ .tooltip.formatter;
+
+ const result = tooltipFormatter({
+ value: [ts, 100],
+ seriesName: 'metric',
+ });
+
+ // Month grain (the dashboard override) should win, so the tooltip title
+ // reads "Jan 2021" rather than the Day-grain "2021-01-07".
+ expect(result).toContain('Jan');
+ expect(result).toContain('2021');
+ expect(result).not.toContain('2021-01-07');
+});
+
+test('tooltip time grain wiring: chart-level time grain drives the tooltip when there is no dashboard override', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ timeGrainSqla: TimeGranularity.YEAR,
+ },
+ queriesData: [temporalQueryData, temporalQueryData],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const tooltipFormatter = (echartOptions as unknown as TooltipFormatterOptions)
+ .tooltip.formatter;
+
+ const result = tooltipFormatter({
+ value: [ts, 100],
+ seriesName: 'metric',
+ });
+
+ expect(result).toContain('2021');
+ expect(result).not.toContain('2021-01-07');
+});
+
+const createTemporalMixedChartProps = (timeFormat: string) => {
+ const rows = [
+ { __timestamp: Date.UTC(2003, 4, 1), metric: 10 },
+ { __timestamp: Date.UTC(2004, 0, 1), metric: 20 },
+ { __timestamp: Date.UTC(2005, 4, 1), metric: 30 },
+ ];
+ const q = createTestQueryData(rows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+ return createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [q, q],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ timeGrainSqla: TimeGranularity.MONTH,
+ xAxisTimeFormat: timeFormat,
+ },
+ queriesData: [q, q],
+ });
+};
+
+test('x-axis forces showMinLabel for time grains so the beginning date stays visible (mixed)', () => {
+ const xAxis = transformProps(createTemporalMixedChartProps('smart_date'))
+ .echartOptions.xAxis as any;
+ expect(xAxis.axisLabel.showMinLabel).toBe(true);
+});
Review Comment:
**Suggestion:** The mixed-timeseries transform does not set `axisLabel.showMinLabel`; it only sets `showMaxLabel` and `alignMaxLabel`. Therefore this newly added assertion fails for the implementation in this change and introduces a permanently failing test unless a corresponding production assignment is added or the assertion is removed. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Mixed Timeseries transform tests fail in CI.
- ⚠️ Pull-request validation is blocked by a false contract.
- ⚠️ The test asserts unsupported minimum-label behavior.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2b6aa0337bb246759d2441963749f32d&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=2b6aa0337bb246759d2441963749f32d&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/test/MixedTimeseries/transformProps.test.ts
**Line:** 1141:1145
**Comment:**
*Api Mismatch: The mixed-timeseries transform does not set `axisLabel.showMinLabel`; it only sets `showMaxLabel` and `alignMaxLabel`. Therefore this newly added assertion fails for the implementation in this change and introduces a permanently failing test unless a corresponding production assignment is added or the assertion is removed.
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%2F42299&comment_hash=38feb0b656119d941ef8ea819077e392611a164e3e530ea41903df7701772009&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42299&comment_hash=38feb0b656119d941ef8ea819077e392611a164e3e530ea41903df7701772009&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts:
##########
@@ -1013,3 +1020,421 @@ test('temporal x coltype wires the time formatter and Time axis', () => {
expect(typeof label).toBe('string');
expect(label).not.toMatch(/NaN/);
});
+
+test('tooltip time grain wiring: dashboard-level extraFormData time grain overrides the chart-level grain in the tooltip', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ // The chart itself is configured with a Day grain...
+ timeGrainSqla: TimeGranularity.DAY,
+ // ...but a dashboard-level filter/override resolves to Month.
+ extraFormData: { time_grain_sqla: TimeGranularity.MONTH },
Review Comment:
**Suggestion:** This test asserts that `extraFormData.time_grain_sqla` changes the tooltip grain, but the implementation constructs the tooltip formatter solely from `tooltipTimeFormat`; it does not read `extraFormData` or use `timeGrainSqla` for tooltip formatting. Consequently, the test either fails because the formatter returns the raw timestamp or passes for an unrelated default format without validating the claimed override. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Tooltip regression coverage does not test dashboard grain overrides.
- ⚠️ Future tooltip changes can pass this false-positive test.
- ⚠️ Mixed Timeseries tooltip behavior remains unverified.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=eec731102e4b4a489dd4bc5a66c51c44&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=eec731102e4b4a489dd4bc5a66c51c44&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/test/MixedTimeseries/transformProps.test.ts
**Line:** 1046:1049
**Comment:**
*Api Mismatch: This test asserts that `extraFormData.time_grain_sqla` changes the tooltip grain, but the implementation constructs the tooltip formatter solely from `tooltipTimeFormat`; it does not read `extraFormData` or use `timeGrainSqla` for tooltip formatting. Consequently, the test either fails because the formatter returns the raw timestamp or passes for an unrelated default format without validating the claimed override.
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%2F42299&comment_hash=70a725e318d776fa1e7403e0c0d196073ea5698e11d1788f00a2afbc893eba15&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42299&comment_hash=70a725e318d776fa1e7403e0c0d196073ea5698e11d1788f00a2afbc893eba15&reaction=dislike'>👎</a>
##########
superset-frontend/plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts:
##########
@@ -1013,3 +1020,421 @@ test('temporal x coltype wires the time formatter and Time axis', () => {
expect(typeof label).toBe('string');
expect(label).not.toMatch(/NaN/);
});
+
+test('tooltip time grain wiring: dashboard-level extraFormData time grain overrides the chart-level grain in the tooltip', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ // The chart itself is configured with a Day grain...
+ timeGrainSqla: TimeGranularity.DAY,
+ // ...but a dashboard-level filter/override resolves to Month.
+ extraFormData: { time_grain_sqla: TimeGranularity.MONTH },
+ },
+ queriesData: [temporalQueryData, temporalQueryData],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const tooltipFormatter = (echartOptions as unknown as TooltipFormatterOptions)
+ .tooltip.formatter;
+
+ const result = tooltipFormatter({
+ value: [ts, 100],
+ seriesName: 'metric',
+ });
+
+ // Month grain (the dashboard override) should win, so the tooltip title
+ // reads "Jan 2021" rather than the Day-grain "2021-01-07".
+ expect(result).toContain('Jan');
+ expect(result).toContain('2021');
+ expect(result).not.toContain('2021-01-07');
+});
+
+test('tooltip time grain wiring: chart-level time grain drives the tooltip when there is no dashboard override', () => {
+ const ts = Date.UTC(2021, 0, 7);
+ const temporalRows = [{ __timestamp: ts, metric: 100 }];
+ const temporalQueryData = createTestQueryData(temporalRows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+
+ const chartProps = createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [temporalQueryData, temporalQueryData],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ timeGrainSqla: TimeGranularity.YEAR,
+ },
+ queriesData: [temporalQueryData, temporalQueryData],
+ });
+
+ const { echartOptions } = transformProps(chartProps);
+ const tooltipFormatter = (echartOptions as unknown as TooltipFormatterOptions)
+ .tooltip.formatter;
+
+ const result = tooltipFormatter({
+ value: [ts, 100],
+ seriesName: 'metric',
+ });
+
+ expect(result).toContain('2021');
+ expect(result).not.toContain('2021-01-07');
+});
+
+const createTemporalMixedChartProps = (timeFormat: string) => {
+ const rows = [
+ { __timestamp: Date.UTC(2003, 4, 1), metric: 10 },
+ { __timestamp: Date.UTC(2004, 0, 1), metric: 20 },
+ { __timestamp: Date.UTC(2005, 4, 1), metric: 30 },
+ ];
+ const q = createTestQueryData(rows, {
+ colnames: ['__timestamp', 'metric'],
+ coltypes: [GenericDataType.Temporal, GenericDataType.Numeric],
+ label_map: { __timestamp: ['__timestamp'], metric: ['metric'] },
+ });
+ return createEchartsTimeseriesTestChartProps<
+ EchartsMixedTimeseriesFormData,
+ EchartsMixedTimeseriesProps
+ >({
+ ...MIXED_TIMESERIES_CHART_PROPS_DEFAULTS,
+ defaultQueriesData: [q, q],
+ formData: {
+ ...formData,
+ x_axis: '__timestamp',
+ metrics: ['metric'],
+ metricsB: ['metric'],
+ groupby: [],
+ groupbyB: [],
+ timeGrainSqla: TimeGranularity.MONTH,
+ xAxisTimeFormat: timeFormat,
+ },
+ queriesData: [q, q],
+ });
+};
+
+test('x-axis forces showMinLabel for time grains so the beginning date stays visible (mixed)', () => {
+ const xAxis = transformProps(createTemporalMixedChartProps('smart_date'))
+ .echartOptions.xAxis as any;
+ expect(xAxis.axisLabel.showMinLabel).toBe(true);
+});
+
+test('x-axis dedup keeps the forced min label when the endpoints format identically (mixed)', () => {
+ // May→May range renders "May" at both boundaries; the dedup must reset per
+ // ECharts pass so the forced min label survives the second pass.
+ const { formatter } = (
+ transformProps(createTemporalMixedChartProps('%b')).echartOptions
+ .xAxis as any
+ ).axisLabel;
+ const min = Date.UTC(2003, 4, 1);
+ const mid = Date.UTC(2004, 0, 1);
+ const max = Date.UTC(2005, 4, 1);
+
+ formatter(min);
+ formatter(mid);
+ formatter(max);
+
+ expect(formatter(min)).toBe('May');
+});
Review Comment:
**Suggestion:** `formatter` closes over a single `lastLabel` value and has no reset mechanism between formatting passes. After formatting `min`, `mid`, and `max`, the final `formatter(min)` call returns an empty string because the previous `max` label is also `May`; this assertion therefore fails and assumes lifecycle behavior the formatter does not provide. [stale reference]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Mixed Timeseries transform tests fail in CI.
- ⚠️ Formatter behavior is incorrectly tested across render passes.
- ⚠️ The regression test cannot model ECharts formatter lifecycle.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=072fafdea5d84410993dd1748514f19a&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=072fafdea5d84410993dd1748514f19a&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/test/MixedTimeseries/transformProps.test.ts
**Line:** 1158:1163
**Comment:**
*Stale Reference: `formatter` closes over a single `lastLabel` value and has no reset mechanism between formatting passes. After formatting `min`, `mid`, and `max`, the final `formatter(min)` call returns an empty string because the previous `max` label is also `May`; this assertion therefore fails and assumes lifecycle behavior the formatter does not provide.
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%2F42299&comment_hash=261fd3369f67d83da6b537bfc87e540cd818283954313a1b2553a64a2d8d482d&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42299&comment_hash=261fd3369f67d83da6b537bfc87e540cd818283954313a1b2553a64a2d8d482d&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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #42299:
URL: https://github.com/apache/superset/pull/42299#issuecomment-5106773506
Holler if you'd like me to add the test.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #42299:
URL: https://github.com/apache/superset/pull/42299#issuecomment-5172077956
I'll see if I can push some more touch-ups here. Would love to close the issue.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on code in PR #42299:
URL: https://github.com/apache/superset/pull/42299#discussion_r3708029170
##########
superset-frontend/plugins/plugin-chart-echarts/src/MixedTimeseries/transformProps.ts:
##########
@@ -728,7 +728,9 @@ export default function transformProps(
nameGap: convertInteger(xAxisTitleMargin),
nameLocation: 'middle',
axisLabel: {
- hideOverlap: !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0),
+ hideOverlap: showMaxLabel
+ ? false
+ : !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0),
Review Comment:
Checked this against the actual code. `interval: 'auto'` on the axis (untouched by this diff) is still ECharts' primary density control, it decides which ticks even get considered before `hideOverlap` ever runs. `hideOverlap` only turns off in the narrow `showMaxLabel` case (0° rotation, time axis, resolved time grain), specifically because that's the one scenario where the forced boundary label needs to survive overlap detection. Not seeing the dense-chart regression here.
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -906,10 +906,12 @@ export default function transformProps(
// When rotation is applied on time axes, hideOverlap can
// aggressively hide the last label. Rotated labels already
// have less overlap, so disabling hideOverlap is safe.
- // At 0° rotation, keep hideOverlap to prevent long labels
- // from overlapping each other, with showMaxLabel to ensure
- // the last data point label stays visible (#37181).
- hideOverlap: !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0),
+ // At 0° rotation, also disable hideOverlap when showMaxLabel
+ // is active so the forced boundary label is never suppressed
+ // by ECharts' overlap detection (#39899).
+ hideOverlap: showMaxLabel
+ ? false
+ : !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0),
Review Comment:
Same reasoning as the MixedTimeseries thread. `interval: 'auto'` is unchanged and still does the bulk of tick thinning, `hideOverlap` is a secondary net that only turns off for the narrow showMaxLabel case where the forced boundary label needs to survive. Not the codebase's only density suppression.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #42299:
URL: https://github.com/apache/superset/pull/42299#discussion_r3708031644
##########
superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts:
##########
@@ -906,10 +906,12 @@ export default function transformProps(
// When rotation is applied on time axes, hideOverlap can
// aggressively hide the last label. Rotated labels already
// have less overlap, so disabling hideOverlap is safe.
- // At 0° rotation, keep hideOverlap to prevent long labels
- // from overlapping each other, with showMaxLabel to ensure
- // the last data point label stays visible (#37181).
- hideOverlap: !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0),
+ // At 0° rotation, also disable hideOverlap when showMaxLabel
+ // is active so the forced boundary label is never suppressed
+ // by ECharts' overlap detection (#39899).
+ hideOverlap: showMaxLabel
+ ? false
+ : !(xAxisType === AxisType.Time && xAxisLabelRotation !== 0),
Review Comment:
✅ **Customized review instruction saved!**
**Instruction:**
> Do not flag disabling hideOverlap when interval: 'auto' still provides tick thinning and showMaxLabel requires the forced boundary label to remain visible.
**Applied to:**
- `superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts`
---
💡 *To manage or update this instruction, visit: [CodeAnt AI Settings](https://app.codeant.ai/org/settings/learnings)*
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix: last date label hidden on time series x-axis (#39899) [superset]
Posted by "prathamesh04 (via GitHub)" <gi...@apache.org>.
prathamesh04 commented on PR #42299:
URL: https://github.com/apache/superset/pull/42299#issuecomment-5077060391
cc @rusackas @sadpandajoe @michael-s-molina @EnxDev — ready for first review. Video testing included in PR description. Bito flagged minor test duplication (line 377 vs 312) but no blocking issues.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org