website: Implement random course picker button #4039
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@Glenn-Chiang is attempting to deploy a commit to the modsbot's projects Team on Vercel. A member of the Team first needs to authorize it. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #4039 +/- ##
==========================================
- Coverage 54.52% 52.80% -1.72%
==========================================
Files 274 290 +16
Lines 6076 6698 +622
Branches 1455 1631 +176
==========================================
+ Hits 3313 3537 +224
- Misses 2763 3161 +398 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi @Glenn-Chiang, thanks for the PR! Your code looks fine, and I think the course search page is a good place to put this feature. However, I think the feature would be more useful if it respected the search filters, so I've pushed some changes to support that as well. Sorry for the late review! |
leslieyip02
changed the title
|
Not a blocker but I would suggest reviewing the UI/UX here a little. The button is quite large and prominent for a feature that I'm not sure many students would use. Also the tooltip goes over the search bar |
|
@ZhangYiJiang Maybe this would be better? I made the button more subtle and shifted the tooltip placement to bottom instead.
|
There was a problem hiding this comment.
The new placement is better, but you might still want to iterate on this.
First it's best to avoid having interactive (the random course button) and non-interactive elements (the courses counter) share the exact same design, since that makes it hard to distinguish what you can interact with
Second I'm not sure adding the icons is a good idea since it doesn't really communicate anything that the text does not already, and kind of just adds noise, the page is already quite complex so you want to minimize these
website/src/views/modules/ModuleFinderContainer/ModuleFinderContainer.tsx
Outdated
Show resolved
Hide resolved
|
Another option would be the bottom of the filter list? This would allow the button to not be as jarring, but still visuallly look like a button. 
|
|
I think that's a better placement yeah. The only change I would ask for is you're currently using a primary, which is typically reserved for the primary interactive element on the screen - think the submit button in a form for example. Here it's btn-secondary or even btn-link, since it's a navigational link of sorts, probably makes more sense |
Context
Closes #3671 random course picker button
Implementation
I added a "Random Course" button to the /courses page. When the button is clicked, the user will be redirected to the page of a random course.
Other Information
Hi, this is my first time contributing to an open source project. I hope that my changes are acceptable and look forward to making more substantial contributions as I learn more about the codebase.