Conversation
…features, add project standards document, and improve form validation and error handling in components.
There was a problem hiding this comment.
Pull request overview
Adds a richer authenticated profile/settings experience (counts, avatar upload, password reset UX) while also refactoring notification handling and upgrading major frontend dependencies.
Changes:
- Implement profile + settings pages (avatar upload, display-name edit, counts charts, reset-password modal).
- Replace
errorNotificationwith a genericnotificationhelper and refactor callers. - Upgrade Mantine/TanStack packages (and add Mantine charts), plus documentation/config updates.
Reviewed changes
Copilot reviewed 38 out of 39 changed files in this pull request and generated 24 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types/settings-user.ts | Adds settings Zod schema + settings-related types. |
| src/types/index.ts | Re-exports new settings-user types. |
| src/rq/query-factories/getQueryOptions.ts | Fixes count query keys/return shapes via select. |
| src/rq/manage-collection.ts | Minor formatting change. |
| src/routes/signup.lazy.tsx | Awaits navigation after signup. |
| src/routes/reset-password.lazy.tsx | Refactors page to use ResetPasswordForm + submitted state. |
| src/routes/_auth/_profile-layout/settings.lazy.tsx | New settings page UI (avatar upload, name edit, reset password modal). |
| src/routes/_auth/_profile-layout/profile.lazy.tsx | New profile page UI with charts and counts. |
| src/hooks/notifications/notification.ts | Introduces generic notification helper. |
| src/hooks/notifications/error-notification.ts | Removes previous error-only notification helper. |
| src/hooks/manage-todos/use-update-todo.ts | Switches to new notification helper on error. |
| src/hooks/manage-todos/use-add-todo.ts | Switches to new notification helper on error. |
| src/hooks/manage-notes/use-update-note.ts | Switches to new notification helper on error. |
| src/hooks/manage-notes/use-add-note.ts | Switches to new notification helper on error. |
| src/hooks/index.ts | Exports display-view context hook. |
| src/hooks/counts/use-counts.tsx | Adds hook to fetch notes/todos counts via useQueries. |
| src/hooks/auth/use-user.tsx | Adds avatar upload + profile update helpers (and local user state). |
| src/hooks/auth/use-auth.tsx | Improves auth error notifications + async reset password. |
| src/database/database.ts | Adds Firebase Storage initialization/export. |
| src/components/templates/reset-password-form.tsx | Extracts reset password form into reusable template. |
| src/components/templates/index.ts | Re-exports new reset-password template. |
| src/components/templates/data-display.tsx | Replaces inline view select with SelectView. |
| src/components/organizms/forms/note-management-form.tsx | Awaits async handleSubmit. |
| src/components/molecules/sticker/todo-sticker.tsx | Adjusts sticker typography sizing. |
| src/components/molecules/sticker/note-sticker.tsx | Adjusts sticker typography sizing. |
| src/components/molecules/nav-bar-user.tsx | Shows user avatar/displayName, updates menu links. |
| src/components/molecules/modals.tsx | Adds overlay props and reset-password modal. |
| src/components/atoms/view-select/select-view.tsx | Adds reusable view selection atom. |
| src/components/atoms/indicator/edit-indicator.tsx | Adds reusable edit indicator atom. |
| src/components/atoms/index.ts | Exports new atoms (but contains a duplicate export). |
| src/Theme/styles.module.css | Adds button variant and tweaks disabled border styling. |
| src/App.tsx | Adds Mantine charts styles + uses notification helper for query errors. |
| package.json | Bumps app version + upgrades Mantine/TanStack deps and adds charts. |
| docs/rebuild-upgrade-plan.md | Adds phased upgrade/refactor plan. |
| cors.json | Adds CORS config (wildcard origin). |
| README.md | Expands project documentation and setup instructions. |
| .vscode/settings.json | Updates workspace editor settings. |
| .cursor/rules/project-standards.mdc | Adds documented engineering rules/conventions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| notification({ message: error.code, type: 'error', title: 'An error occured' }); | ||
| throw new Error(error.code); |
There was a problem hiding this comment.
Typo in notification title: "occured" → "occurred".
| onError: (error, query) => { | ||
| if (typeof query.state.data !== 'undefined') { | ||
| errorNotification({ message: error?.message }); | ||
| notification({ message: error?.message, type: 'error', title: 'An error occured' }); |
There was a problem hiding this comment.
Typo in notification title: "occured" → "occurred".
| notification({ message: error?.message, type: 'error', title: 'An error occured' }); | |
| notification({ message: error?.message, type: 'error', title: 'An error occurred' }); |
| <EditIndicator disabled={true} onClick={() => toggleEdit('email')}> | ||
| <Input | ||
| w={200} | ||
| onChange={e => handleChange(e.target.value)} | ||
| onBlur={handleBlur} | ||
| disabled | ||
| value={state.value} | ||
| /> | ||
| </EditIndicator> |
There was a problem hiding this comment.
EditIndicator is hardcoded disabled={true} here, so the onClick={() => toggleEdit('email')} handler is dead code. Either remove the click handler/indicator wrapper or make email editing actually supported.
| const color = { | ||
| error: `var(--red)`, | ||
| success: 'var(--success)', | ||
| warning: 'var(---warning)', | ||
| info: 'var(--primary)' | ||
| } |
There was a problem hiding this comment.
The color.warning entry uses var(---warning) (triple dash), which is an invalid CSS variable reference and will resolve to nothing if used. Fix it to match the actual variable name (likely var(--warning)).
| // TODO: add error handling to updateUser | ||
| const updateUserData = async (data: any) => { | ||
| if (user) { | ||
| await updateProfile(user, { displayName: data.name }).then(() => { | ||
| notification({ message: 'User data updated successfully', type: 'success', title: 'Success' }); | ||
| }); |
There was a problem hiding this comment.
updateUserData takes data: any, which bypasses type-safety and conflicts with the project standard to not use any for data boundaries. Define a proper input type (e.g., { name: string } or Pick<SettingsValues, 'name'>) and validate/transform before calling updateProfile.
| const { todosCount, notesCount, todosIsPending, notesIsPending } = useCounts(); | ||
| const { data } = useQuery(todosQueries.allTodos()); | ||
| const { user } = useUser(); | ||
| const completedTodos = data?.filter(todo => todo.completed).length || 10; | ||
|
|
There was a problem hiding this comment.
completedTodos defaults to 10 when data is not loaded, which will display incorrect stats (and can exceed todosCount). Default this to 0 and/or derive it only when data is available.
| }, | ||
| onError: (error, variables, rollback) => { | ||
| errorNotification({ message: error?.message }); | ||
| notification({ message: error?.message, type: 'error', title: 'An error occured' }); |
There was a problem hiding this comment.
Typo in notification title: "occured" → "occurred".
| notification({ message: error?.message, type: 'error', title: 'An error occured' }); | |
| notification({ message: error?.message, type: 'error', title: 'An error occurred' }); |
| }, | ||
| onError: (error, variables, rollback) => { | ||
| errorNotification({ message: error?.message }); | ||
| notification({ message: error?.message, type: 'error', title: 'An error occured' }); |
There was a problem hiding this comment.
Typo in notification title: "occured" → "occurred".
| notification({ message: error?.message, type: 'error', title: 'An error occured' }); | |
| notification({ message: error?.message, type: 'error', title: 'An error occurred' }); |
| }, | ||
| onError: (error, variables, rollback) => { | ||
| errorNotification({ message: error?.message }); | ||
| notification({ message: error?.message, type: 'error', title: 'An error occured' }); |
There was a problem hiding this comment.
Typo in notification title: "occured" → "occurred".
| notification({ message: error?.message, type: 'error', title: 'An error occured' }); | |
| notification({ message: error?.message, type: 'error', title: 'An error occurred' }); |
| "react": "^18.3.1", | ||
| "react-dom": "^18.3.1", | ||
| "vite": "^5.4.11", | ||
| "vite": "^5.4.14", | ||
| "vite-plugin-svgr": "^4.3.0", |
There was a problem hiding this comment.
vite is listed in both dependencies and devDependencies (with different version ranges). This can lead to inconsistent installs/resolutions. Keep it in devDependencies only (typical for build tooling) and ensure there's a single version constraint.
No description provided.