Skip to content

Restrict timeseries chart to top 20 users #709

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

Merged
merged 2 commits into from
Nov 8, 2022

Conversation

LanesGood
Copy link
Member

This PR restricts the Time Series Edits Chart to the top users, sorted by edit, up to 20. The limit of 20 users is somewhat arbitrary, set only by the amount of vertical space available for the legend.

image

@LanesGood LanesGood marked this pull request as ready for review November 4, 2022 20:26
@LanesGood
Copy link
Member Author

Marking this as ready for review @maxgrossman @kamicut. I think the main question here is: do admins want to see timeseries charts with more than 20 users? If so, this PR can be closed. However, I think that the number of bars when there are more than 20 users becomes overwhelming.

Copy link
Member

@kamicut kamicut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me and ready to merge, but I think @maxgrossman should reply to the user case question. I suggest we merge and then work on an enhancement for the over 20 users use case (maybe paginate by using the .slice as a window into the users array @LanesGood).

@maxgrossman
Copy link
Collaborator

I think this is cool. I think that 'top x' users makes lots of sense when filtering for edits by big teams or in a country, but less so when I am just looking for a few users.

Maybe if the query returns less than 20 users 'top x users' gets omitted.
Think this could be addressed later. future paginating can address this.

@kamicut kamicut merged commit a8fb744 into enhance/timeseries-ux Nov 8, 2022
@kamicut kamicut deleted the restrict-timeseries-chart branch November 8, 2022 14:29
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 this pull request may close these issues.

3 participants