chore: add avator icon - #56
Conversation
chore: add avator icon geenrate system
There was a problem hiding this comment.
Pull request overview
This PR adds user avatar/identicon functionality to the application header using the @teapotlabs/identeapots library to generate unique visual identifiers for users based on their user ID.
Changes:
- Added @teapotlabs/identeapots dependency and configured VITE_IDENTEAPOT_SALT environment variable across all environments
- Implemented identicon generation in the Share route Header component with async generation on session check
- Updated API types to include user_avatar_url field in session response
- Added CSS styling for circular 40px identicon display with responsive layout adjustments
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| setup/src/types.ts | Added viteIdenteapotSalt to FrontendConfig type |
| setup/src/index.ts | Added VITE_IDENTEAPOT_SALT to frontend environment file generation |
| setup/src/functions.ts | Added environment variable loading and masking for identeapot salt |
| products/frontend/src/routes/Share/components/Header/index.tsx | Implemented identicon generation on session check with state management |
| products/frontend/src/routes/Share/components/Header/index.module.css | Added identicon styling and vertical centering for nav items |
| products/frontend/src/routes/Root/components/Lent/index.module.css | Added width: fit-content for better layout |
| products/frontend/src/routes/Root/components/Borrow/index.module.css | Added width: fit-content for better layout |
| products/frontend/src/api/openapi.d.ts | Added user_avatar_url field to session API response |
| products/frontend/package.json | Added @teapotlabs/identeapots dependency |
| products/frontend/.env.example | Added VITE_IDENTEAPOT_SALT environment variable |
| pnpm-workspace.yaml | Added @teapotlabs/identeapots to catalog |
| pnpm-lock.yaml | Locked @teapotlabs/identeapots version |
| package.json | Added @teapotlabs/identeapots to root dependencies |
| AI.md | Removed obsolete session check code example |
| .github/workflows/preview-frontend.yaml | Added VITE_IDENTEAPOT_SALT secret to preview workflow |
| .github/workflows/deploy-frontend.yaml | Added VITE_IDENTEAPOT_SALT secret to deploy workflow |
| .env.example | Added VITE_IDENTEAPOT_SALT and VITE_CLIENT_URL to root env example |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .identicon { | ||
| width: 40px; | ||
| height: 40px; | ||
| border-radius: 50%; | ||
| } |
There was a problem hiding this comment.
The identicon will be hidden on mobile screens (max-width: 499px) because the entire nav is set to display: none at line 30. This may be intentional, but consider whether the avatar should remain visible on mobile even if other navigation links are hidden, as it's an important user identity indicator.
| VITE_CLIENT_URL= | ||
| VITE_REDIRECT_URL= | ||
| VITE_IDENTEAPOT_SALT= |
There was a problem hiding this comment.
The VITE_CLIENT_URL entry appears on line 3 but was likely already present in the file. However, the ordering in .env.example is inconsistent with products/frontend/.env.example where VITE_CLIENT_URL comes before VITE_REDIRECT_URL. While this doesn't affect functionality, consistency across environment example files improves maintainability.
| const salt = import.meta.env.VITE_IDENTEAPOT_SALT as string; | ||
| setIdenticon(await generateIdenteapot(data.user_id, salt)); |
There was a problem hiding this comment.
The salt value is exposed to the client-side code through environment variables. If the salt is intended to be a secret for security purposes, this approach is insecure as any user can inspect the client-side code and extract the salt. Consider whether this salt needs to be secret, and if so, generate identicons server-side instead.
| <nav className={styles.nav}> | ||
| <NavLink to="/">トップ</NavLink> | ||
| <NavLink to="/gen-group">グループ作成</NavLink> | ||
| <img className={styles.identicon} src={identicon} alt="User Identicon" /> |
There was a problem hiding this comment.
The img element will render with an empty src attribute during the initial render (before the session check completes), which could cause a broken image icon to flash briefly or trigger an unnecessary network request. Consider conditionally rendering the img element only when identicon is not empty, or provide a placeholder/loading state.
| <img className={styles.identicon} src={identicon} alt="User Identicon" /> | |
| {identicon && ( | |
| <img className={styles.identicon} src={identicon} alt="User Identicon" /> | |
| )} |
| const salt = import.meta.env.VITE_IDENTEAPOT_SALT as string; | ||
| setIdenticon(await generateIdenteapot(data.user_id, salt)); |
There was a problem hiding this comment.
The API response now includes user_avatar_url, but the code doesn't use it at all. Instead, it always generates an identicon from the user_id. Consider using the user_avatar_url when available and falling back to the generated identicon only when it's null. This would provide a better user experience by respecting user-uploaded avatars if they exist.
| const salt = import.meta.env.VITE_IDENTEAPOT_SALT as string; | |
| setIdenticon(await generateIdenteapot(data.user_id, salt)); | |
| const userAvatarUrl = (data as any).user_avatar_url; | |
| if (userAvatarUrl) { | |
| setIdenticon(userAvatarUrl); | |
| } else { | |
| const salt = import.meta.env.VITE_IDENTEAPOT_SALT as string; | |
| setIdenticon(await generateIdenteapot(data.user_id, salt)); | |
| } |
| const navigate = useNavigate(); | ||
| const sessionCheckMutation = $api.useMutation('get', '/api/session', { | ||
| onSuccess: async (data) => { | ||
| const salt = import.meta.env.VITE_IDENTEAPOT_SALT as string; |
There was a problem hiding this comment.
Missing validation or default value for VITE_IDENTEAPOT_SALT. If this environment variable is not set, the code will pass 'undefined' (as a string after type assertion) to generateIdenteapot, which could cause unexpected behavior. Consider adding validation to ensure the salt is provided, or document that it's optional.
| const salt = import.meta.env.VITE_IDENTEAPOT_SALT as string; | |
| const salt = import.meta.env.VITE_IDENTEAPOT_SALT; | |
| if (!salt) { | |
| console.warn('VITE_IDENTEAPOT_SALT is not set; skipping identicon generation.'); | |
| return; | |
| } |
| <nav className={styles.nav}> | ||
| <NavLink to="/">トップ</NavLink> | ||
| <NavLink to="/gen-group">グループ作成</NavLink> | ||
| <img className={styles.identicon} src={identicon} alt="User Identicon" /> |
There was a problem hiding this comment.
The alt text "User Identicon" is generic and not user-specific. Consider making it more descriptive or include the username to improve accessibility for screen reader users, such as "Avatar for [username]" or similar.
| const sessionCheckMutation = $api.useMutation('get', '/api/session', { | ||
| onSuccess: async (data) => { | ||
| const salt = import.meta.env.VITE_IDENTEAPOT_SALT as string; | ||
| setIdenticon(await generateIdenteapot(data.user_id, salt)); |
There was a problem hiding this comment.
Missing error handling for the async generateIdenteapot function. If the generation fails, the error will be silently swallowed. Consider adding error handling to manage potential failures gracefully.
| setIdenticon(await generateIdenteapot(data.user_id, salt)); | |
| try { | |
| const icon = await generateIdenteapot(data.user_id, salt); | |
| setIdenticon(icon); | |
| } catch (error) { | |
| // Handle errors from generateIdenteapot to avoid unhandled promise rejections | |
| // and silently swallowed failures. | |
| console.error('Failed to generate identicon:', error); | |
| } |
|
|
||
| useEffect(() => { | ||
| sessionCheckMutation.mutate({ credentials: 'include' }); | ||
| }, []); |
There was a problem hiding this comment.
The useEffect has an empty dependency array but uses sessionCheckMutation, which should be included in the dependencies or the mutation call should be wrapped in a ref or callback to avoid stale closure issues. Additionally, this will cause the session check to run on every component mount, which could be inefficient if the component remounts frequently.
| }, []); | |
| }, [sessionCheckMutation]); |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dbba9d0c57
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| /** Format: uri */ | ||
| user_avatar_url: string | null; |
There was a problem hiding this comment.
Align session response type with backend payload
The OpenAPI type now claims /api/session returns user_avatar_url, but the backend handler in products/backend/src/presentation/routes/check.ts still only returns user_id and user_name. Any frontend code that starts using user_avatar_url based on this type will see undefined at runtime (not string | null) and can mis-handle avatar rendering. Either add the field to the backend response/schema or remove it here to keep the contract accurate.
Useful? React with 👍 / 👎.
No description provided.