cssの修正完了 - #44
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 874ee18be4
ℹ️ 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".
| {props.inviteUrl ? ( | ||
| <button | ||
| className={styles.button} | ||
| type="button" | ||
| onClick={() => props.inviteUrl && inviteUrlHandler(props.inviteUrl)} |
There was a problem hiding this comment.
When the Clipboard API is unavailable or permission is denied (e.g., non-HTTPS context, Safari limitations, or user rejection), this component only renders a copy button and never displays the actual invite URL, so users have no way to obtain the link. The previous UI exposed the URL in a read-only input, which served as a manual fallback. Consider rendering the URL (or showing it on copy failure) so the invite link remains accessible in those environments.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This pull request implements comprehensive CSS and UI improvements for a money-sharing/group expense management application. The changes focus on modernizing the user interface, adding toast notifications for better user feedback, and refactoring components for better maintainability.
Changes:
- Added react-hot-toast library for toast notifications throughout the application
- Refactored components into smaller, more focused modules (Root, GroupDetail, Login pages)
- Created new shared components: Loading, Error, FullPaymentButton, InviteButton
- Replaced generic Button component with specialized button components
- Updated CSS styling with new animations, responsive design, and better layout
Reviewed changes
Copilot reviewed 59 out of 61 changed files in this pull request and generated 16 comments.
Show a summary per file
| File | Description |
|---|---|
| products/frontend/src/share/index.ts | Updated exports: removed Button, added Loading, Error, FullPaymentButton, InviteButton |
| products/frontend/src/share/Loading/* | New animated loading component with wave animation |
| products/frontend/src/share/Error/* | New error display component with animated boxes |
| products/frontend/src/share/InviteButton/* | New component for copying invite URLs with toast feedback |
| products/frontend/src/share/FullPaymentButton/* | New circular button for marking debts as paid |
| products/frontend/src/share/FormButton/* | Renamed from Button, updated margin styling |
| products/frontend/src/routes/Share/index.tsx | Added Toaster configuration for global toast notifications |
| products/frontend/src/routes/Share/components/Header/* | Simplified header layout, removed user display and nav container |
| products/frontend/src/routes/SessionCheck/index.tsx | Replaced text with Loading/Error components |
| products/frontend/src/routes/Root/* | Major refactoring: split into sub-components (Logo, Description, Menu, Group, Borrow, Lent) |
| products/frontend/src/routes/Profile/* | Added form styling and toast notifications |
| products/frontend/src/routes/NotFound/* | Improved 404 page styling |
| products/frontend/src/routes/Login/* | Extracted LoginButton component, improved layout |
| products/frontend/src/routes/Invite/* | Added toast notifications and improved success message display |
| products/frontend/src/routes/GroupDetail/* | Split into sub-components, added toast notifications, improved form styling |
| products/frontend/src/routes/GenerateGroup/* | Added toast notifications, improved form styling |
| products/frontend/src/lib/date.ts | New utility function for date formatting |
| products/frontend/src/index.css | Added color variables for error and disabled states |
| package.json, pnpm-workspace.yaml, pnpm-lock.yaml | Added react-hot-toast dependency |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <div className={styles.memberWrapper}> | ||
| <small className={styles.label}>[メンバー]</small> | ||
| <p className={styles.memberNames}> | ||
| {(group.members ?? []).map((member) => member.user_name).join('、')} |
There was a problem hiding this comment.
The nullish coalescing operator on line 46 is redundant since the group.members array is defined as non-nullable in the Props type definition (lines 17-20). This defensive check is unnecessary and adds confusion about the actual data type.
| <div className={styles.loader}> | ||
| <span className={styles.box}></span> | ||
| <span className={styles.box}></span> | ||
| <span className={styles.box}></span> | ||
| <span className={styles.box}></span> | ||
| <span className={styles.box}></span> | ||
| </div> |
There was a problem hiding this comment.
The Error component displays a "loader" animation with boxes, which is typically associated with loading states, not error states. This creates a confusing user experience where an error message is accompanied by a loading animation. Consider either removing the loader animation from error states or using a different visual indicator more appropriate for errors (like an icon or static visual element).
| <div className={styles.loader}> | ||
| <span className={styles.box}></span> | ||
| <span className={styles.box}></span> | ||
| <span className={styles.box}></span> | ||
| <span className={styles.box}></span> | ||
| <span className={styles.box}></span> | ||
| </div> |
There was a problem hiding this comment.
The empty spans used for the loading animation boxes lack semantic meaning and accessibility context. Consider adding aria-hidden="true" to the loader container div to hide this decorative animation from screen readers, improving the experience for users with assistive technologies.
| <div className={styles.loader}> | ||
| <span className={styles.box}></span> | ||
| <span className={styles.box}></span> | ||
| <span className={styles.box}></span> | ||
| <span className={styles.box}></span> | ||
| <span className={styles.box}></span> | ||
| </div> |
There was a problem hiding this comment.
Similarly, the empty spans used for the error animation boxes lack semantic meaning and accessibility context. Consider adding aria-hidden="true" to the loader container div to hide this decorative animation from screen readers.
| <button | ||
| className={styles.button} | ||
| type="button" | ||
| onClick={() => props.inviteUrl && inviteUrlHandler(props.inviteUrl)} |
There was a problem hiding this comment.
The onClick handler on line 34 has redundant null checking. The props.inviteUrl check is unnecessary because the button is already conditionally rendered based on props.inviteUrl being truthy (line 30), guaranteeing it won't be null at this point. The onClick can be simplified to just call inviteUrlHandler(props.inviteUrl).
| const creditorId = watch('creditor_id'); | ||
|
|
||
| // メンバー一覧の取得 | ||
| // メンバー 一覧の取得 |
There was a problem hiding this comment.
The comment says "メンバー 一覧の取得" (Getting member list) but there's an extra space between "メンバー" and "一覧". This should be "メンバー一覧の取得" without the space for consistency with other similar comments in the codebase.
| // メンバー 一覧の取得 | |
| // メンバー一覧の取得 |
| .box:nth-child(2n) { | ||
| animation-delay: 0s; | ||
| } | ||
| .box:nth-child(2n + 1) { | ||
| animation-delay: 0.6s; |
There was a problem hiding this comment.
The animation delay pattern for the Error component uses 2n (even) and 2n+1 (odd) selectors with only two different delays (0s and 0.6s), which creates a pattern where boxes 2 and 4 animate together (0s) and boxes 1, 3, and 5 animate together (0.6s). This creates an unbalanced animation. Consider using individual nth-child selectors with incremental delays (like in Loading component) for a smoother, more balanced wave effect.
| const FullPaymentButton: FC<Props> = (props: Props) => { | ||
| return ( | ||
| <button className={styles.button} type="button" onClick={props.onClick} disabled={props.disabled}> | ||
| {!props.disabled && '完済'} |
There was a problem hiding this comment.
The button content disappears when disabled (line 13), leaving an empty button. This creates a poor user experience as the button becomes a blank circle when disabled. The text should remain visible (possibly with reduced opacity) to maintain the button's purpose and improve accessibility for screen readers.
| // css | ||
| import styles from './index.module.css'; | ||
| import { Link } from 'react-router'; |
There was a problem hiding this comment.
The Link import from 'react-router' is placed after the CSS import on line 19, breaking the typical import ordering convention. It should be moved up to group with other external library imports, likely near the top of the file after the react import.
| // css | |
| import styles from './index.module.css'; | |
| import { Link } from 'react-router'; | |
| import { Link } from 'react-router'; | |
| // css | |
| import styles from './index.module.css'; |
| <> | ||
| <h3 className={styles.memberTitle}>参加メンバー</h3> | ||
| <p className={styles.memberNames}> | ||
| {(props.groupInfoResult?.members ?? []).map((member) => member.user_name).join('、')} |
There was a problem hiding this comment.
Similarly, the nullish coalescing operator on line 23 is redundant since props.groupInfoResult.members is defined as a non-nullable array in the Props type definition (lines 11-14). This defensive check is unnecessary.
| {(props.groupInfoResult?.members ?? []).map((member) => member.user_name).join('、')} | |
| {props.groupInfoResult.members.map((member) => member.user_name).join('、')} |
No description provided.