Skip to content
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

[SFAII] improve reports with ability to sort graphs/tables by metric values #796

Closed
2 tasks done
awig opened this issue Sep 22, 2022 · 8 comments · Fixed by #802
Closed
2 tasks done

[SFAII] improve reports with ability to sort graphs/tables by metric values #796

awig opened this issue Sep 22, 2022 · 8 comments · Fixed by #802

Comments

@awig
Copy link
Member

awig commented Sep 22, 2022

  • sort table columns ascending/descending
  • sort graphics (see if we can add a button to sort them by value instead of default in the chart)
@awig
Copy link
Member Author

awig commented Dec 23, 2022

implemented table sort in javascript. hardest part figurring where and how to figure to include it in the flask/jinja2 templates but can apply to the other tables besides metrics. @dplarson thoughts? could add up/down arrows too but that doeesn't seem that important

Screen.Recording.2022-12-23.at.12.27.50.PM.mov

@awig
Copy link
Member Author

awig commented Dec 23, 2022

Implemented for all tables besides the "Table of data alignment parameters".

@dplarson
Copy link
Contributor

dplarson commented Jan 3, 2023

@awig looks good and shows the functionality I was expecting. But I do think up/down arrows would be helpful for users as otherwise there isn't a visual indicator that you can sort. (Ideally the up/down arrows would change dynamically as you click to show you which way you are sorting, e.g., both up & down arrows when unsorted, up arrow only when sorting ascending and down arrow only when sorting descending. However, I think even static arrows would be helpful, i.e., add an up/down arrow icon next to each column name to indicate that you can sort but then the icon doesn't change when you click.)

@awig awig mentioned this issue Jan 4, 2023
7 tasks
@awig
Copy link
Member Author

awig commented Jan 5, 2023

Took a bit of playing around with but added the arrows with highlights for sorting.

sort_with_arrrows.mov

@dplarson
Copy link
Contributor

dplarson commented Jan 5, 2023

Looks great to me 👍

@awig
Copy link
Member Author

awig commented Jan 6, 2023

I've added the sort Total graphs with dropdown menus as shown in this video:

sort_total_graphs.mov

Couple of questions/comments for @dplarson

  • Suggestions on the layout/look? i tried a lot of different ways to try the layout but depending on the number of metrics the titles, labels, the look can get very difficult to get looking great. this is the most consistently decent I could get.
  • Note that all the sorted datafrrames are computed prior to display and it is just choosing which one to make visible. May add just a little bit to the compute time but should be negligible.
  • If there is only forecast (x category), I suppress the sort since it wouldn't provide anything.
  • Most importantly, what other categories should this be added to?
    • since they are all temporal categories (time of day, season, year), does it add value to sort those categories? Of course I could add similar logic but i think graphs of say time-of-day with the hours all jumbled up quite odd to look at.

@dplarson
Copy link
Contributor

dplarson commented Jan 6, 2023

@awig I'm happy with the layout/look as shown and I don't think there is a need to add sorting to other categories at this time. (It's possible we may want to add sorting to more places later but I think that will become apparent once we get more folks using this functionality and can then address that through a later PR if necessary.)

@awig
Copy link
Member Author

awig commented Jan 6, 2023

Thanks. I'll go ahead and push the branch and the PR, double check codecov and linter and then it should be good for your to review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants