Cornellians Say Frontend #484
Conversation
|
[diff-counting] Significant lines: 143. |
There was a problem hiding this comment.
Great work on Cornellians Say frontend, Helen! Looks really smooth on my end, in all view sizes. Left a couple comments, on styling mostly, but this is super great work. Perhaps before merging both yours and Jacqueline's features, we can align on some nitpicky styling things (ex. font sizes, padding) because both components are meant to be similar looking -- this could happen in a middle-step PR before going to main maybe, but really exciting!
| * and a valid summary generated. | ||
| */ | ||
| type SummaryProps = { | ||
| classSummary: string; |
There was a problem hiding this comment.
Maybe instead of not showing the CornelliansSay box if there isn't a course summary, we could have a default message that appears?
| display: flex; | ||
| align-items: center; | ||
| gap: 10px; | ||
| margin-bottom: 20px; |
There was a problem hiding this comment.
The margin on the bottom looks a little big imo
| font-weight: bold; | ||
| } | ||
|
|
||
| .summary { |
There was a problem hiding this comment.
I feel like it looks a bit weird for the summary text to be different from the review text - so maybe no need to format this
There was a problem hiding this comment.
Esp the line height looks a bit funky
| difficulty={selectedClass.classDifficulty} | ||
| workload={selectedClass.classWorkload} | ||
| /> | ||
| <div> |
There was a problem hiding this comment.
I think this div may be unnecessary?
| gap: 10px; | ||
| } | ||
|
|
||
| .tag { |
There was a problem hiding this comment.
I think the tags are too big somewhat, maybe reduce some of the padding
There was a problem hiding this comment.
Try
padding: 0 10px;
min-height: 25px;
It also helps when the tag is too long on certain screen widths the text stacks on top of each other and clips the tag container.
Summary
This PR works on the Cornellians Say feature.
PR Type
Mobile + Desktop Screenshots & Recordings
QA - Test Plan
Checked for courses that had summary and all tags, summary and some tags, and no summary. Also checked on different screen sizes.
Breaking Changes & Notes
Added Cornellians Say component to the frontend.
Added to documentation?
What GIF represents this PR?
gif