-
-
Notifications
You must be signed in to change notification settings - Fork 240
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
Fix fact sorting for uptime #591
Conversation
1 similar comment
Hi @jgrammen-agilitypr ! I think that you almost have it working. I would just add an To do that you could add an extra boolean parameter to the And the value of this flag could be passed from the |
Codecov Report
@@ Coverage Diff @@
## master #591 +/- ##
==========================================
- Coverage 82.77% 82.72% -0.06%
==========================================
Files 12 12
Lines 900 903 +3
==========================================
+ Hits 745 747 +2
- Misses 155 156 +1
Continue to review full report at Codecov.
|
ok, I have rebased on voxpupuli:master |
Thanks @jgrammen-agilitypr ! I meant that I imagine that we want to enable the natural time delta sorting only for the specific facts, like "uptime". So in the And then we can pass this parameter as a value for |
…g to table rendering.
ok, I made more adjustments, moved the natural_time_delta_sort around. and made an attempt at modifying the fact function. |
Almost there @jgrammen-agilitypr, check out my comments please. |
…e set natrual sort in macros.html
I made my best attempt at correcting your 2 comments. Thank you for your patience with me as I struggle through this. |
1. Provide the natural_time_delta with a default value for the macro to work on other views than /fact/... 2. Set "natural-time-delta" type in "columnDefs", like documented. 3. Modify natural-time-delta.js to make it work for our case where: a) the value is in fact <a> element - take non-formatted value from the newly added "title" field. b) process values like "3:45 hours". Also refactor this script to make the code easier to understand. 4. As we use natural-time-delta only in /fact/... view, load the new script only there.
I made it work with my commit, @jgrammen-agilitypr. Please check out its commit message to know what I changed. 🙂 Please just rebase and in my opinion then it will be ready to merge. (We also think that in the long term we should rather switch to DataTables plugins to:
Update: we should switch to https://datatables.net/manual/data/orthogonal-data.) |
My branch has been rebased on the 'voxpupuli:master' branch. If you would prefer that I abandon this work and and make some attempt at datatables orthogonal data, just let me know |
I prefer to be pragmatic, so in my opinion if we have a working feature here and the code is ok, then we should merge, release it and let ourselves and the community already use what you did. If you have the time and will to start working on a more mature and universal solution then please go ahead and to it in a separate PR. But I am not the only (de facto) maintainer here - what are your opinions, @bastelfreak / @mterzo / @corey-hammerton ? |
an attempted fix that doesnt appear to work, and may have consequences for other pages