nytai opened a new pull request, #45394:
URL: https://github.com/apache/superset/pull/45394
### SUMMARY
The "Tooltip sort by metric" control promises rows in descending order of
the selected metric. Two independent defects prevented that. Unstacked line
charts were affected by neither, which is why the control appears to work on
some chart types and not others.
**1. Stacked charts reverse the sorted rows.**
`Timeseries/transformProps.ts` runs `rows.reverse()` whenever the chart is
stacked, so the tooltip reads in the same order the series are stacked. That
reverse was added in #23392 (a series-sorting PR) directly below the
pre-existing `tooltipSortByMetric` sort, and runs unconditionally — so it also
inverts the metric sort, rendering descending as ascending. This is what #35673
reports.
#42106 diagnosed the same line and chose to document the behavior rather
than change it, noting the behavior fix as a possible follow-up. This is that
follow-up, so the control description returns to describing a single consistent
ordering.
The reverse's actual purpose is preserved: it still applies whenever sorting
by metric is off, which is the default. Only charts that explicitly enable the
control change.
**2. The sort cannot read object-shaped data items.** `extractTooltipKeys`
read the y value off the raw `data` item, which is only a plain tuple for the
simplest series. Bar series wrap the item in `{ value, label }` to reposition
labels for negative values, and in `{ value, itemStyle }` when colored by the
primary axis. For those, `data[yIndex]` is `undefined`, the comparator returns
`NaN`, and the rows come out in no particular order at all.
Reading the value off `value` — echarts' normalized view of the item, and
the field the tooltip rows themselves are already built from — handles every
shape. Non-finite values (nulls in a gap, missing points) now sort last instead
of poisoning the comparator. This path is shared with Mixed Timeseries, which
gets the fix as well.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Not included — the change was developed and verified against the tooltip
formatter directly rather than in a browser. Happy to add captures if a
reviewer wants them.
### TESTING INSTRUCTIONS
1. Create a stacked Area or Bar chart with several series of clearly
different magnitudes.
2. Enable **Rich tooltip** and **Tooltip sort by metric**, then hover a
point. Rows are ordered highest to lowest. Before this change they were ordered
lowest to highest.
3. Uncheck **Tooltip sort by metric**. Rows return to stack order
(top-of-stack first), unchanged by this PR.
4. On an unstacked Bar chart containing at least one negative value, enable
the control and hover. Rows are ordered highest to lowest. Before this change
they were not sorted at all.
Automated coverage: 9 new tests in `test/Timeseries/transformProps.test.ts`
and `test/utils/series.test.ts`, covering unstacked line, stacked area,
stream-stacked area, stacked bar, bar with negative values, bar colored by
primary axis, the control-disabled case, and the two data shapes at the unit
level. Each behavioral test was confirmed failing against the unpatched source.
### ADDITIONAL INFORMATION
- [x] Has associated issue: fixes #35673
- [ ] 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 a user-visible behavior change for stacked charts with the control
enabled, so it carries an `UPDATING.md` entry under **Next**.
--
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]
