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

Stats feature #215

Merged
merged 15 commits into from
May 13, 2024
Merged

Stats feature #215

merged 15 commits into from
May 13, 2024

Conversation

nevoniuk
Copy link
Collaborator

No description provided.

@nathanielrindlaub
Copy link
Member

nathanielrindlaub commented Apr 9, 2024

Hey @nevoniuk! Sorry about the delay, but I finally had a chance to take a closer look at your progress. It's looking great, and definitely on the right track. A couple thoughts/notes below... let me know if you have any questions.

  1. Layout: can we move the "reviewed count", "image count", and "multi reviewer count" cards to the top row (each occupying 1/3rd of the available space), followed by a 100% width "label list" bar chart in the row below that, followed by a 100% width "Reviewer List" below that?

  2. Responsiveness: Not sure how hard this would be, but ideally the width of the Label List should be 100% of it's card/container and adjust in response to changes in viewport width. Might need to use Rechart's ResponsiveContainer component? I'm not entirely sure I just did a quick google search and found this Stack Overflow post. Also, I'd love for the table in the "Reviewer List" to occupy 100% of its parent container's width as well.

  3. Bar chart: I like that you positioned the bars horizontally... I think that's smart b/c some projects have > 100 different labels. Is there any way to get all of those label names to show up on the y-axis? It seems like it's labeling every other bar, and the "ticks" sit in between the bars rather than in the middle of the bars. Also, can we sort those bars by label count so the top one is the category with the highest label count and the bottom one is the one with the lowest?

Screenshot 2024-04-09 at 3 34 11 PM

  1. Disclaimer text: The disclaimer text at the bottom looks like it's repeating itself. Also, It would be nice to move all of that into InfoIcon components (basically just an (i) icon with a tooltip - see screenshot of one below). Those info icons could live just to the right of the titles at the top of their respective cards.

Screenshot 2024-04-09 at 3 41 04 PM

Last thing: I didn't review your code super closely yet but I did notice some thinks that should have been caught by the linting step that Husky executes whenever you commit. Is that not happening when you commit to this repo? I can help you debug that if that's the case... feel free to reach out to me on Slack any time during the work week and we can screen share and step through it.

@nathanielrindlaub nathanielrindlaub merged commit ef862a5 into main May 13, 2024
2 checks passed
@nathanielrindlaub nathanielrindlaub mentioned this pull request May 13, 2024
3 tasks
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.

2 participants