Safities User Settings #102
Conversation
# Account ## Personal Information - First Name - Last Name ## Public Information - Hacker Tag (must be unique) - Check if profile is searchable # Profile ## Profile Photo ## Profile Data - Bio - Skills (tags) - Discord Username # Registration - Separate page, WIP
# Account ## Personal Information - First Name - Last Name ## Public Information - Hacker Tag (must be unique) - Check if profile is searchable # Profile ## Profile Photo ## Profile Data - Bio - Skills (tags) - Discord Username # Registration - Separate page, WIP
TODO: - Fix resume - Make the registration page prettier - Add a way to go back easier - Just better UI/UX - Figure out these TypeScript errors but it works?
TODO: - Fix resume - Make the registration page prettier - Add a way to go back easier - Just better UI/UX - Figure out RegisterFormSettings.tsx errors
TODO: - Fix resume - Fix sidebar - Add a way to go back easier - Loading buttons when updating
TODO: - Fix sidebar - Add a way to go back easier - Loading buttons when updating
TODO: - Fix sidebar - Add a way to go back easier - Loading buttons when updating
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Will take a look at the files themselves tomorrow. However, looking at the build it could use a bit of mobile optimization at least from what I saw on my phone. Functionality looks good though. |
# Conflicts: # pnpm-lock.yaml
- Improved mobile - Got rid of aside on mobile - Added more left margin - Added spinners to buttons Added a button to go back from registration Added profile picture viewer
|
Fixed the mobile optimization. Should be good now. Added a large amount of UI/UX ease, fixed resume, and drop downs. |
| with: { | ||
| registrationData: true, | ||
| profileData: true, | ||
| }, | ||
| where: eq(users.clerkID, userId!), |
There was a problem hiding this comment.
nit: why does userId need to have the ! behind it? It should already be not null at this point.
There was a problem hiding this comment.
This was to appease TypeScript. It thinks user may be undefined in the props argument.
There was a problem hiding this comment.
I will take a look at this.
| import { useAction } from "next-safe-action/hook"; | ||
| import { modifyAccountSettings } from "@/actions/user-profile-mod"; | ||
| import { Checkbox } from "@/components/shadcn/ui/checkbox"; | ||
| import c from "config"; | ||
| import { Loader2 } from "lucide-react"; | ||
|
|
||
| interface UserProps { |
There was a problem hiding this comment.
nit: There is an easier way to do this with just importing the table itself and using the inferSelect or using zod through the z.infer I believe. This serves to benefit us in the fact that it also becomes dynamic so this interface will never need to be updated.
There was a problem hiding this comment.
Adding on to this, and non-blocking for now, but super common types like this should live in a types.ts inside of our db package. For now, keep it in this file though. Just something worth noting.
| } | ||
|
|
||
| interface AccountSettingsProps { | ||
| user: UserProps; | ||
| } | ||
|
|
||
| export default function AccountSettings({ user }: AccountSettingsProps) { | ||
| const [newFirstName, setNewFirstName] = useState(user.firstName); |
There was a problem hiding this comment.
non-blocking. When you have a lot of different useState hooks like this, it is better to use React Hook form as it handles a lot of the state changes, zod resolving, etc for you. The registration form uses them and also something good to note for later.
| ); | ||
| const [hackerTagTakenAlert, setHackerTagTakenAlert] = useState(false); | ||
|
|
||
| const [isLoading, setIsLoading] = useState(false); |
There was a problem hiding this comment.
nit and non-blocking: useAction should come with loading state as well. However, our action is a bit older here so that may not be the case.
| } | ||
|
|
||
| export default function ProfileSettings({ profile }: ProfileSettingsProps) { | ||
| const [newPronouns, setNewPronouns] = useState<string>(profile.pronouns); |
There was a problem hiding this comment.
non-blocking and nit: Same as before. Please use React Hook for in future forms.
apps/web/src/components/settings/RegistrationForm/RegisterFormSettings.tsx
Outdated
Show resolved
Hide resolved
| }); | ||
| const [uploadedFile, setUploadedFile] = useState<File | null>(null); | ||
| const resumeLink: string = data.resume ?? c.noResumeProvidedURL; | ||
| // @ts-ignore |
There was a problem hiding this comment.
question: Why did this you have to use ts-ignore?
There was a problem hiding this comment.
resumeLink thinks it could be undefined, but I thought the line above, the ?? operator will set resumeLink to c.noResumeProvidedURL if data.resume is nullish (including undefined, right?), so therefor, resumeLink will never be undefined, but I couldn't figure out how to make it think that without the build failing.
There was a problem hiding this comment.
Will take a look at this.
| } | ||
|
|
||
| export default function RegisterForm({ data }: RegisterFormSettingsProps) { | ||
| if (data.heardFrom === null) data.heardFrom = undefined; |
There was a problem hiding this comment.
question: What is the purpose of this? null and undefined should be handled the same in nullish or truthy coalescing as well as the database should treat them the same I believe.
There was a problem hiding this comment.
I couldn't figure out how to get around TypeScript handling them differently. I'll do more research on it.
| import { Loader2 } from "lucide-react"; | ||
|
|
||
| export default function RegistrationSettings() { | ||
| const [isLoading, setIsLoading] = useState(false); |
There was a problem hiding this comment.
nit: Still don't think these need loading states
There was a problem hiding this comment.
I've had times where the loading between pages has taken a while, thus to verify their click was registered.
There was a problem hiding this comment.
Have you seen this on the deployment? Or just local.
There was a problem hiding this comment.
Both. On deployment, it's usually first load, but then it's instant. Looking into it rn
| @@ -12,40 +12,192 @@ import { revalidatePath } from "next/cache"; | |||
| // TODO: Add skill updating | |||
| export const modifyRegistrationData = authenticatedAction( | |||
| z.object({ | |||
There was a problem hiding this comment.
nit and non-blocking: As stated before, common types should be in their own types and also use the types of the tables instead of manually adding them.
| PersonalWebsite, | ||
| }, | ||
| { userId }, | ||
| ) => { | ||
| const user = await db.query.users.findFirst({ |
There was a problem hiding this comment.
nit: we created a safe action for this check. Non-blocking for now.
| const user = await db.query.users.findFirst({ | ||
| where: eq(users.clerkID, userId), | ||
| }); | ||
| if (!user) throw new Error("User not found"); | ||
| let oldHackerTag = user.hackerTag; // change when hackertag is not PK on profileData table |
There was a problem hiding this comment.
note: This is fine for now, but you can also have an onConflict statement so the db can handle this for you. With less lookups.
Took out Back button loader to simplify process.
This reverts commit 9b001c4.
Added settings menu, allowing for updating for user data, profile data, and most of registration data.
Note: had to jump around some typescript for assumed types
Will need to adjust for schema changes
Satisfies (#42)