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

Featured and All Benchmarks #577

Merged
merged 5 commits into from
Jan 1, 2023
Merged

Featured and All Benchmarks #577

merged 5 commits into from
Jan 1, 2023

Conversation

OscarWang114
Copy link
Collaborator

@OscarWang114 OscarWang114 commented Dec 31, 2022

This PR allows users to toggle between featured and all benchmarks. It also shows the preferred usernames.

Currently, the UI layouts of featured and all benchmarks are still identical, where each benchmark is shown as a card. I was planning to change the layout of all benchmarks to a table (like how systems are displayed) but realized more clarifications are needed. I've put my questions in issue #574 .

Backend

  • Created a new DB collection benchmark_featured_list under DB metadata to store the list of featured benchmark ids. This allows admins to maintain the list easily and doesn't require redeployment when the list is modified. Since the list is read-only, it might be even better to store it on Cloud Storage. I might need some pointers on how to create it on GCP 🙏
  • Added preferred_usernames as a required field to BenchmarkConfig in openapi.yaml and modified related API endpoints.

Screen Shot 2022-12-31 at 12 01 55 PM

Frontend

  • Allow users to toggle between featured and all benchmarks. The toggle button is only active at the root page (when parent id == ""), see the last screenshot for an example where the button is inactive.
  • Shows preferred usernames.
  • Created a filter class to organize parent_id and show_featured parameters.
  • Bound the filter with URL params so users can share a page easily via URL, just like before.

Screen Shot 2022-12-31 at 12 20 50 PM

Screen Shot 2022-12-31 at 12 21 42 PM

Screen Shot 2022-12-31 at 12 58 00 PM

Copy link
Contributor

@neubig neubig left a comment

Choose a reason for hiding this comment

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

Thanks! I looked through this and didn't see any major issues, so happy to go ahead and merge. Thanks for wrapping this up!

@neubig neubig merged commit 0b20ec8 into main Jan 1, 2023
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