Fixes & cleans up Username input

[VariableVince]

Description:

Fix:
Error box like "Username must be at least 3 characters long" sometimes still remains after the game starts. See bug report with screenshot: https://discord.com/channels/1284581928254701718/1449169462426079343/1449169462426079343

This is while in main.ts, element username-validation-error is already set to hidden. Most of the time this works but apparently sometimes there's a re-render which removes the 'hidden' tag again. Most probable solution for this edge case: clear validationError first. Then to make sure, still hide element username-validation-error.

Cleanup username.ts:

• Remove cat and clover emojis from validPattern. For single player games, the only other validity checks are done from GameRunner calling sanitize() from Util.ts. And from GameImpl where from addPlayer the PlayerImpl are created, which uses sanitizeUserName also from username.ts, which uses validPattern again. Both GameRunner and PlayerImpl therefor allow emoji. But for muliplayer there's an extra step where ClientJoinMessageSchema enforces UserNameSchema = SafeString. Which does NOT allow cat or clover emoji. So for multiplayer you get an error and can't join a game with cat or clover emoji. To align their behavior more, just disallow the emojis from validPattern from the start. Almost no-one ever used them anyway, they were once put in because a developer liked to use them before the existance of ClientJoinMessageSchema. There's more consolidation/refactoring possible but this is an important first step.
• Remove sending arg max: MAX_USERNAME_LENGTH for translation key invalid_chars, since the translation string doesn't contain params

Cleanup UserNameInput.ts:

• Remove userSettings: isn't used anywhere in the code
• Remove dispatchUsernameEvent: nowhere in the repo is event 'username-change' listened to. Also, dispatchUsernameEvent is only called from connectedCallBack but not anywhere else. It seems like handleChange was made for this. Also main.ts calls usernameInput.getCurrentUsername() and doesn't listen for the event either.

Please complete the following:

• 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

Please put your Discord username so you can be contacted if a bug or regression is found:

tryout33

[coderabbitai]

Walkthrough

Clears username input validation state during modal cleanup, removes event-dispatch coupling from the UsernameInput component, and tightens the username validation pattern to disallow emoji and related characters.

Changes

Cohort / File(s)	Summary
UI Modal Cleanup src/client/Main.ts	Added guard to clear usernameInput.validationError when hiding modals after starting a game so the validation error state isn't retained.
Username Input Decoupling src/client/UsernameInput.ts	Removed UserSettings coupling and the dispatchUsernameEvent method and its invocation; retained username persistence and validation logic.
Validation Pattern Tightening src/core/validations/username.ts, tests/Censor.test.ts	Updated username regex to remove emoji from allowed characters and adjusted validation error message usage; updated tests to reflect emoji removal and sanitization changes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Check for other consumers relying on the removed "username-change" CustomEvent.
• Validate the updated regex doesn't exclude allowed international characters beyond intended emoji removal.
• Confirm modal-close timing ensures the cleared validationError doesn't race with UI updates.

Possibly related PRs

• Fix modal not closing when game starts #1504 — also modifies modal-closing logic in src/client/Main.ts (adds more modal IDs to close).
• REV - Improved Username Censoring #1119 — touches username validation/sanitization (src/core/validations/username.ts) and related tests.
• Fix: Hide username validation error in-game #1110 — also adjusts hiding/resetting username validation error in the lobby-join flow.

Suggested labels

Bug Fix

Suggested reviewers

• evanpelle
• scottanderson

Poem

✨ A tiny fix, a cleaner slate,
Validation cleared before it's late.
Emoji gone from names we store,
Events pared back, code breathes once more,
Modals close — the UI's great. 🎉

Pre-merge checks

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main changes: fixing a username validation error box bug and cleaning up the UsernameInput component with removed unused code.
Description check	✅ Passed	The description is well-related to the changeset, providing detailed context for the bug fix and cleanup rationale across all modified files.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9d10ca7 and 9ad5568.

📒 Files selected for processing (3)

• src/client/Main.ts (1 hunks)
• src/client/UsernameInput.ts (0 hunks)
• src/core/validations/username.ts (2 hunks)

💤 Files with no reviewable changes (1)

• src/client/UsernameInput.ts

