-
Notifications
You must be signed in to change notification settings - Fork 226
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
Start a new top table report type #545
Conversation
Co-authored-by: Yomi Eluwande <yomi.eluwande@gmail.com>
Co-authored-by: Yomi Eluwande <yomi.eluwande@gmail.com>
Is this ready for review? |
We still have a few TODOs left to do from the backend and UI side. If you want you can obviously give feedback already! |
I'll hold off until you deem it ready :) let me know when that's the case, looking forward to this! |
This adds side by side view for the top table and also sorting by table headers
What's the status on this? |
I need to finish the aggregation work. I wanted to do this in a stream but didn't end up streaming. I'll finish it regardless then. |
I just pushed my latest UI changes to make the table look nice. it has all the agreed features e.g sorting. @metalmatze it'd be nice to also receive the units from the backend so we can use that to format the numbers in the table. Lastly, did we say we wanted to have additional columns for |
Very nice! |
sounds good! |
This way the state of the profile view (be it icicle, table or both of them) is preserved when the URL is shared.
Another round of reviews would be perfect now! |
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.
Amazing!! lgtm 🎉
We need to follow up in a couple of places:
Other than that we need to obviously build the UI parts :)