Lobby chat panel

[Phantasm0009]

Resolves #2490

Description:

Adds a new Lobby Chat feature for private game lobbies. When enabled by the host, players can communicate in a live chat panel before the match begins. Useful for confirming teams, rules, and ready-checks without needing external tools like Discord.

Key Features:

• In-lobby chat panel appears for all players if enabled during lobby creation.
• Messages are styled:
  • Local player's messages: right-aligned and tinted
  • Other players: left-aligned

• All UI strings passed through translateText() with keys added to en.json.
• Chat uses event-bus binding on event-bus:ready, with lazy fallback.
• Toggle for "Enable Lobby Chat" appears in host settings and is disabled for public games.

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:

.fruitymctooty

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

Warning

Rate limit exceeded

@Phantasm0009 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 7 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 7a28c49 and 254469d.

📒 Files selected for processing (5)

• src/client/ClientGameRunner.ts (2 hunks)
• src/client/components/LobbyChatPanel.ts (1 hunks)
• src/core/Schemas.ts (6 hunks)
• src/server/GameServer.ts (2 hunks)
• tests/LobbyChatSchemas.test.ts (1 hunks)

Walkthrough

This pull request adds in-lobby chat functionality to enable player communication before matches. It introduces a new frontend chat panel component, backend message validation and broadcasting, schema definitions for chat messages, event bus integration, and localization strings. Changes span client UI, server-side handling, core type definitions, tests, and configuration updates.

Changes

Cohort / File(s)	Summary
Mock & Test Setup __mocks__/jose.js, jest.config.ts, eslint.config.js, tsconfig.json	Establishes mock jose library for JWT testing; configures Jest to use mocks; adds mock files to ESLint and TypeScript project includes; updates Husky to v10 compatibility
Frontend Chat Component src/client/components/LobbyChatPanel.ts	New LitElement-based chat panel with message history, input field (300-char limit), EventBus integration, auto-scroll, and client ID tracking for message alignment
Client Message Transport & Events src/client/Transport.ts, src/client/ClientGameRunner.ts	Introduces SendLobbyChatEvent class; registers event handlers; exposes EventBus and client ID on window; dispatches event-bus:ready and lobby-chat:message custom events
Lobby UI Integration src/client/HostLobbyModal.ts, src/client/JoinPrivateLobbyModal.ts	Adds chatEnabled state and option-card toggle (private lobbies only); conditionally renders LobbyChatPanel; wires chat config to game payload
Game Config & Single Player src/client/SinglePlayerModal.ts, src/server/GameManager.ts, src/server/MapPlaylist.ts	Adds chatEnabled: false field to game configuration in client and server default game setups
Core Schema & Type System src/core/Schemas.ts	Defines ClientLobbyChatSchema and ServerLobbyChatSchema; extends ClientMessage and ServerMessage unions; adds chatEnabled boolean field to GameConfigSchema with default false
Server Message Handling src/server/GameServer.ts	Enforces chat disabled for public lobbies; validates chat enabled status and game phase; broadcasts lobby chat messages to all active clients
Localization resources/lang/en.json	Adds lobby_chat translation object with keys: title, placeholder, enable, send
Test Suites & Configuration tests/LobbyChatPanel.test.ts, tests/LobbyChatSchemas.test.ts, tests/util/Setup.ts	Adds i18n key presence validation; introduces schema validation tests for GameConfig, ClientLobbyChatMessage, and ServerLobbyChatMessage; includes chatEnabled in test configuration

Sequence Diagram

sequenceDiagram
    actor Player
    participant LobbyChatPanel
    participant EventBus
    participant Transport
    participant Server
    participant Clients

    Player->>LobbyChatPanel: Types message + presses Send
    LobbyChatPanel->>EventBus: dispatch(SendLobbyChatEvent)
    EventBus->>Transport: onSendLobbyChat handler invoked
    Transport->>Server: Send lobby_chat message {type, text, clientID}
    
    Server->>Server: Validate: game in Lobby phase?
    Server->>Server: Validate: chat enabled & not public?
    Server->>Clients: Broadcast lobby_chat {type, sender, text, username, isHost}
    
    Clients->>Clients: Trigger lobby-chat:message event
    Clients->>LobbyChatPanel: Event caught via EventBus listener
    LobbyChatPanel->>LobbyChatPanel: Append message to history
    LobbyChatPanel->>Player: Render message (aligned by sender)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~30–45 minutes

Areas requiring extra attention:

• Schema validation logic in src/core/Schemas.ts — verify chat message type structure matches both client and server expectations
• Server broadcast mechanism in src/server/GameServer.ts — ensure chat is restricted to Lobby phase and non-public lobbies
• EventBus integration in LobbyChatPanel.ts — confirm event listener lifecycle, memory cleanup, and window object exposure
• Chat toggle state sync in HostLobbyModal.ts and JoinPrivateLobbyModal.ts — verify config updates propagate correctly to server
• Test coverage — LobbyChatPanel tests are noted as temporary placeholders pending ESM fix

Suggested labels

Feature - New, Feature - Frontend, UI/UX, Feature - Test

Suggested reviewers

• evanpelle
• scottanderson
• drillskibo

Poem

