You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "developerzohaib786 (via GitHub)" <gi...@apache.org> on 2025/11/20 05:35:57 UTC
[PR] fix(echarts): ensure last month label appears on time series x-axis [superset]
developerzohaib786 opened a new pull request, #36197:
URL: https://github.com/apache/superset/pull/36197
### SUMMARY
Fixes #33905
This PR resolves an issue where the last month's label doesn't appear on the X-axis for Time Series Line Charts with monthly time grain.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!-- If you can, please provide before/after screenshots showing the label now appearing -->
### DESCRIPTION OF CHANGE
The issue was caused by the `hideOverlap: true` setting in the x-axis label configuration, which prevented the final label from displaying when ECharts detected potential overlap.
**Changes made:**
1. **Updated** `superset-frontend/plugins/plugin-chart-echarts/src/Timeseries/transformProps.ts`
- Modified `axisLabel.hideOverlap` logic to set `false` for time-based axes (AxisType.Time)
- Set `true` for categorical/value axes to maintain overlap prevention for non-time axes
2. **Added** tests in `superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts`
- Test verifying `hideOverlap: false` for time-based x-axis
- Test verifying `hideOverlap: true` for categorical x-axis (when `xAxisForceCategorical` is enabled)
### TESTING INSTRUCTIONS
1. Create a Time Series Line Chart using example datasets
2. Set time grain to "month"
3. Verify that the last month's label now appears on the X-axis
4. Test with different time grains (day, week, quarter, year) to ensure labels display correctly
5. Test with categorical axis to ensure overlap prevention still works
### CHECKLIST
- [ ] Has associated issue: #33905
- [ ] Required feature flags:
- [ ] Change will be tested in Cypress tests
- [ ] Code follows style guidelines
- [ ] Tests pass locally with my changes
- [ ] Added tests that prove my fix is effective or that my feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] Extended the README / documentation, if necessary
- [ ] Added an entry to UPDATING.md for any breaking changes or significant updates
### ADDITIONAL NOTES
This fix specifically targets time-based axes where showing all temporal labels (including the last one) is important for data interpretation. For categorical and value axes, overlap prevention remains enabled to maintain readability.
--
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(echarts): ensure last month label appears on time series x-axis [superset]
Posted by "developerzohaib786 (via GitHub)" <gi...@apache.org>.
developerzohaib786 commented on PR #36197:
URL: https://github.com/apache/superset/pull/36197#issuecomment-3588701474
> Any indication of how this might affect existing charts/dashboards? It seems on the surface to make sense to me, but I'm not sure if this will cause any "surprises" for users that upgrade and a new data point appears or something.
> Any indication of how this might affect existing charts/dashboards? It seems on the surface to make sense to me, but I'm not sure if this will cause any "surprises" for users that upgrade and a new data point appears or something.
should i attach screenshots?
--
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(echarts): ensure last month label appears on time series x-axis [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #36197:
URL: https://github.com/apache/superset/pull/36197#issuecomment-3555935868
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(echarts): ensure last month label appears on time series x-axis [superset]
Posted by "SBIN2010 (via GitHub)" <gi...@apache.org>.
SBIN2010 commented on PR #36197:
URL: https://github.com/apache/superset/pull/36197#issuecomment-3792566365
I think this bug has already been fixed. Going to close for now, feel free to reopen if it hasn't been fixed.
--
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(echarts): ensure last month label appears on time series x-axis [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #36197:
URL: https://github.com/apache/superset/pull/36197#issuecomment-3583603487
Any indication of how this might affect existing charts/dashboards? It seems on the surface to make sense to me, but I'm not sure if this will cause any "surprises" for users that upgrade and a new data point appears or something.
--
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(echarts): ensure last month label appears on time series x-axis [superset]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #36197:
URL: https://github.com/apache/superset/pull/36197#discussion_r2547183561
##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts:
##########
@@ -723,3 +723,42 @@ describe('legend sorting', () => {
]);
});
});
+
+describe('x-axis label behavior', () => {
+ it('should set hideOverlap to false for time-based x-axis', () => {
+ const chartProps = new ChartProps(chartPropsConfig);
+ const transformed = transformProps(
+ chartProps as EchartsTimeseriesChartProps,
+ );
+
+ expect(transformed.echartOptions.xAxis).toEqual(
+ expect.objectContaining({
+ axisLabel: expect.objectContaining({
+ hideOverlap: false,
+ }),
+ }),
+ );
+ });
Review Comment:
The test claims to verify "time-based x-axis" behavior, but it doesn't actually create a time-based axis. Without providing `coltypes` and `colnames` in the `queriesData`, the `getColtypesMapping()` function returns an empty object, causing `xAxisDataType` to be `undefined`. This results in `getAxisType()` returning `AxisType.Category` (not `AxisType.Time`), so the test is actually checking categorical axis behavior.
To properly test time-based axis behavior, add `coltypes` and `colnames` to the query data:
```typescript
const timeBasedChartProps = new ChartProps({
...chartPropsConfig,
queriesData: [
{
...queriesData[0],
colnames: ['__timestamp', 'San Francisco', 'New York'],
coltypes: [GenericDataType.Temporal, GenericDataType.Numeric, GenericDataType.Numeric],
},
],
});
```
You'll also need to import `GenericDataType` from `@superset-ui/core` or `@apache-superset/core/api/core`.
--
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(echarts): ensure last month label appears on time series x-axis [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #36197:
URL: https://github.com/apache/superset/pull/36197#issuecomment-3583690126
Superset uses Git pre-commit hooks courtesy of [pre-commit](https://pre-commit.com/). To install run the following:
```
pip3 install -r requirements/development.txt
pre-commit install
```
A series of checks will now run when you make a git commit.
Alternatively it is possible to run pre-commit by running pre-commit manually:
```
pre-commit run --all-files
```
--
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(echarts): ensure last month label appears on time series x-axis [superset]
Posted by "SBIN2010 (via GitHub)" <gi...@apache.org>.
SBIN2010 closed pull request #36197: fix(echarts): ensure last month label appears on time series x-axis
URL: https://github.com/apache/superset/pull/36197
--
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(echarts): ensure last month label appears on time series x-axis [superset]
Posted by "SBIN2010 (via GitHub)" <gi...@apache.org>.
SBIN2010 commented on code in PR #36197:
URL: https://github.com/apache/superset/pull/36197#discussion_r2547234359
##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts:
##########
@@ -723,3 +723,42 @@ describe('legend sorting', () => {
]);
});
});
+
+describe('x-axis label behavior', () => {
Review Comment:
Could you avoid using describe and it and use test instead to follow our [testing guidelines](https://github.com/apache/superset/wiki/Testing-Guidelines-and-Best-Practices#avoid-nesting-when-youre-testing)?
--
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(echarts): ensure last month label appears on time series x-axis [superset]
Posted by "developerzohaib786 (via GitHub)" <gi...@apache.org>.
developerzohaib786 commented on code in PR #36197:
URL: https://github.com/apache/superset/pull/36197#discussion_r2548549494
##########
superset-frontend/plugins/plugin-chart-echarts/test/Timeseries/transformProps.test.ts:
##########
@@ -723,3 +723,42 @@ describe('legend sorting', () => {
]);
});
});
+
+describe('x-axis label behavior', () => {
Review Comment:
yes i have changed, please re run tests
have a good day
--
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