feat: Refine email linking UX and i18n#3622#3642
Draft
nicolas-machugounot wants to merge 13 commits intoopenfrontio:mainfrom
Draft
feat: Refine email linking UX and i18n#3622#3642nicolas-machugounot wants to merge 13 commits intoopenfrontio:mainfrom
nicolas-machugounot wants to merge 13 commits intoopenfrontio:mainfrom
Conversation
…rs dans le processus de contribution et les exigences de qualité du code.
…es diviser en plusieurs parties
…ront afin d'améliorer le processus de contribution et de validation.
…endance à Discord, incluant les objectifs, les endpoints API, et les critères d'acceptation.
…session authentifiée et payload email + redirectDomain. Ajout d’une section UI dans le compte connecté Discord (sans email lié) pour saisir un email de secours et lancer la liaison. Gestion des retours utilisateur via alertes succès/erreur, sans casser le flow existant de magic link classique.
…mail de liaison Gestion du callback #link-email?link-email-token=... qui confirme la liaison Appel POST /auth/link/email/confirm pour valider le token et lier l'email au compte UI améliorée dans le modal compte (section de liaison email pour Discord sans email lié) Messages d'alerte explicites et rafraîchissement auto après confirmation
…session authentifiée et payload email + redirectDomain. Ajout d’une section UI dans le compte connecté Discord (sans email lié) pour saisir un email de secours et lancer la liaison. Gestion des retours utilisateur via alertes succès/erreur, sans casser le flow existant de magic link classique.
…th.ts. Ajout de "ignoreDeprecations": "6.0" dans tsconfig.json pour lever l’erreur liée à baseUrl.
Contributor
WalkthroughAdds email backup/link workflow for Discord-linked users (frontend UI, client auth helpers, URL handling), new translation keys, a staged implementation doc for Issue Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as AccountModal
participant ClientAuth as Client Auth
participant Server as Backend API
User->>UI: Enter email and click "start link"
UI->>ClientAuth: sendLinkEmailStart(email)
ClientAuth->>Server: POST /auth/link/email/start (with Auth header)
Server-->>ClientAuth: 200 OK
ClientAuth-->>UI: true
UI-->>User: Show success alert, clear input
rect rgba(100, 150, 200, 0.5)
Note over User,Server: User receives email and clicks verification link
end
User->>UI: Navigate to URL with `#link-email-token`
UI->>ClientAuth: confirmEmailLink(token)
ClientAuth->>Server: POST /auth/link/email/confirm (token)
Server-->>ClientAuth: 200 OK with { email }
ClientAuth-->>UI: { email }
UI-->>User: Show success alert
Note over UI,User: Page reloads after 1.5s to refresh session
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/client/AccountModal.ts (1)
357-385: Add in-flight protection to prevent duplicate link-start requests.A quick double-click can send multiple start-link requests. Consider a small loading guard and button disable state.
Suggested refactor
- `@state`() private linkEmail: string = ""; + `@state`() private linkEmail: string = ""; + `@state`() private isStartingEmailLink: boolean = false; ... - <button + <button `@click`="${this.handleStartEmailLink}" + ?disabled=${this.isStartingEmailLink} class="w-full rounded-xl border border-white/5 bg-gradient-to-r from-blue-600 to-blue-700 px-6 py-3 text-sm font-bold uppercase tracking-wider text-white shadow-lg transition-all hover:from-blue-500 hover:to-blue-600 hover:shadow-blue-900/40" > ... private async handleStartEmailLink() { + if (this.isStartingEmailLink) return; const email = this.linkEmail.trim(); if (!email) { alert(translateText("account_modal.enter_email_address")); return; } - const success = await sendLinkEmailStart(email); - if (success) { - alert( - translateText("account_modal.recovery_email_sent", { - email, - }), - ); - this.linkEmail = ""; - return; - } - - alert(translateText("account_modal.failed_to_send_recovery_email")); + this.isStartingEmailLink = true; + try { + const success = await sendLinkEmailStart(email); + if (success) { + alert( + translateText("account_modal.recovery_email_sent", { + email, + }), + ); + this.linkEmail = ""; + return; + } + alert(translateText("account_modal.failed_to_send_recovery_email")); + } finally { + this.isStartingEmailLink = false; + } }Also applies to: 416-435
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/AccountModal.ts` around lines 357 - 385, renderEmailLinkSection currently allows rapid double-clicks to trigger duplicate requests; add an in-flight guard property (e.g., linkInProgress or isLinkingEmail) and use it to disable the "send backup link" button and short-circuit subsequent calls. Update handleStartEmailLink to check the flag at the start, set it true before beginning the async request, and set it false in both success and error/finally paths so the button becomes enabled again; also bind the button disabled state to that flag in renderEmailLinkSection. Apply the same pattern to the similar email-link UI/handler referenced around lines 416-435.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/client/AccountModal.ts`:
- Around line 423-434: The success/failure alerts in the sendLinkEmailStart flow
use recovery-email translation keys; update the messages to use backup-link
specific keys instead. In the block that calls sendLinkEmailStart(email) and
shows alerts, replace translateText("account_modal.recovery_email_sent", { email
}) with the backup-link success key (e.g., "account_modal.backup_link_sent" or
similar) and replace
translateText("account_modal.failed_to_send_recovery_email") with the
backup-link failure key (e.g., "account_modal.failed_to_send_backup_link"); keep
clearing this.linkEmail and the control flow the same so only the translation
keys change.
In `@src/client/Auth.ts`:
- Around line 269-271: The code reads an untyped response JSON and returns {
email } without validating it; update the logic around const json = await
response.json() in Auth.ts to verify that json.email exists and is a string
(e.g., typeof json.email === 'string') before returning the success object, and
if the check fails, return an appropriate error path or throw a descriptive
error so the function's typed success shape is upheld.
In `@tsconfig.json`:
- Line 10: The TS compiler option "ignoreDeprecations" in tsconfig is set to an
incompatible value ("6.0"); update the JSON property "ignoreDeprecations" to
"5.0" so it matches the installed TypeScript version (e.g., change the value of
the "ignoreDeprecations" key in tsconfig.json from "6.0" to "5.0").
---
Nitpick comments:
In `@src/client/AccountModal.ts`:
- Around line 357-385: renderEmailLinkSection currently allows rapid
double-clicks to trigger duplicate requests; add an in-flight guard property
(e.g., linkInProgress or isLinkingEmail) and use it to disable the "send backup
link" button and short-circuit subsequent calls. Update handleStartEmailLink to
check the flag at the start, set it true before beginning the async request, and
set it false in both success and error/finally paths so the button becomes
enabled again; also bind the button disabled state to that flag in
renderEmailLinkSection. Apply the same pattern to the similar email-link
UI/handler referenced around lines 416-435.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c4fffa37-9e8a-4ecc-a896-3dec29315ba9
📒 Files selected for processing (9)
.github/agents/openfront-contributor.agent.md.github/chatmodes/openfront-contributor.chatmode.md.github/prompts/openfront-contribution-readiness.prompt.mddocs/issue-3622-login-alternative-plan.mdresources/lang/en.jsonsrc/client/AccountModal.tssrc/client/Auth.tssrc/client/Main.tstsconfig.json
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
Author
|
Resolves #3622 Description:This PR refines the email linking UX and i18n for issue #3622. It keeps the PR focused on the client-side email backup flow, with clearer labels, trimmed email input, a translated callback failure message, and a documentation update that matches the POST confirmation contract. Scope
Changes
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:_nicolitoo Testing performed
|
github-project-automation
bot
moved this from Development
to Final Review
in OpenFront Release Management
…s chemins et des dépréciations
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/client/Main.ts`:
- Around line 314-320: Avoid unsafe Event-to-CustomEvent casts by typing the
event parameter in the listeners: change the "leave-lobby" listener to supply a
callback signature that accepts CustomEvent<{ lobby: string | number; cause?:
string }> (or simply omit the parameter since handleLeaveLobby doesn't use it)
and invoke this.handleLeaveLobby with that typed event; change the
"update-game-config" listener to accept CustomEvent<{ config: unknown }> and
pass that typed event into handleUpdateGameConfig so the handler can safely
access event.detail.config; keep the "kick-player" listener as-is (it already
binds this) but ensure any handlers that read event.detail use appropriately
typed CustomEvent parameters (refer to handleLeaveLobby and
handleUpdateGameConfig for the exact handler names).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 93a29ce0-3030-4bd5-8230-ca946991fcc7
📒 Files selected for processing (3)
src/client/Auth.tssrc/client/Main.tstsconfig.json
🚧 Files skipped from review as they are similar to previous changes (1)
- tsconfig.json
Comment on lines
+314
to
+320
| document.addEventListener("leave-lobby", (event: Event) => { | ||
| void this.handleLeaveLobby(event as CustomEvent); | ||
| }); | ||
| document.addEventListener("kick-player", this.handleKickPlayer.bind(this)); | ||
| document.addEventListener( | ||
| "update-game-config", | ||
| this.handleUpdateGameConfig.bind(this), | ||
| ); | ||
| document.addEventListener("update-game-config", (event: Event) => { | ||
| this.handleUpdateGameConfig(event as CustomEvent); | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify emitters/listeners for leave-lobby and update-game-config event shape usage.
rg -n --type=ts -C3 'addEventListener\("leave-lobby"|addEventListener\("update-game-config"'
rg -n --type=ts -C3 'dispatchEvent\(\s*new\s+CustomEvent\("leave-lobby"|dispatchEvent\(\s*new\s+CustomEvent\("update-game-config"'
rg -n --type=ts -C3 'dispatchEvent\(\s*new\s+Event\("leave-lobby"|dispatchEvent\(\s*new\s+Event\("update-game-config"'Repository: openfrontio/OpenFrontIO
Length of output: 825
🏁 Script executed:
#!/bin/bash
# Find all dispatch sites for leave-lobby event
rg -n --type=ts 'dispatchEvent.*leave-lobby|CustomEvent.*leave-lobby' -A 2 -B 2Repository: openfrontio/OpenFrontIO
Length of output: 1474
🏁 Script executed:
#!/bin/bash
# Find all dispatch sites for update-game-config event
rg -n --type=ts 'dispatchEvent.*update-game-config|CustomEvent.*update-game-config' -A 2 -B 2Repository: openfrontio/OpenFrontIO
Length of output: 368
🏁 Script executed:
#!/bin/bash
# Check the handler implementations
rg -n --type=ts 'handleLeaveLobby|handleUpdateGameConfig' -A 8Repository: openfrontio/OpenFrontIO
Length of output: 2221
Avoid unchecked Event → CustomEvent casts.
Lines 315 and 319 use type assertions to cast plain Event to CustomEvent, but the listeners should declare the event type directly in the callback signature. While current dispatch sites consistently use CustomEvent with proper detail objects, explicit typing prevents runtime errors if the contract changes.
For "leave-lobby": The detail contains { lobby: string | number, cause?: string }. Line 315's handler doesn't access the event anyway, so the cast is unnecessary.
For "update-game-config": The detail contains { config: unknown }. Line 319's handler directly accesses event.detail.config without type safety.
document.addEventListener("leave-lobby", (event: CustomEvent<{ lobby: string | number; cause?: string }>) => {
void this.handleLeaveLobby(event);
});
document.addEventListener("update-game-config", (event: CustomEvent<{ config: unknown }>) => {
this.handleUpdateGameConfig(event);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| document.addEventListener("leave-lobby", (event: Event) => { | |
| void this.handleLeaveLobby(event as CustomEvent); | |
| }); | |
| document.addEventListener("kick-player", this.handleKickPlayer.bind(this)); | |
| document.addEventListener( | |
| "update-game-config", | |
| this.handleUpdateGameConfig.bind(this), | |
| ); | |
| document.addEventListener("update-game-config", (event: Event) => { | |
| this.handleUpdateGameConfig(event as CustomEvent); | |
| }); | |
| document.addEventListener("leave-lobby", (event: CustomEvent<{ lobby: string | number; cause?: string }>) => { | |
| void this.handleLeaveLobby(event); | |
| }); | |
| document.addEventListener("kick-player", this.handleKickPlayer.bind(this)); | |
| document.addEventListener("update-game-config", (event: CustomEvent<{ config: unknown }>) => { | |
| this.handleUpdateGameConfig(event); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/client/Main.ts` around lines 314 - 320, Avoid unsafe Event-to-CustomEvent
casts by typing the event parameter in the listeners: change the "leave-lobby"
listener to supply a callback signature that accepts CustomEvent<{ lobby: string
| number; cause?: string }> (or simply omit the parameter since handleLeaveLobby
doesn't use it) and invoke this.handleLeaveLobby with that typed event; change
the "update-game-config" listener to accept CustomEvent<{ config: unknown }> and
pass that typed event into handleUpdateGameConfig so the handler can safely
access event.detail.config; keep the "kick-player" listener as-is (it already
binds this) but ensure any handlers that read event.detail use appropriately
typed CustomEvent parameters (refer to handleLeaveLobby and
handleUpdateGameConfig for the exact handler names).
nicolas-machugounot
changed the title
Contributor
|
Looks like build + tests are failing, and you need to make sure to update the description, also you comitted the pr.md's which you used to prompt AI for.. |
This was referenced Apr 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
This PR refines the email linking UX and i18n for issue #3622. It keeps the PR focused on the client-side email backup flow, with clearer labels, trimmed email input, a translated callback failure message, and a documentation update that matches the POST confirmation contract.
Scope
Changes
Testing performed
npm run lint
npm test -- tests/TranslationSystem.test.ts
I have added screenshots for all UI updates
I process any text displayed to the user through translateText() and I've added it to the en.json file
I have added relevant tests to the test directory
I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced