Anikesh0001 opened a new pull request, #45519:
URL: https://github.com/apache/superset/pull/45519
### SUMMARY
Fixes #45474.
Since #44628, `getGrainBarMaxWidth` caps the bar width at one time-grain
bucket for every grain in `TIMEGRAIN_TO_TIMESTAMP`. That cap was meant for
sparse **sub-daily** data, as the comment in `Timeseries/transformers.ts` says
("For a sub-daily time grain, transformProps.ts overrides this…"). For Day and
coarser grains it misbehaves when the data is spaced further apart than the
grain, which is the common case of the default Day grain on pre-aggregated
monthly or yearly rows. One day is then a sliver of the axis, and the bars are
drawn as hairlines.
This change follows the fix proposed in the issue:
- `getGrainBarMaxWidth` returns `undefined` when the grain is a day or
longer (`grainMs >= ONE_DAY_MS`).
- Both call sites (`Timeseries/transformProps.ts` and
`MixedTimeseries/transformProps.ts`) already keep the default `barMaxWidth:
100` in that case, so ECharts sizes the bars from the spacing of the data
again, as in 6.1.0.
- The sub-daily behaviour from #44628 is unchanged, and its tests still pass.
- The docstring now says that only sub-daily grains get a grain-derived cap.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
No screenshots from me. The issue has 6.1.0 vs 7.0.0rc2 screenshots of the
same chart. The numbers come from the new tests, with an 800px chart and data
from 2019-01-01 to 2024-01-01:
| Case | `barMaxWidth` before | after |
|---|---|---|
| Bar chart, Day grain, yearly data | 0.42px | 100 (default cap) |
| Bar chart, Day grain, monthly data | 2.27px | 100 |
| Bar chart, Month grain, yearly data | 12.90px | 100 |
| Mixed chart (bar series), Day grain, yearly data | 0.42px | 100 |
### TESTING INSTRUCTIONS
New tests:
-
`plugins/plugin-chart-echarts/test/Timeseries/Bar/sparseSubDailyBarGeometry.test.ts`:
a Day grain on yearly and on monthly data, and a Month grain on yearly data,
keep the default bar width cap.
-
`plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps.test.ts`: the
same for a mixed chart with a bar series, with a Day grain on yearly data.
All 4 fail on `master` (they receive the hairline widths above) and pass
with this change.
Run locally with Node 24.16 / npm 11:
- `npx jest plugins/plugin-chart-echarts`: 101 suites, 1391 tests passed.
The existing `PT1H` tests for #44628 still pass.
- `oxlint` and `oxfmt --check` on the changed files: clean.
- `node scripts/check-type.js package=superset-frontend <changed files>`
(after `tsc --build` of the plugin project): no errors.
To check it manually, follow the steps in #45474: a Bar Chart on a dataset
with one row per year, x-axis on the temporal column, Time Grain left at Day.
The bars should have normal width, as in 6.1.0.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #45474
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
*Generative AI disclosure, following the [ASF
guidelines](https://www.apache.org/legal/generative-tooling.html): I used
Claude Code (Claude Opus 5.5) to help investigate this issue and draft the
change, the tests and this description. I reviewed the result, checked that the
new tests fail on `master`, and ran the checks above locally.*
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