🎤 In lobbies now, players chat and cheer,
Before the match, voices crystal clear,
No Discord needed, rules align,
Teams confirm, readiness defined,
One lobby, one voice—community divine ✨

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 (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'Lobby chat panel' clearly describes the main feature added in this changeset—a new chat panel component for game lobbies.
Description check	✅ Passed	The description is well-related to the changeset, explaining the lobby chat feature, its use case, key features, UI styling, and testing checklist.
Linked Issues check	✅ Passed	The PR addresses all objectives from issue #2490 [#2490]: provides an in-lobby chat panel, enables player communication while waiting for match start, facilitates team/rule confirmation and status updates, and restricts chat to private lobbies only.
Out of Scope Changes check	✅ Passed	All changes are scoped to lobby chat implementation: UI components, schemas, event handling, server validation, test coverage, and configuration updates. Supporting changes to Husky and ESLint are necessary maintenance and are proportional.

---

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: 2

🧹 Nitpick comments (7)

src/client/graphics/layers/StructureIconsLayer.ts (1)

275-276: Consider extracting filter constants to reduce as any casts.

The as any cast is repeated six times. You could define typed constants once and reuse them, which improves readability and keeps the type workaround in one place:

// Near top of file or in a shared utils
const OUTLINE_RED = new OutlineFilter(2, 0xff0000) as PIXI.Filter;
const OUTLINE_GREEN = new OutlineFilter(2, 0x00ff00) as PIXI.Filter;
const OUTLINE_WHITE = new OutlineFilter(2, 0xffffff) as PIXI.Filter;

Then use [OUTLINE_RED], [OUTLINE_GREEN], etc. in your filter assignments. This keeps the type cast in one spot and makes the intent clearer.

src/client/ClientGameRunner.ts (1)

77-84: Consider alternatives to global window pollution.

While exposing EventBus and clientID globally enables lightweight components to access them, this pattern bypasses type safety and creates implicit dependencies.

Consider these alternatives:

Option 1: Pass EventBus via CustomEvent detail

-  (window as any).__eventBus = eventBus;
-  (window as any).__clientID = lobbyConfig.clientID;
   document.dispatchEvent(
     new CustomEvent("event-bus:ready", { 
+      detail: { eventBus, clientID: lobbyConfig.clientID },
       bubbles: true, 
       composed: true 
     }),
   );

Then components can capture the EventBus from the event and store it internally.

Option 2: Create a typed global interface

If globals are necessary, at least make them type-safe:

declare global {
  interface Window {
    __eventBus?: EventBus;
    __clientID?: ClientID;
  }
}

window.__eventBus = eventBus;
window.__clientID = lobbyConfig.clientID;

src/server/GameServer.ts (1)

299-315: Avoid re-parsing the raw message; use the already-validated payload

You already validated the message with ClientMessageSchema and have clientMsg typed. Re‑parsing message and pulling .text from any is unnecessary and slightly weaker.

You can simplify and keep things fully type‑safe:

-          case "lobby_chat": {
-            // Validate lobby chat usage: must be in lobby phase and chat enabled
-            if (this.phase() !== GamePhase.Lobby) {
-              return;
-            }
-            if (!this.gameConfig.chatEnabled || this.isPublic()) {
-              return;
-            }
-            // Broadcast to all clients in the same lobby
-            const payload = JSON.stringify({
-              type: "lobby_chat",
-              sender: client.clientID,
-              text: (JSON.parse(message) as any).text,
-            });
-            this.activeClients.forEach((c) => c.ws.send(payload));
-            break;
-          }
+          case "lobby_chat": {
+            // Validate lobby chat usage: must be in lobby phase and chat enabled
+            if (this.phase() !== GamePhase.Lobby) {
+              return;
+            }
+            if (!this.gameConfig.chatEnabled || this.isPublic()) {
+              return;
+            }
+            // Broadcast to all clients in the same lobby
+            const payload = JSON.stringify({
+              type: "lobby_chat",
+              sender: client.clientID,
+              text: clientMsg.text,
+            });
+            this.activeClients.forEach((c) => c.ws.send(payload));
+            break;
+          }

src/client/HostLobbyModal.ts (1)

55-56: Private-lobby chat toggle wiring looks coherent end-to-end

State → checkbox → putGameConfig → conditional <lobby-chat-panel> render are all consistent, and using translateText("lobby_chat.*") keeps it in the i18n flow.

If you ever reuse this modal for non‑private lobbies, consider also disabling or hiding the checkbox in the UI when the underlying gameType is public, so the client and server rules stay aligned from the player’s point of view. For the current private‑only usage, this is fine as is.

Also applies to: 414-435, 591-602, 800-801

src/client/Transport.ts (1)

175-178: Type the lobby_chat client message instead of using as any

The event wiring is consistent with other intents, but the sendMsg(msg as any) weakens type safety and hides future mistakes.

Consider exporting a dedicated TS type for lobby chat messages from src/core/Schemas.ts (and adding it to the ClientMessage union), then using it here, e.g.:

// After you export something like `ClientLobbyChatMessage` from Schemas:
import {
  // ...
  ClientLobbyChatMessage,
} from "../core/Schemas";

// ...

private onSendLobbyChat(event: SendLobbyChatEvent) {
  const msg: ClientLobbyChatMessage = {
    type: "lobby_chat",
    text: event.text,
    clientID: this.lobbyConfig.clientID,
  };
  this.sendMsg(msg);
}

This keeps the transport strongly typed and in sync with the Zod schema.

Also applies to: 264-264, 646-654

src/client/components/LobbyChatPanel.ts (1)

45-50: Wait for Lit’s update before auto‑scrolling to ensure it reaches the bottom

Right now you update messages and immediately query .messages to set scrollTop. Because Lit renders asynchronously, the container might not yet include the new DOM when you read scrollHeight, so the scroll can be off.

You can make this deterministic with updateComplete:

  private onIncoming = (e: CustomEvent<{ sender: string; text: string }>) => {
     const { sender, text } = e.detail;
-    this.messages = [...this.messages, { sender, text }];
-    const container = this.renderRoot.querySelector(".messages") as HTMLElement;
-    if (container) container.scrollTop = container.scrollHeight;
+    this.messages = [...this.messages, { sender, text }];
+    this.updateComplete.then(() => {
+      const container = this.renderRoot.querySelector(
+        ".messages",
+      ) as HTMLElement | null;
+      if (container) {
+        container.scrollTop = container.scrollHeight;
+      }
+    });
   };

This keeps the scroll behavior stable even under heavier chat traffic.

src/core/Schemas.ts (1)

492-496: Schema structure looks good; consider inlining text definition for clarity.

The server lobby chat schema follows the discriminated union pattern correctly. The text field uses SafeString.max(300), which works but chains a 300-char limit onto SafeString's existing 1000-char limit. Both constraints apply, so the effective limit is 300.

For clarity, consider defining the text field inline:

 export const ServerLobbyChatSchema = z.object({
   type: z.literal("lobby_chat"),
   sender: ID,
-  text: SafeString.max(300),
+  text: z.string()
+    .regex(/^([a-zA-Z0-9\s.,!?@#$%&*()\-_+=[\]{}|;:"'/\u00a9|\u00ae|[\u2000-\u3300]|\ud83c[\ud000-\udfff]|\ud83d[\ud000-\udfff]|\ud83e[\ud000-\udfff]|[üÜ])*$/u)
+    .max(300),
 });

Or extract a shared ChatMessageText schema if both client and server schemas use the same validation.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7d8c1c2 and 64d269b.

⛔ Files ignored due to path filters (1)

• package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (18)

• __mocks__/jose.js (1 hunks)
• jest.config.ts (1 hunks)
• package.json (1 hunks)
• resources/lang/en.json (1 hunks)
• src/client/ClientGameRunner.ts (2 hunks)
• src/client/HostLobbyModal.ts (5 hunks)
• src/client/JoinPrivateLobbyModal.ts (4 hunks)
• src/client/SinglePlayerModal.ts (1 hunks)
• src/client/Transport.ts (3 hunks)
• src/client/components/LobbyChatPanel.ts (1 hunks)
• src/client/graphics/layers/StructureIconsLayer.ts (4 hunks)
• src/core/Schemas.ts (6 hunks)
• src/server/GameManager.ts (1 hunks)
• src/server/GameServer.ts (2 hunks)
• src/server/MapPlaylist.ts (1 hunks)
• tests/LobbyChatPanel.test.ts (1 hunks)
• tests/LobbyChatSchemas.test.ts (1 hunks)
• tests/util/Setup.ts (1 hunks)

🧰 Additional context used

🧠 Learnings (9)

📓 Common learnings

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.

📚 Learning: 2025-06-14T00:56:15.437Z

Learnt from: Aotumuri
Repo: openfrontio/OpenFrontIO PR: 786
File: src/core/Cosmetics.ts:1-1
Timestamp: 2025-06-14T00:56:15.437Z
Learning: The `import { base64url } from "jose";` syntax works correctly in ESM environments for the jose library, contrary to potential assumptions about needing specific ESM import paths like "jose/util/base64url".

Applied to files:

• jest.config.ts
• __mocks__/jose.js

📚 Learning: 2025-06-02T14:27:37.609Z

Learnt from: andrewNiziolek
Repo: openfrontio/OpenFrontIO PR: 1007
File: resources/lang/de.json:115-115
Timestamp: 2025-06-02T14:27:37.609Z
Learning: For OpenFrontIO project: When localization keys are renamed in language JSON files, the maintainers separate technical changes from translation content updates. They wait for community translators to update the actual translation values rather than attempting to translate in the same PR. This allows technical changes to proceed while ensuring accurate translations from native speakers.

Applied to files:

• resources/lang/en.json

📚 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/SinglePlayerModal.ts
• src/client/ClientGameRunner.ts
• src/server/GameServer.ts
• src/client/JoinPrivateLobbyModal.ts
• src/client/components/LobbyChatPanel.ts
• src/client/HostLobbyModal.ts

📚 Learning: 2025-08-12T00:31:50.144Z

Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 1752
File: src/core/game/Game.ts:750-752
Timestamp: 2025-08-12T00:31:50.144Z
Learning: In the OpenFrontIO codebase, changes to the PlayerInteraction interface (like adding canDonateGold and canDonateTroops flags) do not require corresponding updates to src/core/Schemas.ts or server serialization code.

Applied to files:

• src/client/SinglePlayerModal.ts
• tests/LobbyChatSchemas.test.ts
• src/server/GameServer.ts
• src/core/Schemas.ts
• src/server/MapPlaylist.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/ClientGameRunner.ts

📚 Learning: 2025-10-21T20:06:04.823Z

Learnt from: Saphereye
Repo: openfrontio/OpenFrontIO PR: 2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.

Applied to files:

• src/client/ClientGameRunner.ts
• src/client/HostLobbyModal.ts

📚 Learning: 2025-05-21T04:10:33.435Z

Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 784
File: src/core/game/StatsImpl.ts:34-38
Timestamp: 2025-05-21T04:10:33.435Z
Learning: In the codebase, PlayerStats is defined as `z.infer<typeof PlayerStatsSchema>` where PlayerStatsSchema has `.optional()` applied at the object level, making PlayerStats a union type that already includes undefined (PlayerStats | undefined).

Applied to files:

• src/core/Schemas.ts

📚 Learning: 2025-05-21T04:10:33.435Z

Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 784
File: src/core/game/StatsImpl.ts:34-38
Timestamp: 2025-05-21T04:10:33.435Z
Learning: In the codebase, PlayerStats is defined as a type inferred from a Zod schema that is marked as optional, which means PlayerStats already includes undefined as a possible type (PlayerStats | undefined).

Applied to files:

• src/core/Schemas.ts

🧬 Code graph analysis (5)

tests/LobbyChatSchemas.test.ts (1)

src/core/Schemas.ts (3)

• GameConfigSchema (161-181)
• ClientLobbyChatSchema (524-528)
• ServerLobbyChatSchema (492-496)

src/client/Transport.ts (1)

src/core/EventBus.ts (1)

• GameEvent (1-1)

src/server/GameServer.ts (2)

src/server/MapPlaylist.ts (1)

• gameConfig (78-105)

src/core/configuration/DefaultConfig.ts (1)

• gameConfig (266-268)

src/client/components/LobbyChatPanel.ts (2)

src/core/EventBus.ts (1)

• EventBus (7-44)

src/client/Transport.ts (1)

• SendLobbyChatEvent (176-178)

src/client/HostLobbyModal.ts (2)

src/client/LangSelector.ts (1)

• translateText (258-278)

src/client/Utils.ts (1)

• translateText (92-147)

🪛 GitHub Actions: 🧪 CI

__mocks__/jose.js

[error] 1-1: ESLint Parsing error: /home/runner/work/OpenFrontIO/OpenFrontIO/mocks/jose.js was not found by the project service. Consider either including it in the tsconfig.json or including it in allowDefaultProject.

🔇 Additional comments (18)

src/client/graphics/layers/StructureIconsLayer.ts (2)

505-506: LGTM!

The hex color values (0xffffff for white) and new constructor signature are correctly applied for the focused structure highlighting.

---

1-1: I'll help you verify the review comment about @pixi/filter-outline package compatibility. Let me start by examining the project's dependencies and verifying the package information.
<function_calls>

#!/bin/bash

Find package.json to check PixiJS version

find . -name "package.json" -type f | head -5


</function_calls>

Now let me check the PixiJS version and examine the package configuration:
<function_calls>

#!/bin/bash

Read package.json to find PixiJS version

if [ -f "package.json" ]; then
cat package.json | grep -A 2 -B 2 "pixi"
fi


</function_calls>

Let me also verify the current state of the file to understand the import change:
<function_calls>

#!/bin/bash

Check the StructureIconsLayer.ts file

if [ -f "src/client/graphics/layers/StructureIconsLayer.ts" ]; then
head -20 src/client/graphics/layers/StructureIconsLayer.ts
fi


</function_calls>

Now let me search the web for information about @pixi/filter-outline compatibility:
<function_calls>

@pixi/filter-outline npm package compatibility pixi.js v8

</function_calls>

tests/util/Setup.ts (1)

64-64: LGTM!

The addition of chatEnabled: false correctly disables lobby chat in test configurations by default. Tests can override this via the _gameConfig parameter if they need to test chat functionality.

resources/lang/en.json (1)

506-511: LGTM!

The lobby chat localization keys are well-structured and follow the existing naming conventions. The English strings are clear and grammatically correct.

src/client/ClientGameRunner.ts (1)

138-147: LGTM!

The lobby chat message relay logic is clean and straightforward. Dispatching a custom event allows UI components to listen for chat messages without tight coupling to the transport layer.

src/client/SinglePlayerModal.ts (1)

578-578: LGTM!

Setting chatEnabled: false for single-player games is the correct behavior, as lobby chat is only relevant for multiplayer lobbies.

src/server/GameManager.ts (1)

54-54: LGTM!

Adding chatEnabled: false to the default game configuration is appropriate. This ensures chat is disabled by default and must be explicitly enabled, which is a safe default behavior.

jest.config.ts (1)

8-8: I'll verify whether the jose mock covers all usage across the codebase. Let me search for all jose imports and check the mock implementation.

#!/bin/bash

Find all imports from jose library

echo "=== Jose imports in codebase ==="
rg -n --type=ts --type=tsx 'from ["']jose' -A 1

echo ""
echo "=== Jose function calls/usage ==="
rg -n --type=ts --type=tsx '\b(base64url|jwtVerify|decodeJwt|JWK)\b' -C 1

echo ""
echo "=== Checking mocks/jose.js content ==="
cat -n mocks/jose.js 2>/dev/null || echo "File not found at mocks/jose.js"

echo ""
echo "=== Checking alternative mock paths ==="
find . -name "jose.js" -o -name "jose.ts" 2>/dev/null | head -20

package.json (1)

35-35: The @pixi/[email protected] package version is valid and free from known security vulnerabilities.

Web verification confirms that @pixi/filter-outline version 5.2.0 exists on the npm registry (published Feb 28, 2023) and has no reported security vulnerabilities according to Snyk and the npm package database.

src/server/MapPlaylist.ts (1)

78-104: Defaulting chatEnabled to false for public playlist is correct

Keeping chatEnabled explicitly false in the public playlist config matches the schema default and the server’s public‑lobby enforcement. No change needed.

tests/LobbyChatPanel.test.ts (1)

1-35: Good minimal coverage for lobby_chat i18n keys

These lightweight tests are a reasonable stopgap: they ensure the new lobby_chat.* keys exist and are non‑empty until proper component tests are possible.

src/server/GameServer.ts (1)

119-124: Server-side guard for public lobby chat is in the right place

Using isPublic() here to force chatEnabled to false ensures clients can’t enable chat for public games even if they try to PUT it in the config.

src/client/JoinPrivateLobbyModal.ts (1)

8-9: Joiner-side chatEnabled reflection and UI are wired correctly

You read data.gameConfig?.chatEnabled on each poll and gate <lobby-chat-panel> on that flag, so joiners only see chat when the host has enabled it on the server. This matches the intended “host controls chat toggle” behavior.

Also applies to: 22-23, 78-107, 335-337

tests/LobbyChatSchemas.test.ts (1)

1-63: Schema tests cover the key lobby chat behaviors

These tests validate the chatEnabled default and the main happy/validation paths for client/server lobby chat messages, including the max‑length guard. Good targeted coverage for the new schemas.

src/core/Schemas.ts (4)

92-93: LGTM! Type unions extended correctly.

The lobby chat message types are properly added to the client and server message unions, following the existing patterns in the codebase.

Also applies to: 100-101, 117-118

---

170-171: LGTM! Good use of default value for backward compatibility.

The chatEnabled field with .default(false) ensures existing game configs without this field will parse correctly and default to disabled chat. This is the right approach for adding a new optional feature flag to an established schema.

---

524-528: Client schema mirrors server schema correctly.

The client lobby chat schema properly uses clientID to identify the sender, matching the pattern used in BaseIntentSchema for other client-originated messages. The same optional refactor for the text field definition applies here.

---

505-505: LGTM! Schemas properly integrated into discriminated unions.

Both lobby chat schemas are correctly added to their respective discriminated unions, enabling type-safe runtime parsing of chat messages.

Also applies to: 564-564

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

src/client/components/LobbyChatPanel.ts (1)

20-33: Consider typed event handlers instead of as any casts.

The as any casts work but lose type safety. You can use a typed listener approach:

  connectedCallback(): void {
    super.connectedCallback();
-   document.addEventListener("lobby-chat:message", this.onIncoming as any);
+   document.addEventListener("lobby-chat:message", this.onIncoming as EventListener);
    // ...
-   document.addEventListener("event-bus:ready", this.onBusReady as any);
+   document.addEventListener("event-bus:ready", this.onBusReady as EventListener);
  }

Or keep the handler signature compatible with EventListener by checking the event type inside the handler.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 64d269b and f31a3b0.

📒 Files selected for processing (4)

• .husky/pre-commit (1 hunks)
• eslint.config.js (1 hunks)
• src/client/components/LobbyChatPanel.ts (1 hunks)
• tsconfig.json (1 hunks)

✅ Files skipped from review due to trivial changes (1)

• tsconfig.json

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

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.

📚 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/components/LobbyChatPanel.ts

🧬 Code graph analysis (1)

src/client/components/LobbyChatPanel.ts (2)

src/core/EventBus.ts (1)

• EventBus (7-44)

src/client/Transport.ts (1)

• SendLobbyChatEvent (176-178)

🔇 Additional comments (5)

eslint.config.js (1)

28-28: LGTM! Follows the established pattern.

The addition of __mocks__/jose.js to allowDefaultProject is consistent with how fileMock.js is handled on line 27. This ensures ESLint can properly lint the new jose mock file.

src/client/components/LobbyChatPanel.ts (2)

1-10: Clean imports and typed interface.

Good use of a simple typed interface for ChatMessage. Keeps things clear.

---

76-108: Good template structure with proper 300-char limit.

Clean render method. The maxlength="300" on the input matches the server-side limit. Message alignment logic is straightforward.

.husky/pre-commit (2)

5-6: npx lint-staged usage looks fine

Keeping npx lint-staged here is simple and clear, and matches common Husky setups as long as Node is on PATH.

---

1-3: Based on my research, the review comment is substantially accurate regarding Husky v10 and git hooks:

Verified claims:

1. Shebang is recommended: While Husky v10 executes hooks via sh (potentially wrapping execution), a POSIX-sh compatible shebang (#!/bin/sh) is still a best practice for portability and clarity. This is not strictly required by Husky but is strongly recommended for hooks.
2. PATH export is a no-op: export PATH="$PATH" does nothing functionally and the comment above it is misleading.
3. File must be executable: Husky requires hook files to be executable (chmod +x).
4. POSIX-sh compatibility: The hook code should be POSIX-sh compatible (which npx lint-staged is).

Minor clarification: The review slightly overstates the severity by saying missing shebang causes "exec format error" and hook won't run. With Husky v10, the shebang is less critical because Husky wraps execution. However, adding it is still the right move for robustness and follows industry best practices.

---

Add a proper shebang and clean up the PATH line

The hook should include #!/bin/sh for clarity and portability, even though Husky v10 wraps execution. Also, export PATH="$PATH" is a no-op; the comment above it is misleading.

I suggest:

+#!/bin/sh

-# Husky v10 compatible pre-commit (removed deprecated husky.sh sourcing)
-# Ensure PATH includes typical Node locations if needed (optional on most setups)
-export PATH="$PATH"
+# Husky v10 compatible pre-commit (removed deprecated husky.sh sourcing)
+# (Optional) Uncomment if you need to extend PATH, e.g., for custom bins.
+# export PATH="$PATH:./node_modules/.bin"

Also ensure .husky/pre-commit is executable (chmod +x .husky/pre-commit).

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

src/client/components/LobbyChatPanel.ts (1)

45-55: Async auto-scroll after DOM update looks correct

Using await this.updateComplete before querying .lcp-messages and adjusting scrollTop fixes the earlier race where scroll could happen before the new message rendered. The null-check on container is also enough here.

🧹 Nitpick comments (3)

src/client/components/LobbyChatPanel.ts (3)

20-39: Avoid any casts on event listeners and globals

The lifecycle wiring looks correct and cleanup is handled, but you can tighten types a bit:

• Replace as any on addEventListener / removeEventListener with a typed listener, e.g. use EventListener or change onIncoming to accept Event and cast inside.
• Consider defining a small typed helper for reading (window as any).__eventBus and (window as any).__clientID so these globals are concentrated and strongly typed in one place.

This keeps the component more idiomatic and avoids any holes.

// Example pattern for onIncoming typing:
private onIncoming = async (e: Event) => {
  const { sender, text } = (e as CustomEvent<{ sender: string; text: string }>).detail;
  // ...
};

---

65-84: sendMessage flow and failure handling are in good shape

The flow is nice and simple:

• Trim + early return on empty.
• Lazy attach to global __eventBus.
• Warn and keep the text if the bus is still missing.
• Enforce the 300-char cap before emitting and clear the input only on success.

If you want to reduce duplication later, you could pull the “resolve bus from window.__eventBus” part into a small helper, but not required for this PR.

---

125-138: Guard global style injection for non-DOM environments

The prefixed CSS classes are good for avoiding conflicts, but the top-level document.head.appendChild(style) runs at module load. If this file is ever imported in a non-DOM environment (SSR, some tests), document may be undefined and blow up on import.

You can make this safer with a small guard:

-const style = document.createElement("style");
-style.textContent = `
-...
-`;
-document.head.appendChild(style);
+if (typeof document !== "undefined") {
+  const style = document.createElement("style");
+  style.textContent = `
+  ...
+  `;
+  document.head.appendChild(style);
+}

Functionality stays the same in the browser, but imports become safer elsewhere.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f31a3b0 and 546b608.

📒 Files selected for processing (1)

• src/client/components/LobbyChatPanel.ts (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

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.

📚 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/components/LobbyChatPanel.ts

🧬 Code graph analysis (1)

src/client/components/LobbyChatPanel.ts (2)

src/core/EventBus.ts (1)

• EventBus (7-44)

src/client/Transport.ts (1)

• SendLobbyChatEvent (176-178)

🔇 Additional comments (1)

src/client/components/LobbyChatPanel.ts (1)

1-19: Imports and basic state look solid

Imports match usage, and the ChatMessage shape plus messages / inputText state are clear and minimal. No changes needed here.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (4)

src/client/components/LobbyChatPanel.ts (4)

20-33: Consider improving type safety for window properties.

The repeated (window as any).__eventBus and (window as any).__clientID accesses lose TypeScript's protection. Consider declaring these properties in a global type definition or creating typed accessors.

For example, add to a global.d.ts file:

declare global {
  interface Window {
    __eventBus?: EventBus;
    __clientID?: string;
  }
}

Then use window.__eventBus directly without casting. This provides autocomplete and catches typos at compile time.

---

45-55: Consider adding a message history limit.

The messages array grows unbounded. In a long lobby session, this could accumulate hundreds of messages, impacting memory and render performance.

Apply this diff to keep only the most recent messages:

  private onIncoming = async (
    e: CustomEvent<{ sender: string; text: string }>,
  ) => {
    const { sender, text } = e.detail;
-   this.messages = [...this.messages, { sender, text }];
+   const MAX_MESSAGES = 100;
+   const updated = [...this.messages, { sender, text }];
+   this.messages = updated.slice(-MAX_MESSAGES);
    await this.updateComplete;
    const container = this.renderRoot.querySelector(
      ".lcp-messages",
    ) as HTMLElement | null;
    if (container) container.scrollTop = container.scrollHeight;
  };

This keeps the last 100 messages, which is sufficient for lobby chat and prevents unbounded growth.

---

86-122: Consider adding accessibility attributes.

The chat panel lacks semantic markup and ARIA labels, which impacts screen reader users.

Apply these accessibility improvements:

  render() {
    return html`
      <div class="lcp-container">
-       <div class="lcp-messages">
+       <div class="lcp-messages" role="log" aria-live="polite" aria-label="Lobby chat messages">
          ${this.messages.map((m) => {
            const isSelf =
              this.myClientID !== null && m.sender === this.myClientID;
            const cls = isSelf ? "lcp-msg lcp-right" : "lcp-msg lcp-left";
            return html`<div class="${cls}">
              <span class="lcp-sender">${m.sender}:</span> ${m.text}
            </div>`;
          })}
        </div>
        <div class="lcp-input-row">
          <input
            class="lcp-input"
            type="text"
            maxlength="300"
+           aria-label="Chat message input"
            .value=${this.inputText}
            @input=${(e: Event) =>
              (this.inputText = (e.target as HTMLInputElement).value)}
            @keydown=${(e: KeyboardEvent) => {
              if (e.key === "Enter") {
                e.preventDefault();
                e.stopPropagation();
                this.sendMessage();
              }
            }}
            placeholder=${translateText("lobby_chat.placeholder")}
          />
-         <button class="lcp-send" @click=${() => this.sendMessage()}>
+         <button class="lcp-send" @click=${() => this.sendMessage()} aria-label="Send message">
            ${translateText("lobby_chat.send")}
          </button>
        </div>
      </div>
    `;
  }

This improves usability for assistive technologies.

---

129-142: Optional: Guard against duplicate style injection.

The style is injected at module scope, which is fine for typical builds. However, adding a guard prevents accidental duplication if the module is ever imported dynamically multiple times.

Apply this diff to add a simple guard:

-// Basic Tailwind-like classes, prefixed to avoid global conflicts
-const style = document.createElement("style");
-style.textContent = `
-.lcp-container { display:flex; flex-direction:column; gap:8px; max-height:240px; }
-.lcp-messages { overflow-y:auto; border:1px solid #444; border-radius:8px; padding:8px; height:180px; background:#111; color:#ddd; display:flex; flex-direction:column; gap:6px; }
-.lcp-msg { font-size: 0.9rem; max-width: 80%; padding:6px 10px; border-radius:10px; background:#1b1b1b; }
-.lcp-msg.lcp-left { align-self: flex-start; }
-.lcp-msg.lcp-right { align-self: flex-end; background:#243b55; }
-.lcp-sender { color:#9ae6b4; margin-right:4px; }
-.lcp-input-row { display:flex; gap:8px; }
-.lcp-input { flex:1; border-radius:8px; padding:6px 10px; color:#000; }
-.lcp-send { border-radius:8px; padding:6px 12px; }
-`;
-document.head.appendChild(style);
+// Basic Tailwind-like classes, prefixed to avoid global conflicts
+if (!document.getElementById("lobby-chat-panel-styles")) {
+  const style = document.createElement("style");
+  style.id = "lobby-chat-panel-styles";
+  style.textContent = `
+.lcp-container { display:flex; flex-direction:column; gap:8px; max-height:240px; }
+.lcp-messages { overflow-y:auto; border:1px solid #444; border-radius:8px; padding:8px; height:180px; background:#111; color:#ddd; display:flex; flex-direction:column; gap:6px; }
+.lcp-msg { font-size: 0.9rem; max-width: 80%; padding:6px 10px; border-radius:10px; background:#1b1b1b; }
+.lcp-msg.lcp-left { align-self: flex-start; }
+.lcp-msg.lcp-right { align-self: flex-end; background:#243b55; }
+.lcp-sender { color:#9ae6b4; margin-right:4px; }
+.lcp-input-row { display:flex; gap:8px; }
+.lcp-input { flex:1; border-radius:8px; padding:6px 10px; color:#000; }
+.lcp-send { border-radius:8px; padding:6px 12px; }
+`;
+  document.head.appendChild(style);
+}

This ensures styles are only injected once even in edge cases.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 546b608 and 4894fa4.

📒 Files selected for processing (1)

• src/client/components/LobbyChatPanel.ts (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

📓 Common learnings

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.

📚 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/components/LobbyChatPanel.ts

🔇 Additional comments (7)

src/client/components/LobbyChatPanel.ts (7)

1-10: LGTM! Clean imports and interface.

The imports are minimal and focused. The ChatMessage interface is appropriately simple for internal state management.

---

12-18: LGTM! Component setup follows Lit conventions.

The state management and private property declarations are clean and appropriately typed.

---

35-39: LGTM! Proper cleanup in disconnectedCallback.

The event listener cleanup is symmetric with connectedCallback and prevents memory leaks.

---

41-43: LGTM! Public API for EventBus injection.

This method provides a clean way to explicitly set the EventBus, useful for testing and initialization scenarios.

---

57-63: LGTM! Lazy attachment logic is sound.

The handler correctly re-attempts to capture the global EventBus and client ID when the bus becomes ready, with appropriate fallback for myClientID.

---

65-84: LGTM! Message sending logic is robust.

The method correctly handles:

• Empty input validation
• Lazy EventBus attachment
• Failure detection with user-visible warning
• 300-character limit enforcement matching server validation
• Input clearing only on success

---

124-126: LGTM! Light DOM usage is documented.

The decision to use light DOM for style integration is clearly documented. The lcp- class prefix mitigates naming conflicts.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/client/graphics/layers/StructureIconsLayer.ts (1)

275-276: Consider a helper function to reduce duplication and remove as any casts.

The as any casts on all OutlineFilter usages bypass type checking. If the filter is working at runtime, this is acceptable, but the casts hide potential type mismatches. A small helper function can centralize the cast and make the code cleaner:

// Add near top of file or in a shared utility
function createOutlineFilter(
  thickness: number,
  color: number,
): PIXI.Filter {
  return new OutlineFilter(thickness, color) as unknown as PIXI.Filter;
}

Then replace usages:

-this.ghostUnit.container.filters = [
-  new OutlineFilter(2, 0xff0000) as any,
-];
+this.ghostUnit.container.filters = [createOutlineFilter(2, 0xff0000)];

This keeps the workaround in one place and avoids scattering as any throughout the file.

Also applies to: 293-297, 299-302, 499-500

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0d83cf8 and a806606.

📒 Files selected for processing (1)

• src/client/graphics/layers/StructureIconsLayer.ts (4 hunks)

🧰 Additional context used

🧠 Learnings (1)

📓 Common learnings

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.

[Phantasm0009]

please review

[Lavodan]

What is this used for?

[Phantasm0009]

Its being used to mock the jose library for testing and its configured in the jest.config,test. Its imported by TestServerConfig.ts and it provides mock implementations for JWT functions during tesing.

[coderabbitai]

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9a5628d and 7a28c49.

📒 Files selected for processing (6)

• resources/lang/en.json (1 hunks)
• src/client/ClientGameRunner.ts (2 hunks)
• src/client/HostLobbyModal.ts (5 hunks)
• src/client/components/LobbyChatPanel.ts (1 hunks)
• src/server/GameServer.ts (2 hunks)
• src/server/MapPlaylist.ts (1 hunks)

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

• src/server/MapPlaylist.ts
• src/client/ClientGameRunner.ts
• resources/lang/en.json

🧰 Additional context used

🧠 Learnings (3)

📚 Learning: 2025-08-12T00:31:50.144Z

Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 1752
File: src/core/game/Game.ts:750-752
Timestamp: 2025-08-12T00:31:50.144Z
Learning: In the OpenFrontIO codebase, changes to the PlayerInteraction interface (like adding canDonateGold and canDonateTroops flags) do not require corresponding updates to src/core/Schemas.ts or server serialization code.

Applied to files:

• src/server/GameServer.ts

📚 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/server/GameServer.ts
• src/client/HostLobbyModal.ts
• src/client/components/LobbyChatPanel.ts

📚 Learning: 2025-10-21T20:06:04.823Z

Learnt from: Saphereye
Repo: openfrontio/OpenFrontIO PR: 2233
File: src/client/HostLobbyModal.ts:891-891
Timestamp: 2025-10-21T20:06:04.823Z
Learning: For the HumansVsNations game mode in `src/client/HostLobbyModal.ts` and related files, the implementation strategy is to generate all nations and adjust their strength for balancing, rather than limiting lobby size based on the number of available nations on the map.

Applied to files:

• src/client/HostLobbyModal.ts

🧬 Code graph analysis (3)

src/server/GameServer.ts (2)

src/server/MapPlaylist.ts (1)

• gameConfig (80-107)

src/core/configuration/DefaultConfig.ts (1)

• gameConfig (268-270)

src/client/HostLobbyModal.ts (2)

src/client/LangSelector.ts (1)

• translateText (258-278)

src/client/Utils.ts (1)

• translateText (92-147)

src/client/components/LobbyChatPanel.ts (2)

src/core/EventBus.ts (1)

• EventBus (7-44)

src/client/Transport.ts (1)

• SendLobbyChatEvent (176-178)

🔇 Additional comments (12)

src/server/GameServer.ts (2)

119-124: Server-side enforcement is appropriate.

This defensive check ensures public lobbies cannot enable chat even if a malicious client attempts to send chatEnabled: true. The server must not rely solely on client-side validation.

---

317-323: Add explicit public lobby check.

While chatEnabled is enforced to be false for public lobbies, an explicit check provides clearer intent and defense-in-depth.

Apply this diff:

 case "lobby_chat": {
   if (this.phase() !== GamePhase.Lobby) {
     return;
   }
+  if (this.isPublic()) {
+    return;
+  }
   if (!this.gameConfig.chatEnabled) {
     return;
   }

⛔ Skipped due to learnings

Learnt from: woodydrn
Repo: openfrontio/OpenFrontIO PR: 1836
File: src/client/Main.ts:482-482
Timestamp: 2025-08-17T20:48:49.411Z
Learning: In PR #1836, user woodydrn prefers to keep changes minimal and focused on the specific issue (clientID persistence) rather than refactoring redundant code in JoinLobbyEvent dispatchers. They want to avoid scope creep in focused bug fix PRs.

src/client/HostLobbyModal.ts (5)

28-28: LGTM!

Import follows the project's convention for component imports.

---

55-55: LGTM!

Default false aligns with the server-side default in GameConfig.

---

415-435: LGTM!

The chat toggle implementation correctly updates state and propagates changes to the server. The i18n key usage is appropriate.

---

599-611: LGTM!

Conditional rendering is clean. The component correctly relies on the global EventBus rather than prop drilling.

---

813-813: LGTM!

Including chatEnabled in the game config payload is correct and aligns with the server-side handling.

src/client/components/LobbyChatPanel.ts (5)

20-34: LGTM!

Lifecycle hooks correctly set up event listeners and clean up on disconnect. The lazy EventBus binding with event-bus:ready fallback is appropriate for initialization race conditions.

---

40-50: LGTM!

The handler correctly awaits updateComplete before scrolling, ensuring the new message is rendered before adjusting scroll position.

---

52-57: LGTM!

The fallback handler ensures the component can bind to the EventBus even if it's initialized after the component connects.

---

59-75: LGTM!

The method correctly handles missing EventBus with a warning, enforces the 300-character limit to match server validation, and only clears input after successful send.

---

118-163: LGTM!

Styles use the lcp- prefix to avoid global namespace conflicts, and rgba provides appropriate transparency as requested in past reviews.