🧰 Additional context used

🧠 Learnings (2)

📚 Learning: 2025-06-09T02:20:43.637Z

Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.

Applied to files:

• src/client/Main.ts

📚 Learning: 2025-10-08T17:14:49.369Z

Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: In `ClientGameRunner.ts`, the `myPlayer` field is always set when `shouldPreventWindowClose()` is called, so the null check in that method is sufficient without needing to fetch it again from `gameView.playerByClientID()`.

Applied to files:

• src/client/Main.ts

🧬 Code graph analysis (1)

src/core/validations/username.ts (1)

src/client/Utils.ts (1)

• translateText (92-147)

🪛 GitHub Actions: 🧪 CI

src/core/validations/username.ts

[error] 68-94: Username validation: Unicode characters (e.g., 🐈, ü) are not being accepted. Test expected validateUsername to return true for 'Cat🐈Üser' but received false.

---

[error] 96-132: Username sanitization: Unicode characters (e.g., Ünicode🐈Test) are being stripped/removed unexpectedly. Test expected sanitizeUsername to preserve Unicode but received output without the emoji.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Deploy to openfront.dev

🔇 Additional comments (1)

src/client/Main.ts (1)

502-512: Solid fix for the error box edge case; validationError is a proper public contract.

The validationError property on UsernameInput is explicitly marked with the @property decorator (line 15 of src/client/UsernameInput.ts) with type string and default value "". This confirms:

• It's an intentional public API, not internal state
• "" is the correct "no error" value (used consistently in lines 63 and 65)
• Clearing it before hiding the DOM element prevents re-render from restoring the error message

The pattern matches the existing direct DOM cleanup style in Main.ts. The fix is safe and correct.

[Lavodan]

Looks good just left some minor comments if we want to do it right-right.
CodeRabbit is also correct about the incorrect test, should be changed to suit the new changes

[Lavodan]

Could we adhere to DRY by importing the expression from UserNameSchema, or does the logic not work in this context?

[VariableVince]

About that, see CodeRabbits response: #2619 (comment)

Also, this PR isn't meant as a full refactor. We could still do that later on. Because now, there are several places with validation/sanitization which could be consolidated more. Maybe in the two weeks that are coming. This is just meant as a bug fix for now where single player is handled differently than multiplayer when it comes to accepting emoji (plus the not-so-clear error message for multiplayer leaves the player in the dark as to the reason).

[Lavodan]

This approach is good for the current paradigm we're using.
I'm just wondering, is there any reason to keep these elements in the DOM at all? As far as I know, the only way we handle getting the main menu back is by refreshing the page anyways, so why do we keep these elements in place?

[VariableVince]

Good question. As said above this is just meant a bugfix (and some obvious cleanup), not as a full refactor. Maybe we can do that later on?

[Lavodan]

Sounds good

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

tests/Censor.test.ts (2)

108-111: Good update to reflect emoji removal.

The test correctly verifies that valid Unicode characters like ü remain allowed while emoji are no longer accepted.

Consider adding an explicit test case to verify emoji rejection for clarity:

test("rejects emoji characters", () => {
  const res = validateUsername("User🐈Name");
  expect(res.isValid).toBe(false);
  expect(res.error).toBe("username.invalid_chars");
});

This would make the behavioral change more explicit in the test suite, though the existing "invalid chars" test on line 100 already covers this scenario.

---

126-126: Test input correctly updated for emoji removal.

The sanitization test now verifies that Unicode characters (Ü) and spaces are preserved while invalid characters (!) are removed.

For explicit coverage of emoji handling in sanitization, consider adding a dedicated test case:

{ input: "User🐈Name", expected: "User Name" },

This would clearly document how emoji are handled during sanitization (replaced/removed), though the existing test coverage should already validate this behavior.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9ad5568 and 2d371ce.

📒 Files selected for processing (2)

• src/core/validations/username.ts (2 hunks)
• tests/Censor.test.ts (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• src/core/validations/username.ts

🧰 Additional context used

🧬 Code graph analysis (1)

tests/Censor.test.ts (1)

src/core/validations/username.ts (1)

• validateUsername (97-132)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Deploy to openfront.dev

[evanpelle]

thanks!