-
Notifications
You must be signed in to change notification settings - Fork 14k
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(TimeTableViz): sort by first metric #18896
Conversation
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.
any way we can add a test for this?
cd09cee
to
0ff7468
Compare
Codecov Report
@@ Coverage Diff @@
## master #18896 +/- ##
==========================================
+ Coverage 66.21% 66.41% +0.20%
==========================================
Files 1633 1640 +7
Lines 63210 63478 +268
Branches 6409 6409
==========================================
+ Hits 41852 42159 +307
+ Misses 19698 19659 -39
Partials 1660 1660
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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, thanks for the test!
(cherry picked from commit 760dab9)
SUMMARY
Sort
TimeTableViz
(time series table) by the first metric descending. Currently, no sorting is applied, so time series tables show the "first" n series, which can be confusing to users and isn't very helpful. Sorting by the first metric descending is more consistent with other chart types.Note time series tables do not have a sort by control - might be better to add this in the future, but this should still an improvement for now.
TESTING INSTRUCTIONS
Create time series viz. Confirm that an order by clause exists in the query.
ADDITIONAL INFORMATION
@john-bodley @ktmud ?