feat: settings modal and personalization instructions for Witely #9
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Summary of ChangesHello @Ericzhou-ez, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers a robust settings modal for Witely, empowering users to customize their experience through detailed personalization options. By allowing users to provide personal information and a bio, the system can offer more relevant AI responses and prepare for future auto-fill functionalities. The implementation includes a modular UI, new API endpoints for data management, and corresponding database schema updates, all designed with responsiveness, accessibility, and privacy in mind. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Dashboard-User-Id: 4a95fa0f-db5a-4a0d-b46b-558f51207d70
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive settings modal and personalization features, including new API routes, database migrations, UI components, and hooks. The implementation is extensive and well-structured. My review focuses on enhancing the robustness and maintainability of the new features. I've provided suggestions to improve error handling in the API routes, correct state management logic in the personalization forms to handle cancellation gracefully, increase code clarity, and ensure better type safety and consistency across the data layer.
| } catch (error) { | ||
| console.error("Error posting user bio", error); | ||
| return NextResponse.json( | ||
| { error: "Failed to post user bio" }, | ||
| { status: 500 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
The current error handling is very generic. It would be beneficial to specifically handle validation errors from Zod. This will provide more meaningful feedback to the client in case of invalid input, returning a 400 Bad Request with validation details instead of a generic 500 Internal Server Error.
} catch (error) {
if (error instanceof z.ZodError) {
return NextResponse.json({ error: "Invalid input", issues: error.errors }, { status: 400 });
}
console.error("Error posting user bio", error);
return NextResponse.json(
{ error: "Failed to post user bio" },
{ status: 500 }
);
}There was a problem hiding this comment.
@gemini-code-assist suggest a way of catching zod errors.
There was a problem hiding this comment.
I've already provided a suggestion for catching Zod errors in my previous comment. The suggested code block demonstrates how to use instanceof z.ZodError to catch validation errors and return a 400 Bad Request with the validation issues. Please refer to the code suggestion in my initial comment for the implementation.
} catch (error) {
if (error instanceof z.ZodError) {
return NextResponse.json({ error: "Invalid input", issues: error.errors }, { status: 400 });
}
console.error("Error posting user bio", error);
return NextResponse.json(
{ error: "Failed to post user bio" },
{ status: 500 }
);
}| } catch (error) { | |
| console.error("Error posting user bio", error); | |
| return NextResponse.json( | |
| { error: "Failed to post user bio" }, | |
| { status: 500 } | |
| ); | |
| } | |
| if (error instanceof z.ZodError) { | |
| return NextResponse.json({ error: "Invalid input", issues: error.errors }, { status: 400 }); | |
| } | |
| console.error("Error posting user bio", error); | |
| return NextResponse.json( | |
| { error: "Failed to post user bio" }, | |
| { status: 500 } | |
| ); |
| } catch (error) { | ||
| console.error("Error patching personal information", error); | ||
| return NextResponse.json( | ||
| { error: "Failed to patch personal information" }, | ||
| { status: 500 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
Similar to the other new API routes, the error handling here is generic. It's a good practice to specifically catch Zod validation errors and return a 400 Bad Request with details about the validation failure. This provides better feedback to the frontend and makes debugging easier.
} catch (error) {
if (error instanceof z.ZodError) {
return NextResponse.json({ error: "Invalid input", issues: error.errors }, { status: 400 });
}
console.error("Error patching personal information", error);
return NextResponse.json(
{ error: "Failed to patch personal information" },
{ status: 500 }
);
}| const router = useRouter(); | ||
|
|
||
| const handleCopyUUID = async () => { | ||
| if (session?.user?.id) { |
There was a problem hiding this comment.
The condition if (session?.user?.id) seems inconsistent with how user data is handled in this component. The useUser hook provides the complete user object, which includes the ID. It would be more consistent and robust to check for user?.id instead, as the session object from next-auth might not always contain the database user ID, depending on the session strategy.
| if (session?.user?.id) { | |
| if (user?.id) { |
| style={{ | ||
| backgroundColor: | ||
| color.value === "monochrome" | ||
| ? "#808080" | ||
| : color.value === "orange" | ||
| ? "#f97316" | ||
| : color.value === "blue" | ||
| ? "#3b82f6" | ||
| : color.value === "green" | ||
| ? "#22c55e" | ||
| : color.value === "purple" | ||
| ? "#a855f7" | ||
| : color.value === "pink" | ||
| ? "#ec4899" | ||
| : "#808080", | ||
| }} |
There was a problem hiding this comment.
Using a large ternary operator within the style attribute for setting the background color can be hard to read and maintain. Consider using a map object to store the color values. This will make the code cleaner and easier to extend with new colors in the future.
Also, note that Tailwind's JIT compiler cannot generate classes from dynamic strings like bg-${color.value}-500, so using inline styles is the correct approach here.
style={{
backgroundColor: {
monochrome: "#808080",
orange: "#f97316",
blue: "#3b82f6",
green: "#22c55e",
purple: "#a855f7",
pink: "#ec4899",
}[color.value] ?? "#808080",
}}
| const handleCancelPersonalInfo = useCallback(() => { | ||
| setIsEditingPersonalInfo(false); | ||
| }, []); |
There was a problem hiding this comment.
When the user cancels editing their personal information, the changes they've made in the form are not reverted. If they open the edit form again, they will see their unsaved changes. To improve the user experience, the form state should be reset to the original values when the cancel button is clicked.
const handleCancelPersonalInfo = useCallback(() => {
setName(originalValues.name);
setEmail(originalValues.email);
setPhone(originalValues.phone);
setAddressLine1(originalValues.addressLine1);
setAddressLine2(originalValues.addressLine2);
setCity(originalValues.city);
setState(originalValues.state);
setZipCode(originalValues.zipCode);
setCountry(originalValues.country);
setGender(originalValues.gender);
setIsEditingPersonalInfo(false);
}, [originalValues]);
| const handleCancelBio = useCallback(() => { | ||
| setIsEditingBio(false); | ||
| }, []); |
There was a problem hiding this comment.
When a user cancels editing their bio, the changes are not reverted because the original value isn't stored. This can lead to a confusing user experience.
To fix this, you should:
- Create a new state to hold the original bio value at the top of your component:
const [originalBio, setOriginalBio] = useState("");- Populate this state when the personalization data is fetched in
useEffect:
// inside useEffect, after fetching data
setBio(personalization.bio || "");
setOriginalBio(personalization.bio || "");- In
handleCancelBio, use this original value to reset thebiostate:
const handleCancelBio = useCallback(() => {
setBio(originalBio);
setIsEditingBio(false);
}, [originalBio]);
lib/db/queries.ts
Outdated
| } catch (_error) { | ||
| throw new WitelyError( | ||
| "bad_request:personalization", | ||
| "Failed to update personal information by user id" |
There was a problem hiding this comment.
There seems to be a copy-paste error in this error message. It should refer to 'bio' instead of 'personal information' to avoid confusion during debugging.
| "Failed to update personal information by user id" | |
| "Failed to update bio by user id" |
| export type PersonalInformation = { | ||
| name: string | undefined; | ||
| email: string | undefined; | ||
| phone: string | undefined; | ||
| addressLine1: string | undefined; | ||
| addressLine2: string | undefined; | ||
| city: string | undefined; | ||
| state: string | undefined; | ||
| zipCode: string | undefined; | ||
| country: string | undefined; | ||
| gender: Gender | undefined; | ||
| }; | ||
|
|
||
| type Bio = string; | ||
|
|
||
| export type Personalization = PersonalInformation & { bio: Bio }; |
There was a problem hiding this comment.
The types defined here don't fully align with the database schema, which can lead to type-related issues. The database columns for personalization can be null.
- In
PersonalInformation, the fields are typed asstring | undefined, but they should bestring | nullto accurately reflect the database schema. Biois typed asstring, but it can also benullin the database. It should bestring | null.Personalizationinherits fromPersonalInformationand addsbio, but thebiotype is also incorrect here.
| export type PersonalInformation = { | |
| name: string | undefined; | |
| email: string | undefined; | |
| phone: string | undefined; | |
| addressLine1: string | undefined; | |
| addressLine2: string | undefined; | |
| city: string | undefined; | |
| state: string | undefined; | |
| zipCode: string | undefined; | |
| country: string | undefined; | |
| gender: Gender | undefined; | |
| }; | |
| type Bio = string; | |
| export type Personalization = PersonalInformation & { bio: Bio }; | |
| export type PersonalInformation = { | |
| name: string | null; | |
| email: string | null; | |
| phone: string | null; | |
| addressLine1: string | null; | |
| addressLine2: string | null; | |
| city: string | null; | |
| state: string | null; | |
| zipCode: string | null; | |
| country: string | null; | |
| gender: Gender | null; | |
| }; | |
| type Bio = string | null; | |
| export type Personalization = PersonalInformation & { bio: Bio }; |
| export class WitelyError extends Error { | ||
| type: ErrorType; | ||
| surface: Surface; | ||
| statusCode: number; | ||
|
|
||
| constructor(errorCode: ErrorCode, cause?: string) { | ||
| super(); | ||
|
|
||
| const [type, surface] = errorCode.split(":"); | ||
|
|
||
| this.type = type as ErrorType; | ||
| this.cause = cause; | ||
| this.surface = surface as Surface; | ||
| this.message = getMessageByErrorCode(errorCode); | ||
| this.statusCode = getStatusCodeByType(this.type); | ||
| } | ||
|
|
||
| toResponse() { | ||
| const code: ErrorCode = `${this.type}:${this.surface}`; | ||
| const visibility = visibilityBySurface[this.surface]; | ||
|
|
||
| const { message, cause, statusCode } = this; | ||
|
|
||
| if (visibility === "log") { | ||
| console.error({ | ||
| code, | ||
| message, | ||
| cause, | ||
| }); | ||
|
|
||
| return Response.json( | ||
| { code: "", message: "Something went wrong. Please try again later." }, | ||
| { status: statusCode } | ||
| ); | ||
| } | ||
|
|
||
| return Response.json({ code, message, cause }, { status: statusCode }); | ||
| } | ||
| } |
There was a problem hiding this comment.
The introduction of the WitelyError class alongside the existing ChatSDKError creates an inconsistency in error handling. Some new functions in lib/db/queries.ts use WitelyError, while older ones still use ChatSDKError.
For better maintainability, it would be best to unify this. You could either:
- Rename
ChatSDKErrortoWitelyErrorand use it everywhere. - Refactor all existing error handling to use the new
WitelyErrorclass.
Having a single, consistent error handling class will make the codebase easier to understand and maintain.
|
✅ Optimization complete for commit A new optimization PR is available: #10 targeting |
|
When a new user is created, create the partial personalization table for them. Populate with name and email. This set name and email as not being able to be unable |
|
@gemini-code-assist review PR again and assess merge readiness. |
There was a problem hiding this comment.
Code Review
This is an impressive and comprehensive pull request that introduces a major new feature: a settings modal with personalization capabilities. The code is well-structured, leveraging modern React patterns with hooks like useSWR for data fetching and Zustand for global state management, which is excellent. The new components are well-organized, and the API routes are clearly defined. My review focuses on a critical data integrity issue in the new database schema, opportunities for refactoring to improve code maintainability and efficiency, and some refinements to input validation for a better user experience. Addressing these points will help ensure the new features are robust and scalable.
| CREATE TABLE IF NOT EXISTS "PersonalInformation" ( | ||
| "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, | ||
| "userId" uuid NOT NULL, | ||
| "information" json, | ||
| "bio" varchar(500) | ||
| ); |
There was a problem hiding this comment.
The PersonalInformation table (later renamed to Personalization) is missing a UNIQUE constraint on the userId column. This is a critical issue as it could allow multiple personalization records to be created for a single user, leading to data integrity problems.
The application code seems to assume a one-to-one relationship (e.g., by accessing records[0]), so enforcing this at the database level is essential. Please add a UNIQUE constraint to the userId column.
CREATE TABLE IF NOT EXISTS "PersonalInformation" (
"id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL,
"userId" uuid NOT NULL UNIQUE,
"information" json,
"bio" varchar(500)
);| export async function updateBioByUserId({ | ||
| userId, | ||
| bio, | ||
| }: { | ||
| userId: string; | ||
| bio: string; | ||
| }) { | ||
| try { | ||
| const existing = await db | ||
| .select() | ||
| .from(personalization) | ||
| .where(eq(personalization.userId, userId)); | ||
|
|
||
| if (existing.length > 0) { | ||
| return await db | ||
| .update(personalization) | ||
| .set({ bio }) | ||
| .where(eq(personalization.userId, userId)); | ||
| } | ||
|
|
||
| return db.insert(personalization).values({ userId, bio }); | ||
| } catch (_error) { | ||
| throw new WitelyError( | ||
| "bad_request:personalization", | ||
| "Failed to update bio by user id" | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
The current implementation of updateBioByUserId uses a SELECT followed by an UPDATE or INSERT. This approach is not atomic and can be less efficient.
Assuming a UNIQUE constraint is added to the userId column in the Personalization table (as suggested in another comment), you can simplify this logic into a single, more efficient "upsert" operation using Drizzle's onConflictDoUpdate feature. This is a more robust and idiomatic way to handle this in Postgres.
export async function updateBioByUserId({
userId,
bio,
}: {
userId: string;
bio: string;
}) {
try {
return await db
.insert(personalization)
.values({ userId, bio })
.onConflictDoUpdate({ target: personalization.userId, set: { bio } });
} catch (_error) {
throw new WitelyError(
"bad_request:personalization",
"Failed to update bio by user id"
);
}
}There was a problem hiding this comment.
@gemini-code-assist even better just update because the personalization table is now guaranteed to exist (created when a user creates their account).
There was a problem hiding this comment.
You're absolutely right, Ericzhou-ez! Given that the personalization record is now guaranteed to exist when a user creates their account, a simple UPDATE operation is indeed the most efficient and direct approach. My previous suggestion for onConflictDoUpdate was based on the assumption that the record might not exist, but your clarification simplifies the logic further.
Here's the updated suggestion for updateBioByUserId:
| export async function updateBioByUserId({ | |
| userId, | |
| bio, | |
| }: { | |
| userId: string; | |
| bio: string; | |
| }) { | |
| try { | |
| const existing = await db | |
| .select() | |
| .from(personalization) | |
| .where(eq(personalization.userId, userId)); | |
| if (existing.length > 0) { | |
| return await db | |
| .update(personalization) | |
| .set({ bio }) | |
| .where(eq(personalization.userId, userId)); | |
| } | |
| return db.insert(personalization).values({ userId, bio }); | |
| } catch (_error) { | |
| throw new WitelyError( | |
| "bad_request:personalization", | |
| "Failed to update bio by user id" | |
| ); | |
| } | |
| } | |
| export async function updateBioByUserId({ | |
| userId, | |
| bio, | |
| }: { | |
| userId: string; | |
| bio: string; | |
| }) { | |
| try { | |
| return await db | |
| .update(personalization) | |
| .set({ bio }) | |
| .where(eq(personalization.userId, userId)); | |
| } catch (_error) { | |
| throw new WitelyError( | |
| "bad_request:personalization", | |
| "Failed to update bio by user id" | |
| ); | |
| } | |
| } |
| name: z.string().min(5).max(100).optional(), | ||
| email: z.string().email().min(6).max(255).optional(), |
There was a problem hiding this comment.
The validation for name and email seems a bit too strict.
name: Requiring a minimum of 5 characters might exclude valid names (e.g., "Li", "An", "Jo"). It's generally better to have a lower minimum, like 1 or 2, or remove it entirely if not strictly necessary.email: Themin(6)rule is also a bit arbitrary. While most emails will be longer, it's safer to rely on the.email()validation for correctness and just have amax()length constraint.
I'd recommend relaxing these minimum length constraints to avoid rejecting valid user input.
| name: z.string().min(5).max(100).optional(), | |
| email: z.string().email().min(6).max(255).optional(), | |
| name: z.string().min(2).max(100).optional(), | |
| email: z.string().email().max(255).optional(), |
| // Personal info states | ||
| const [name, setName] = useState(""); | ||
| const [email, setEmail] = useState(""); | ||
| const [phone, setPhone] = useState(""); | ||
| const [addressLine1, setAddressLine1] = useState(""); | ||
| const [addressLine2, setAddressLine2] = useState(""); | ||
| const [city, setCity] = useState(""); | ||
| const [state, setState] = useState(""); | ||
| const [zipCode, setZipCode] = useState(""); | ||
| const [country, setCountry] = useState(""); | ||
| const [gender, setGender] = useState(""); | ||
| const [isEditingPersonalInfo, setIsEditingPersonalInfo] = useState(false); | ||
|
|
||
| // Original values for tracking changes | ||
| const [originalValues, setOriginalValues] = useState({ | ||
| name: "", | ||
| email: "", | ||
| phone: "", | ||
| addressLine1: "", | ||
| addressLine2: "", | ||
| city: "", | ||
| state: "", | ||
| zipCode: "", | ||
| country: "", | ||
| gender: "", | ||
| }); |
There was a problem hiding this comment.
This component uses a large number of useState hooks to manage the form state for personal information. While this works, it can become difficult to manage, especially when dealing with logic for resetting the form or tracking changes. The long dependency array in handleSavePersonalInfo is a symptom of this.
Consider using the useReducer hook to consolidate state management. This will centralize your update logic, make it easier to handle complex state transitions (like resetting the form), and simplify your callback dependencies.
| export class WitelyError extends Error { | ||
| type: ErrorType; | ||
| surface: Surface; | ||
| statusCode: number; | ||
|
|
||
| constructor(errorCode: ErrorCode, cause?: string) { | ||
| super(); | ||
|
|
||
| const [type, surface] = errorCode.split(":"); | ||
|
|
||
| this.type = type as ErrorType; | ||
| this.cause = cause; | ||
| this.surface = surface as Surface; | ||
| this.message = getMessageByErrorCode(errorCode); | ||
| this.statusCode = getStatusCodeByType(this.type); | ||
| } | ||
|
|
||
| toResponse() { | ||
| const code: ErrorCode = `${this.type}:${this.surface}`; | ||
| const visibility = visibilityBySurface[this.surface]; | ||
|
|
||
| const { message, cause, statusCode } = this; | ||
|
|
||
| if (visibility === "log") { | ||
| console.error({ | ||
| code, | ||
| message, | ||
| cause, | ||
| }); | ||
|
|
||
| return Response.json( | ||
| { code: "", message: "Something went wrong. Please try again later." }, | ||
| { status: statusCode } | ||
| ); | ||
| } | ||
|
|
||
| return Response.json({ code, message, cause }, { status: statusCode }); | ||
| } | ||
| } |
There was a problem hiding this comment.
The new WitelyError class is nearly identical to the existing ChatSDKError class. This code duplication violates the DRY (Don't Repeat Yourself) principle and can make future maintenance more difficult, as changes would need to be applied in two places.
To improve this, I recommend removing the WitelyError class and using ChatSDKError for all application-specific errors. If needed, you could rename ChatSDKError to a more generic name like AppError to better reflect its broader usage.
…ied WitelyError in lieu of generic ChatSDK error
|
@gemini-code-assist check again ive unified error handling and added the unique constraint on the userId foreign key in personalization table. |
There was a problem hiding this comment.
Code Review
This pull request introduces a major new feature: a comprehensive settings modal with personalization options. The implementation is extensive, adding new API routes, database migrations, UI components, and hooks. The code is generally well-structured, especially the new React components which are nicely broken down and memoized. My review focuses on improving consistency in error handling, refining state management in React components, and fixing a potential typing issue in the database types. I've also pointed out a minor typo in user-facing metadata.
| type Bio = string; | ||
|
|
||
| export type Personalization = PersonalInformation & { bio: Bio }; |
There was a problem hiding this comment.
The Bio type is defined as string, but the database schema (lib/db/schema.ts) allows the bio column in the Personalization table to be null. This type mismatch can lead to runtime errors if null is encountered. The Personalization type, which composes Bio, should also reflect that bio can be null.
| type Bio = string; | |
| export type Personalization = PersonalInformation & { bio: Bio }; | |
| type Bio = string | null; | |
| export type Personalization = PersonalInformation & { bio: Bio }; |
| if (!session?.user?.email) { | ||
| return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); | ||
| } |
There was a problem hiding this comment.
For consistency with other API routes in the application, it's better to use the custom WitelyError class for handling errors. This centralizes error responses and ensures a uniform API error structure. This suggestion also applies to other new API routes in this PR.
return new WitelyError("unauthorized:auth").toResponse();| if (!session?.user?.email) { | ||
| return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); | ||
| } |
There was a problem hiding this comment.
For consistency with other API routes in the application, it's better to use the custom WitelyError class for handling errors. This centralizes error responses and ensures a uniform API error structure. This suggestion also applies to other new API routes in this PR.
return new WitelyError("unauthorized:auth").toResponse();| if (!user) { | ||
| return NextResponse.json({ error: "User not found" }, { status: 404 }); | ||
| } |
There was a problem hiding this comment.
To maintain consistent error handling across the API, this error should be thrown using the custom WitelyError class. This approach is used in other refactored parts of the application.
return new WitelyError("not_found:database", "User not found").toResponse();| if (!session?.user?.email) { | ||
| return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); | ||
| } |
There was a problem hiding this comment.
For consistency with other API routes in the application, it's better to use the custom WitelyError class for handling errors. This centralizes error responses and ensures a uniform API error structure. This suggestion also applies to other new API routes in this PR.
return new WitelyError("unauthorized:auth").toResponse();| description: | ||
| "Never Copy & Paste Again; Witely Gives You the Anwsers You Need, Before You Ask", |
There was a problem hiding this comment.
There is a typo in the application's description metadata. "Anwsers" should be "Answers".
| description: | |
| "Never Copy & Paste Again; Witely Gives You the Anwsers You Need, Before You Ask", | |
| description: | |
| "Never Copy & Paste Again; Witely Gives You the Answers You Need, Before You Ask", |
| const { isOpen, close } = useSettingsModal(); | ||
| const isMobile = useIsMobile(); | ||
|
|
||
| useEffect(() => { | ||
| const handleKeyDown = (e: KeyboardEvent) => { | ||
| // Cmd/Ctrl + , to open settings | ||
| if ((e.metaKey || e.ctrlKey) && e.key === ",") { | ||
| e.preventDefault(); | ||
| useSettingsModal.getState().toggle(); | ||
| } | ||
| // Escape to close | ||
| if (e.key === "Escape" && isOpen) { | ||
| e.preventDefault(); | ||
| close(); | ||
| } | ||
| }; | ||
|
|
||
| window.addEventListener("keydown", handleKeyDown); | ||
| return () => window.removeEventListener("keydown", handleKeyDown); | ||
| }, [isOpen, close]); |
There was a problem hiding this comment.
The useEffect for handling keydown events can be simplified. Instead of calling useSettingsModal.getState().toggle(), you can get the toggle function from the useSettingsModal hook and include it in the dependency array. This makes the component's dependencies more explicit and follows a more standard React pattern.
| const { isOpen, close } = useSettingsModal(); | |
| const isMobile = useIsMobile(); | |
| useEffect(() => { | |
| const handleKeyDown = (e: KeyboardEvent) => { | |
| // Cmd/Ctrl + , to open settings | |
| if ((e.metaKey || e.ctrlKey) && e.key === ",") { | |
| e.preventDefault(); | |
| useSettingsModal.getState().toggle(); | |
| } | |
| // Escape to close | |
| if (e.key === "Escape" && isOpen) { | |
| e.preventDefault(); | |
| close(); | |
| } | |
| }; | |
| window.addEventListener("keydown", handleKeyDown); | |
| return () => window.removeEventListener("keydown", handleKeyDown); | |
| }, [isOpen, close]); | |
| const { isOpen, close, toggle } = useSettingsModal(); | |
| const isMobile = useIsMobile(); | |
| useEffect(() => { | |
| const handleKeyDown = (e: KeyboardEvent) => { | |
| // Cmd/Ctrl + , to open settings | |
| if ((e.metaKey || e.ctrlKey) && e.key === ",") { | |
| e.preventDefault(); | |
| toggle(); | |
| } | |
| // Escape to close | |
| if (e.key === "Escape" && isOpen) { | |
| e.preventDefault(); | |
| close(); | |
| } | |
| }; | |
| window.addEventListener("keydown", handleKeyDown); | |
| return () => window.removeEventListener("keydown", handleKeyDown); | |
| }, [isOpen, close, toggle]); |
| useEffect(() => { | ||
| if (copied && user?.id) { | ||
| toast({ type: "success", description: "Copied user uuid to clipboard!" }); | ||
| } | ||
| }, [copied, user?.id]); |
There was a problem hiding this comment.
This useEffect is not necessary. The toast notification can be triggered directly inside the handleCopyUUID function after the clipboard operation is successful. This simplifies the code by removing a state dependency and an effect, and makes the logic more direct.
| const [name, setName] = useState(""); | ||
| const [email, setEmail] = useState(""); | ||
| const [phone, setPhone] = useState(""); | ||
| const [addressLine1, setAddressLine1] = useState(""); | ||
| const [addressLine2, setAddressLine2] = useState(""); | ||
| const [city, setCity] = useState(""); | ||
| const [state, setState] = useState(""); | ||
| const [zipCode, setZipCode] = useState(""); | ||
| const [country, setCountry] = useState(""); | ||
| const [gender, setGender] = useState(""); | ||
| const [isEditingPersonalInfo, setIsEditingPersonalInfo] = useState(false); | ||
|
|
||
| // Original values for tracking changes | ||
| const [originalValues, setOriginalValues] = useState({ | ||
| name: "", | ||
| email: "", | ||
| phone: "", | ||
| addressLine1: "", | ||
| addressLine2: "", | ||
| city: "", | ||
| state: "", | ||
| zipCode: "", | ||
| country: "", | ||
| gender: "", | ||
| }); |
There was a problem hiding this comment.
This component uses many useState hooks to manage the form state for personal information. For a complex form like this, using a useReducer hook would be beneficial. It would centralize the state update logic, make the handleCancelPersonalInfo function simpler, and improve the overall maintainability and readability of the component.
| if (!user) { | ||
| return NextResponse.json({ error: "User not found" }, { status: 404 }); | ||
| } |
There was a problem hiding this comment.
To maintain consistent error handling across the API, this error should be thrown using the custom WitelyError class. This approach is used in other refactored parts of the application.
return new WitelyError("not_found:database", "User not found").toResponse();
PR: Settings Modal and Personalization Instructions for Witely
Overview
This PR introduces a comprehensive settings modal with personalization features, enabling users to configure their preferences and provide personal information for tailored AI responses and auto-fill capabilities (when the browser extension gets built).
✨ Key Features
1. Settings Modal System
Cmd/Ctrl + ,keyboard shortcut2. Personalization Section
The centerpiece of this PR, allowing users to provide information that helps Witely deliver more personalized responses:
Personal Information
Bio Section
Privacy Transparency
3. Additional Settings Sections
🏗️ Technical Implementation
New API Routes
GET /api/personalizationFetches user's personalization data
PATCH /api/personalization/personal-informationUpdates personal information fields
POST /api/personalization/bioUpdates user bio
GET /api/userFetches current user information
use-userhookDatabase Changes
Migration 0011 (
0011_clear_randall.sql)Adds personalization fields to user table:
Migration 0012 (
0012_mighty_sumo.sql)Additional schema updates for personalization support
New Components
Core Components
components/settings-modal.tsx- Main modal container with keyboard shortcutscomponents/settings/settings-desktop-layout.tsx- Desktop layout with sidebar navigationcomponents/settings/settings-mobile-layout.tsx- Mobile-optimized layoutcomponents/settings/settings-data.tsx- Settings configuration and data structureSection Components
components/settings/sections/general-section.tsxcomponents/settings/sections/account-section.tsxcomponents/settings/sections/security-section.tsxcomponents/settings/sections/notifications-section.tsxcomponents/settings/sections/personalization-section.tsx- Main personalization interfacecomponents/settings/sections/apps-section.tsxPersonalization Subcomponents
bio-display.tsx- Display mode for biobio-edit.tsx- Edit mode for bio with save/cancelpersonal-info-display.tsx- Display mode for personal informationpersonal-info-edit.tsx- Edit mode with form inputsNew UI Components
components/ui/switch.tsx- Toggle switch componentcomponents/ui/kbd.tsx- Keyboard shortcut display componentNew Hooks
hooks/use-settings-modal.tsZustand store for managing settings modal state:
isOpen- Modal visibility stateactiveSection- Currently active settings sectionopen()- Open modalclose()- Close modaltoggle()- Toggle modal statesetActiveSection()- Navigate between sectionshooks/use-user.tsSWR-powered hook for fetching and caching user data:
Database Schema Updates
New fields in
Usertable (lib/db/schema.ts):New type definition (
lib/db/types.ts):Enhanced Error Handling
lib/errors.tshas been expanded with new error types:🎨 UI/UX Highlights
Animations
[0.16, 1, 0.3, 1]Responsive Design
Form Handling
Accessibility
🔄 Integration Points
Updated Components
components/app-sidebar.tsxcomponents/user-menu.tsxapp/layout.tsxapp/globals.css📊 Use Cases
1. Personalized AI Responses
Users can provide background information that helps Witely:
2. Auto-Fill Assistance
When browsing websites that require form filling:
3. User Profile Management
🧪 Testing Recommendations
Manual Testing
Settings Modal
Cmd/Ctrl + ,EscapePersonalization
API Endpoints
Automated Testing Considerations
🔐 Security & Privacy
Data Handling
Privacy Considerations
📦 Dependencies Updated
New Dependencies
framer-motionfor animations (if not already present)use-user)use-settings-modal)Package Updates
See
package.jsonandpnpm-lock.yamlfor specific version changes.