-
Notifications
You must be signed in to change notification settings - Fork 22
Remove datatable-sort-badge when only one column is sorted #1110
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1110 +/- ##
=======================================
Coverage 71.35% 71.35%
=======================================
Files 63 63
Lines 4151 4151
=======================================
Hits 2962 2962
Misses 1189 1189 🚀 New features to boost your workflow:
|
datalab
|
Project |
datalab
|
Branch Review |
bc/datatable-sorted
|
Run status |
|
Run duration | 07m 29s |
Commit |
|
Committer | Ben Charmes |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
0
|
|
0
|
|
0
|
|
477
|
View all changes introduced in this branch ↗︎ |
8f1df30
to
f88673a
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.
Having some trouble testing this locally, it seems to work well for the ID column, but I'm getting errors when trying to filter with any of the enum columns (e.g., item type, creators). Does this work for you locally @BenjaminCharmes?
I've just tried again and everything seems to be working fine for me |
Ah, I think I'm seeing some unrelated problem then. It also works well when sorting for me, but if I try to add a filter I'm getting a runtime error from the dev server. However, I also see the same error on main now 🙃 so I'll have to look into it...
EDIT: I think this is perhaps related to the last primevue update that I merged in #1138 -- will see if reverting it helps. |
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.
Thanks @BenjaminCharmes, this does seem to work well! I think it would be a good idea to add a couple more tests for the data table component tests for search, sort and filtering (also with up to date example data too with block types, nfiles). I'll raise a separate issue to track this!
Closes #1108
The numbers always appear when you're in
sort-mode="multiple"
and prime-vue only removes them when you're insort-mode="single"
. This allows the user to know that they can sort several columns at once.I guess we can find a way of removing the "1" when only one column is sorted, but I couldn't find anything in the prime-vue docs, or even using the custom theme we have in datalab (the two columns have exactly the same css classes), so I had to add another custom css class.
It's a first attempt to see what it could be.