rlei-odes opened a new pull request, #43428:
URL: https://github.com/apache/superset/pull/43428
### SUMMARY
The ECharts timeseries family exposes **Minor ticks** and **Minor Split
Line**
as controls, but the main gridlines and main axis ticks are hardcoded in the
transform, on the lines immediately around them:
```ts
minorTick: { show: isSmallChart ? false : minorTicks },
minorSplitLine: { show: isSmallChart ? false : minorSplitLine },
splitLine: { show: !isSmallChart }, // no control
axisTick: { show: !isSmallChart }, // no control
```
So the faint gridlines between the labelled ticks can be turned off, while
the
main ones they subdivide cannot. That asymmetry reads as accidental rather
than
deliberate.
This adds the two matching checkboxes, **Gridlines** and **Axis ticks**, to
the
Customize tab of every chart that already carries the minor pair: Bar, Line,
Area, Step, Scatter, Smooth Line and Mixed.
Each control covers exactly the axes its minor counterpart already covers:
| Control | Axes | Mirrors |
| --- | --- | --- |
| Gridlines | value axis; also removes them from the x-axis when that axis
is numeric | `minorSplitLine` |
| Axis ticks | both axes | `minorTicks` |
Only the value axis has gridlines to begin with — ECharts defaults both
category
and time axes to `splitLine: { show: false }` — so the control mirrors
`minorSplitLine` in practice. The one exception is a numeric x-axis, which
does
default to gridlines. Unticking removes those too, by writing `show: false`
on
the x-axis only when hiding. Ticking never writes the key, so no axis gains
gridlines it does not already have.
Three details worth flagging for review:
- **Both default to on, so nothing renders differently until a user unticks
one.** The transform destructures from `{ ...DEFAULT_FORM_DATA,
...formData }`,
so charts saved before this change pick the defaults up too. Where an axis
had
no `axisTick` key at all, the ticked state writes `show: 'auto'` rather
than
`show: true` — `'auto'` is ECharts' own default, and it resolves to
*hidden*
on a banded category axis. Forcing `true` would have added tick marks that
bar charts do not currently draw.
- **The small-chart behaviour is preserved.** Upstream already suppresses
gridlines and ticks below `compactChartHeight`, and the new controls are
written as `isSmallChart ? false : gridlines` — matching the line above
them —
so a ticked box cannot put gridlines back on a chart too small to carry
them.
- **The Mixed chart's secondary y-axis keeps `splitLine: { show: false }`
unconditionally**, as it does today, so the two grids can never double up.
The control governs the primary axis only.
- **On the Mixed chart, Axis ticks has nothing to act on in the common
case.**
Neither of its axes overrides `axisTick`, and ECharts' `'auto'` resolves to
hidden whenever the other axis is a category or time scale — which covers
most
Mixed charts. The checkbox is subtractive, so with no ticks drawn there is
nothing for it to hide. Making it meaningful there would mean *adding*
ticks
by default, which this PR deliberately does not do. Happy to drop the
control
from the Mixed panel instead if reviewers prefer.
Motivation: a sparse, low-gridline chart currently requires hand-written
JSON in
*Customize → ECharts Options*. Background:
https://github.com/apache/superset/discussions/43426
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
**Before** — gridlines and axis ticks as they render today.
<img width="1737" height="1079" alt="gridline_controls_before"
src="https://github.com/user-attachments/assets/2407593c-a5d9-4e09-9524-044effeb1a8d";
/>
**After** — the same chart with both unticked.
<img width="1737" height="1079" alt="gridline_controls_after"
src="https://github.com/user-attachments/assets/8b7d06f3-4192-4566-814d-02d35c9e9d3d";
/>
**The two new controls**, on the Customize tab beside the minor pair they
mirror.
<img width="502" height="979" alt="gridline_controls_ui"
src="https://github.com/user-attachments/assets/4f2070b7-1c75-415e-801a-65a243103694";
/>
### TESTING INSTRUCTIONS
Unit tests: `npm run test --
plugins/plugin-chart-echarts/test/Timeseries/transformProps
plugins/plugin-chart-echarts/test/MixedTimeseries/transformProps`
Manually, on any Bar or Mixed chart:
1. Open an existing chart that has never seen these controls and confirm it
renders exactly as before — this is the regression that matters most.
2. Customize → untick **Gridlines**. The horizontal lines across the plot
area
disappear; the axis labels and ticks stay.
3. Untick **Axis ticks**. The small marks on both axes disappear.
4. Re-tick both and confirm the chart returns to its original look.
5. On a Bar chart, set Orientation to Horizontal and untick **Gridlines**.
The
gridlines still follow the value axis, which is now the horizontal one.
6. Shrink a chart on a dashboard below 100px tall. Gridlines and ticks stay
hidden regardless of the checkboxes, as they do today.
#### Charts verified by hand
| Chart | Gridlines | Axis ticks |
| --- | --- | --- |
| Bar (vertical and horizontal) | works | works |
| Line | works | works |
| Area | works | works |
| Scatter | works | works |
| Mixed | works | nothing to hide — see above |
| Step, Smooth Line | not checked separately | not checked separately |
Step and Smooth Line share `Timeseries/transformProps.ts` with Bar, Line,
Area
and Scatter, with no branching between them, so the four checked charts cover
the same code. Histogram, Waterfall and Box Plot are untouched: they carry
neither `minorTicks` nor `minorSplitLine`, and this PR only adds controls
where
the minor pair already exists.
Two things noticed while testing that are **not** caused by this PR,
recorded in
case they are news:
- On Area, the existing **Minor ticks** control draws nothing, while the same
control works on Bar, Line and Mixed. Those charts run the identical
transform, and this PR does not touch `minorTick` in any file, so the cause
lies in how ECharts computes minor tick positions for that axis extent
(`getMinorTicksCoords` returns an empty list and the builder bails).
- On Mixed, minor ticks render in a different colour from the gridlines. That
is ECharts' own defaulting: `minorTick` inherits its stroke from
`axisTick`,
falling back to the axis line colour, whereas `splitLine` has its own theme
token.
### ADDITIONAL INFORMATION
- [x] Has associated issue:
https://github.com/apache/superset/discussions/43426
- [ ] 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
- [x] 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]
