TimeSeries/BarChart: Add support for sorting series in the tooltip #43615
Conversation
dprokop
requested review from
meanmina,
jackw,
joshhunt and
axelavargas
and removed request for
a team
dprokop
force-pushed
the
tooltip-sort-order
branch
from
dfb4b9c
to
a36f956
Compare
| * Null values are always sorted to the end regardless of the sort order | ||
| */ | ||
| function sortValues(sort: TooltipSortOrder) { | ||
| return (a: number | null | undefined, b: number | null | undefined) => { |
There was a problem hiding this comment.
| return (a: number | null | undefined, b: number | null | undefined) => { | |
| return (a: any, b: any) => { |
There was a problem hiding this comment.
@ryantxu this fn is now moved to grafana-data/arrayUtils and now has the support for string values (using localeCompare)
|
|
||
| if (sort === TooltipSortOrder.Descending) { | ||
| return b - a; | ||
| } | ||
| return a - b; |
There was a problem hiding this comment.
| if (sort === TooltipSortOrder.Descending) { | |
| return b - a; | |
| } | |
| return a - b; | |
| const order = a > b ? 1: -1; | |
| if (sort === TooltipSortOrder.Descending) { | |
| return order * -1; | |
| } | |
| return order; |
Can we implement this so it also works for string inputs? Maybe something like the above (untested!!!) approach. For the time series tooltip it likely makes no difference, but if forced to numbers, then we will for sure need custom implementations for other panels (though we are already working on that anyway)
There was a problem hiding this comment.
you'd want to use https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/localeCompare
if you want it to be fast, you'd first init & cache an Intl.Collator instance and use that: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/localeCompare#performance
There was a problem hiding this comment.
That's done now, thanks for the feedback folks!
That's tru and it's because of the custom tooltip being used there. But In the latest comit I've added this feature to BarChart too |
dprokop
changed the title
dprokop
removed request for
meanmina,
jackw,
joshhunt,
axelavargas and
oscarkilhed
| @@ -230,6 +235,10 @@ export const TooltipPlugin: React.FC<TooltipPluginProps> = ({ | |||
| }); | |||
| } | |||
|
|
|||
| if (sortOrder !== SortOrder.None) { | |||
| series.sort((a, b) => arrayUtils.sortValues(sortOrder)(a.value, b.value)); | |||
There was a problem hiding this comment.
In this, a.valueandb.value is formatted display value, whether should use the original value to sort ? I am not clearly on the context, hope someone can explain it if i wrong :)
There was a problem hiding this comment.
Can you please open a separate issue with your findings?
Closes #38861
This PR brings ability to sort series in the Timeseries panel's tooltip as it was possible with the old graph panel. Additionally, a migration for legacy tooltip options is added.
Note that the
null/undefinedvalues are sorted at the end of the tooltip regardless the sort order configured.Screen.Recording.2022-01-03.at.14.10.24.mov