s1ny1998 opened a new pull request, #43299:
URL: https://github.com/apache/superset/pull/43299
### SUMMARY
Timeseries charts render with a large blank strip on the left: the plot area
starts well inside the card, far outside the tick labels. It is barely
noticeable on a wide desktop chart, but it eats roughly a third of the plot
area on a narrow one.
The cause is in `getPadding()`
(`plugins/plugin-chart-echarts/src/Timeseries/transformers.ts`):
```ts
left:
yAxisTitlePosition === 'Left'
? TIMESERIES_CONSTANTS.gridOffsetLeft + (Number(yAxisTitleMargin) || 0)
: TIMESERIES_CONSTANTS.gridOffsetLeft,
```
The Y-axis title margin is added whenever the title position is `'Left'`,
without checking that a title is actually rendered. The control defaults
(`sections/chartTitle.tsx`) are position `'Left'` and margin `50`, so every
chart with an empty `y_axis_title` — the default — reserves `20 + 50 = 70px` on
the left. Because `grid.containLabel` is enabled, that reservation sits
*outside* the axis labels, so it is pure dead space.
The guard already exists: callers compute `addYAxisTitleOffset =
!!yAxisTitle && convertInteger(yAxisTitleMargin) !== 0`, and the `'Top'`
position branch consumes it. Only the `'Left'` branch ignored it.
Changes:
- Gate the left title margin on `addYAxisTitleOffset`. Charts with a Y-axis
title keep exactly the spacing they have today; charts without one reclaim the
50px.
- `Gantt/transformProps.ts` hardcoded `false` for that flag at both
`getPadding()` call sites while relying on the `'Left'` branch for its left
padding, so it now computes the flag properly and passes it to both. Without
that, gating the left branch would have silently removed Gantt's title spacing.
- Added a `getPadding()` unit test covering the no-title case.
This supersedes #43064, which was closed while in draft. Same change,
rebased onto current master with a clean history.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — the visible effect is the left grid offset shrinking from 70px to 20px
on charts with no Y-axis title, which is easiest to confirm with the testing
instructions below.
### TESTING INSTRUCTIONS
Unit tests:
```bash
cd superset-frontend
npm run test --
plugins/plugin-chart-echarts/test/Timeseries/transformers.test.ts
```
Manually:
1. Create a Line Chart (or any ECharts timeseries viz) and leave **Customize
→ Y Axis title** empty.
2. Before this change the plot area starts ~70px from the left edge, leaving
a visible blank strip outside the tick labels. After it, the strip is gone and
the chart fills the card.
3. Set a **Y Axis title** with position `Left`. Spacing is unchanged from
current behaviour — the title still gets its 50px margin.
4. Repeat with a Gantt chart, with and without a Y-axis title, to confirm
its padding is unchanged when a title is set.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] 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
--
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]
