Issue 10 member pages #61
Conversation
…auses/game-dev into issue-10-Member_pages
…plate but idk what im doing
…from "idk bruh merge conflict"
…orrectly displays data
…auses/game-dev into issue-10-Member_pages
…auses/game-dev into issue-10-Member_pages
…fied member profile spacing
client/public/frame.png
Outdated
There was a problem hiding this comment.
What's the source for this image? Are we sure we have the rights to use it?
(An SVG is preferable if possible)
There was a problem hiding this comment.
The image is directly from the Figma design, so I assumed it is ok to use - but will remove if not.
| </div> | ||
| </div> | ||
| </div> | ||
| <div className="m-auto min-h-80 w-11/12"> |
There was a problem hiding this comment.
We should remove these placeholders before merging into main
| <div className="m-auto h-fit bg-[#1B1F4C] text-[#CED1FE]"> | ||
| <div className="border-y-[#9CA4FD]lg:mx-10 mx-2 flex flex-wrap items-center justify-center gap-y-5 py-7"> |
There was a problem hiding this comment.
Best practice would be to use the existing defined colours or define new ones in the globals.css
| </div> | ||
| <div className="flex w-4/5 flex-col gap-2 rounded-md p-2.5 font-firaCode"> | ||
| <div className="font-jersey10 text-4xl"> | ||
| {" "} |
There was a problem hiding this comment.
Not sure what this is doing.
client/src/hooks/useMember.ts
Outdated
| if (!res.data.active) { | ||
| throw new Error("Member is not active; raising error"); | ||
| } | ||
| return res.data; |
There was a problem hiding this comment.
This should be a backend check, not frontend. To the frontend there should be no difference between inactive and non-existent members.
client/src/pages/members/[id].tsx
Outdated
| const { data: member, isLoading, isError } = useMember(id); | ||
|
|
||
| if (isLoading) { | ||
| return <p>Loading member...</p>; |
There was a problem hiding this comment.
We can just leave it blank while loading.
server/game_dev/serializers.py
Outdated
| "about", | ||
| "pronouns" | ||
| "pronouns", | ||
| "active" |
There was a problem hiding this comment.
Whether the member is active should not be sent to the frontend. To an outside observer there shouldn't be a difference between non-existent and inactive members
… sanitised correct input
|
Could you please add screenshots of any UI changes to the PR? thanks :) |
|
Here is the updated UI: |
Change Summary
Change Form
Fill this up (NA if not available). If a certain criteria is not met, can you please give a reason.
Other Information
[Is there anything in particular in the review that I should be aware of?]
Related issue