-
Notifications
You must be signed in to change notification settings - Fork 13.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(plugin-chart-echarts): add x-axis sort to multi series #23644
Conversation
export function sortRows( | ||
rows: DataRecord[], | ||
xAxis: string, | ||
xAxisSortSeries: SortSeriesType, | ||
xAxisSortSeriesAscending: boolean, | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I originally set out to implement this with lodash
, similar to how sortAndFilterSeries
was implemented, but in this case it felt clearer to write this out like this. But if this function looks too explicit I can rewrite this with the equivalent lodash
functions (refactoring should be simple, as the tests should work as-is).
Codecov Report
@@ Coverage Diff @@
## master #23644 +/- ##
==========================================
+ Coverage 67.93% 67.94% +0.01%
==========================================
Files 1918 1918
Lines 73891 73935 +44
Branches 8058 8078 +20
==========================================
+ Hits 50199 50237 +38
+ Misses 21633 21632 -1
- Partials 2059 2066 +7
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
79a5171
to
bedaf4e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
SUMMARY
Currently it's only possible to sort non-temporal charts by the series values if there are no dimensions selected. This adds the same sorting options to dimensional charts that's available for sorting the series. We still default to category name, so current charts will not be affected. This PR also moves the series sorting controls to
@superset-ui/chart-controls
to make them generally available.Also note that in this case we're doing the sorting in the frontend, while in the dimensionless case we're doing the sorting in the backend. In the future we may want to consider adding this type of logic to the backend, or move all sorting to the frontend.
multidim-sort.mp4
ADDITIONAL INFORMATION