Add sorting to Skunk HTML report - #142
Conversation
JuanVqz
left a comment
There was a problem hiding this comment.
Hey @poonambhagaur61-tech, Thanks for taking the time to contribute. I reviewed and have some questions for the issue creator. Feel free to share your opinions as well.
As for me, the changes looks good in general, so, let's wait for him, but if no response we can decide what to do, thanks!
There was a problem hiding this comment.
When loading the report, the arrow icons do not show up
They should be visible like so; however, I'm unsure if they show a neutral hint or the actual initial sort preference. The reports start with:

For the neutral hint, I added this .table-header::after { content: " ⇅"; opacity: .35 }
@fbuys,
Would you be happy with a neutral hint or more like an initial arrow that matches the actual pre-sorted order?
And, since you are around, what do you think about the sorting implementation? As for the code, I'd say it looks good, but as for what columns you want to be sorted, What are the criteria to follow, etc.?
There was a problem hiding this comment.
Thank you for the helpful feedback. I’ve updated the report to indicate the initial sort direction. Since the report is initially sorted by Skunk Score in descending order, the Skunk Score column now displays the descending arrow by default, while the other columns remain neutral until selected. I’ve also kept the existing numeric and alphabetical sorting behavior unchanged.
Please let me know if you have any further suggestions or would prefer a different sorting behavior. Thank you again for your guidance!
JuanVqz
left a comment
There was a problem hiding this comment.
Thanks for the update, @poonambhagaur61-tech! Showing the arrow only on the column being sorted works well, and the report now opens with Skunk Score ▼, which matches the default order.
Please add an entry to CHANGELOG.md under "main (unreleased)", something like: * [FEATURE: Add sortable columns to the Skunk HTML report](https://github.com/fastruby/skunk/pull/142) and address the code changes I left as diff please.
Co-authored-by: Juan Vásquez <javasgon@gmail.com>
Co-authored-by: Juan Vásquez <javasgon@gmail.com>
Co-authored-by: Juan Vásquez <javasgon@gmail.com>
Co-authored-by: Juan Vásquez <javasgon@gmail.com>
|
Thank you for the feedback! I’ve added the requested CHANGELOG.md entry under the unreleased section and linked it to PR #142. I’ve also addressed the requested changes. Please let me know if any further adjustments are needed. |
Co-authored-by: Juan Vásquez <javasgon@gmail.com>
Co-authored-by: Juan Vásquez <javasgon@gmail.com>
|
No problem! I’ve updated the CHANGELOG.md as requested. Thank you for your guidance and feedback throughout the process. I’d be grateful if you could consider merging the PR when everything looks good. |
|
@poonambhagaur61-tech thank you for taking the time to contribute to our gem. have you ever heard about hacktoberfest? Is going to be running next month if you want to contribute I’ll probably be adding this gem to it, so, if you contribute it will count on there. |
|
Thank you so much, Juan! Yes, I’ve heard about Hacktoberfest, and I’d definitely love to participate. I really enjoyed contributing to Skunk and learned a lot from the review process. I’d be happy to continue contributing to the gem during Hacktoberfest. Thanks again for the opportunity and your guidance! |
This PR adds sortable columns to the Skunk HTML report.
Tests: 88 runs, 126 assertions, 0 failures, 0 errors, 0 skips
Scope: sorting applies to the HTML report (
skunk_overview.html) only. The console and JSON reports keep their current output, ordered by Skunk Score.Closes #116