Conversation
whnbaek
commented
Apr 8, 2026
whnbaek
commented
- Signup: show conflict details when email/student number already registered
- Signup: hide username/password form when conflicts exist
- Sync: remove email section (email is identity, not syncable)
- Update descriptions to reflect email verification before sync
- Remove text-dimmed from dashboard canvas description
- Change canvas badge to "Canvas 토큰으로도 가입 가능"
- Signup: show conflict details when email/student number already registered - Signup: hide username/password form when conflicts exist - Sync: remove email section (email is identity, not syncable) - Update descriptions to reflect email verification before sync - Remove text-dimmed from dashboard canvas description - Change canvas badge to "Canvas 토큰으로도 가입 가능"
There was a problem hiding this comment.
Pull request overview
This PR updates the Canvas-based signup/sync experience by adding signup conflict handling, adjusting the sync UI to treat email as identity (not a sync target), and refining related i18n copy.
Changes:
- Add conflict display in Canvas signup preview and hide the username/password signup form when conflicts exist.
- Remove email diff selection/rendering from the Canvas sync “diff/apply” UI and update copy to reflect email verification prior to syncing.
- Minor UI/i18n tweaks (dashboard description styling, badge text updates, new conflict/mismatch strings).
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| locale/ko.json | Updates Canvas descriptions and adds new conflict/mismatch strings (Korean). |
| locale/en.json | Updates Canvas descriptions and adds new conflict/mismatch strings (English). |
| app/[locale]/signup/CanvasSignupFlow.tsx | Shows conflict details and conditionally hides the signup credential form when conflicts exist. |
| app/[locale]/(authorized)/page.tsx | Removes text-dimmed styling from the Canvas dashboard description. |
| app/[locale]/(authorized)/canvas/CanvasDiff.tsx | Removes email diff selection/actions/rendering from Canvas sync diff UI. |
| api/canvas.ts | Extends preview type with conflicts and removes add_email from supported Canvas actions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "statusMember": "Joined", | ||
| "statusPending": "Pending approval", | ||
| "canvasLinkedBadge": "Canvas Linked", | ||
| "canvasLinkedBadge": "Also joinable via Canvas token", |
There was a problem hiding this comment.
The string "Also joinable via Canvas token" is grammatically incomplete; it should include an article (e.g., "via a Canvas token") to read naturally in English.
| "canvasLinkedBadge": "Also joinable via Canvas token", | |
| "canvasLinkedBadge": "Also joinable via a Canvas token", |
| const selectableItems = [ | ||
| ...(diff.profiles?.toAdd ?? []).map(p => `sn_${p.studentNumber}`), | ||
| ...diff.emails.toAdd.map(e => `em_${e.local}@${e.domain}`), | ||
| ...diff.groups.items.filter(g => g.status === 'new' || g.status === 'pending').map(g => `g_${g.groupIdx}`), | ||
| ]; |
There was a problem hiding this comment.
Canvas sync UI no longer renders or applies email diffs, but CanvasDiff still includes an emails section in the client type. If the backend still returns email-only changes, the UI will show "noChanges" even though the payload contains differences (they're just ignored). Consider removing emails from CanvasDiff (and the backend response) or explicitly marking/handling email diffs as non-syncable so the UI messaging stays accurate.
| @@ -34,7 +40,6 @@ export type CanvasDiff = { | |||
|
|
|||
| export type CanvasAction = | |||
| | { type: 'add_student_number'; studentNumber: string } | |||
| | { type: 'add_email'; emailLocal: string; emailDomain: string } | |||
| | { type: 'add_group'; groupIdx: number }; | |||
|
|
|||
There was a problem hiding this comment.
CanvasAction no longer includes add_email, but CanvasDiff still models emails as a sync diff. To keep the client API model aligned with the new "email is identity, not syncable" behavior, consider also removing the emails field from CanvasDiff (and updating the /canvas/sync response) so future UI/logic doesn’t accidentally assume email syncing is supported.
