Conversation
dburkhart07
left a comment
There was a problem hiding this comment.
some very small things. amazing initial pass! 😴 👎
| if (isLoading) { | ||
| return ( | ||
| <Center minH="100vh"> | ||
| <Spinner /> |
There was a problem hiding this comment.
Note to @sam-schu and @Yurika-Kan: I've noticed the usage of the isLoading + Spinner combo has been used pretty inconsistently throughout the app. We have a lot of attributes that make API calls and could use an isLoading attribute, for a potential future cleanup ticket. We should decide though whether we want to make most API calls like this utilize a loader, or just not use it at all.
dburkhart07
left a comment
There was a problem hiding this comment.
Correct me if I'm wrong, but I feel like this whole profile system is unnecessarily abstracted, to the point where it just feels kinda hard to read and work with for future changes we may make. Could we just have this structure instead:
profilePage.tsx: General wrapper that has the "Profile" header at the top, and then calls components profileLeftPanel, and profileAccountInfo components.
profileLeftPanel: Can keep as is
profileAccountInfo: Should have the info of both the account and layout, with all the saving and editing functionality. Passing the editing function as a prop seems somewhat extra, and I feel like we could easily just keep track of a simple useState editing or not in this component. The saving functionality I get since we need to fetch them again, but the editing seems over-abstracted. With that, I also feel like we really only need 1 component, and should be able to merge profileAccountInfo and profileLayout together.
lmk if u disagree ✂️
dburkhart07
left a comment
There was a problem hiding this comment.
small things so imma trust to approve. we are so back! 💃 8️⃣ 0️⃣
Yurika-Kan
left a comment
There was a problem hiding this comment.
good stuff!!!!
small nits but lgtm
so exciting to have profile pages ~ thank you amy!
ℹ️ Issue
Closes SSF-166
📝 Description
frontend/src/components/profileprofilePageinfrontend/src/containers, route toapp.tsx(http://localhost:4200/profile), and link to profile page in homepage✔️ Verification
verified pages render properly for different roles and changed tests pass



