-
Notifications
You must be signed in to change notification settings - Fork 3
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
Added Sort Functionality #291
Conversation
} ); | ||
</script> | ||
{% endblock %} | ||
|
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.
Since this is repeated a few times, what do you think about moving it to a macro?
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.
Yes I also thought about this but eventually went ahead since I was not sure about the exact file for putting it . I am thinking of adding an additional file in macros for this would that be okay ?
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.
That sounds great! Thanks, @Sherwin-14 :)
@@ -94,6 +94,13 @@ | |||
return new bootstrap.Tooltip(tooltipTriggerEl) | |||
}) | |||
</script> | |||
|
|||
<script src="https://code.jquery.com/jquery-3.5.1.js"></script> |
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.
Is JQuery a required dependency of DataTables? These days there's document.querySelector()
which can do lots (all?) of the same selection magic as JQuery, so I've been getting away without it.
What do you think?
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.
It seems like JQuery is a required depedency of DataTables however the website provides an example on how you can sort a table without calling into JQuery (it is still a dependency and has to be loaded though)
https://datatables.net/examples/non_jquery/init.html
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.
Actually I have found an alternative solution where jQuery wont be required. It can be found in the following link https://fiduswriter.github.io/simple-datatables/documentation/
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.
Good find! I really like the idea of using a lighter-weight library. Do you think that would be an improvement as well?
Please don't feel obligated to change, I just wanted to make sure JQuery was required for DataTables if we are including it :)
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 think we should go for a solution with the lighter-weight library. I feel it would be better choice although it looks a bit different from DataTables with more spaces between the sorting buttons. but yeah if we have an option of choosing a lighter-weight library then maybe we should proceed with it .
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.
Right on, let's do it! Thanks for your input :)
@Sherwin-14 sorry this has been sitting so long! We're all prioritized on different projects for now, but hopefully will have time to get to this soon. We did just release your recent change to the node colors! Thanks for your work :) https://github.com/nsidc/usaon-benefit-tool/releases/tag/v2.0.2 |
I have added the sort functionality using DataTables. Please have a look at it .
resolves #218
📚 Documentation preview 📚: https://usaon-benefit-tool--291.org.readthedocs.build/en/291/