Adding sort button to Docs/Alerts list #2747
Conversation
| /* eslint-disable max-len */ | ||
| /* eslint-disable no-trailing-spaces */ | ||
| /* eslint-disable indent */ |
There was a problem hiding this comment.
These should actually be addressed instead of suppressed
src/index.js
Outdated
| /* rows.map((r) => { | ||
| console.log(`${JSON.stringify(r.columns)}`); | ||
| }); */ |
src/index.js
Outdated
|
|
||
| function addSortButton(el, idx) { | ||
| const img = document.createElement('img'); | ||
| img.src = "https://uxwing.com/wp-content/themes/uxwing/download/arrow-direction/up-down-arrows-icon.png" |
There was a problem hiding this comment.
Be better to have this locally and referenced relatively.
(Assuming no copyright/permissions issue)
|
Probably better to remove unrelated commits, squash it all, and use rebase (vs merge) in the future |
|
Okay, @kingthorin , thank you for the feedback. I will close this PR and after fixing it all, open it again. |
|
Thanks. For future reference you could have applied the changes to the existing branch/PR and just force pushed. |
|
Yes, preferable to keep it open. |
Signed-off-by: Lucas Bergholz <[email protected]>
|
I pushed to the repo the changes asked in this topic. I also took out the unrelated commits. The image used for the button is copyright free. |
|
Thanks @LucasBergholz I'll try to pull it and test it in the next few days. |
|
Filtering seems to be broken. You can filter, but then if you sort the filter is discarded and can't be re-applied. |
Co-authored-by: Ana Rocha <[email protected]> Signed-off-by: Lucas Bergholz <[email protected]>
|
Hello @kingthorin , i saw your feedback on the filtering and sorting not working together and made the necessary changes that resolve the bug you mentioned before. I created a array named elements which is updated every time a filtering happens. This way, the sort button only sorts the elements array, which contains the filtered lines of the table. Hope this makes it 100% functional now. Im glad to hear more feedbacks if needed! |
|
Thanks for tackling that. I probably won't get a chance to test it till Thursday. |
|
Works really well ... but, we already have support for sortable tables :/ |
|
A nice enhancement to the current mechanism would be to add hover over text like |
|
And sorry for not pointing this out earlier - I'd forgotten we'd added this, I was just searching for how to set the cursor as I remembered we had done that 😉 |
Proposal of sort button for the Alert Table. It identifies if the content of the column is a number or a string, and sort it in alphabetical order, or if it is a number, in upwards or downwards order. This is my first contribution in ZAP, so, if i can change anything to improve the functionality, let me know.