Course details components (professors)#390
Course details components (professors)#390lyangji1011 wants to merge 15 commits intobog-f25-sprint4from
Conversation
### Summary Resolves #353 <!-- What does this PR change and why? Discuss any breaking changes. --> caches seating info and removes tooltip from existing design. conditionally renders seat and waitlist info based on color of background. ### How to Test flame my code but my functionality is flawless bog --------- Co-authored-by: Uma Anand <63426041+uma-anand@users.noreply.github.com> Co-authored-by: Uma Anand <uma2005anand@gmail.com>
### Summary - wrong font color - incorrect padding ### How to Test Deploy preview --------- Co-authored-by: Uma Anand <uma2005anand@gmail.com>
|
uma-anand
left a comment
uma-anand
left a comment
There was a problem hiding this comment.
Code is clean! Thanks for implementing the components in the specified way. I've requested a couple changes.
Also, curious about the light mode design, did you talk to Cynthia about it? Otherwise, you may want to check with her.
| display: flex; | ||
| align-items: center; | ||
|
|
||
| &:nth-child(1) { width: 60px; margin-right: 20px; } |
There was a problem hiding this comment.
confused about why we are using hardcoded width and margins. The widths are fine except for the last column. Also try to replace them with percentages. I notice paddings in the Figma- is there a reason you used margins?
| justify-content: center; | ||
| } | ||
|
|
||
| &:nth-child(1) { width: 60px; margin-right: 20px; } |
There was a problem hiding this comment.
same note as before. Also if this is repeating, try to combine with above.
|
Also, one thing @afazio1 pointed out is while we don't have mobile designs just yet, we still want to make this adaptive to other screen sizes. Right now, it doesn't resize even for smaller desktops. See if you can make it a little more responsive. |
|
I pretty much addressed everything:
|
uma-anand
left a comment
uma-anand
left a comment
There was a problem hiding this comment.
Looks good! You can
- Go ahead and remove the sandbox now
- Just make the metric card float to next line instead of cropping out for smaller screen sizes
And then you should be good to go!!
|
Mobile designs are now on the Figma for you to take a look at. Let me know if you have time to implement, otherwise I can take a look. |
uma-anand
left a comment
uma-anand
left a comment
There was a problem hiding this comment.
LGTM! Thanks for working through all the changes!
4 participants
Summary
Resolves #379
Breadcrumbcomponent with labels and linksMetricsCardcomponent that takes in a list of metrics and returns a card of them in a rowProfessorInfoCardcomponent that displays prof name and information about sections they teach. Did not include MetricsCard inside this card, since ticket said to ignore fetching data for metrics/sandbox/ProfessorInfothat displays each componentChecklist
How to Test
Testing sandbox page at
/sandbox/ProfessorInfo