Main Menu UI Overhaul #2829
Main Menu UI Overhaul #2829
Conversation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/client/AccountModal.ts (1)
29-46: Add cleanup for the event listener to prevent memory leaks.The event listener added to
documentin the constructor is never removed. If the component is disconnected and reconnected multiple times, multiple listeners will accumulate, causing memory leaks and potentially stale state updates.🧹 Proposed fix: Add disconnectedCallback to clean up
Store a reference to the bound handler and remove it when the component is disconnected:
+ private boundUserMeHandler: (event: Event) => void; + constructor() { super(); - document.addEventListener("userMeResponse", (event: Event) => { + this.boundUserMeHandler = (event: Event) => { const customEvent = event as CustomEvent; if (customEvent.detail) { this.userMeResponse = customEvent.detail as UserMeResponse; if (this.userMeResponse?.player?.publicId === undefined) { this.statsTree = null; this.recentGames = []; } } else { this.statsTree = null; this.recentGames = []; this.requestUpdate(); } - }); + }; + + document.addEventListener("userMeResponse", this.boundUserMeHandler); } + + disconnectedCallback() { + super.disconnectedCallback(); + document.removeEventListener("userMeResponse", this.boundUserMeHandler); + }
🧹 Nitpick comments (2)
src/client/AccountModal.ts (2)
526-546: Remove redundantrequestUpdate()call.The
requestUpdate()at line 545 is unnecessary since it's already called in both the.then()block (line 538) and the.catch()block (line 543).♻️ Simplify by removing the redundant call
.catch((err) => { console.warn("Failed to fetch user info in AccountModal.open():", err); this.isLoadingUser = false; this.requestUpdate(); }); - this.requestUpdate(); }
238-290: Consider extracting the repeated email-linking UI into a helper method.The email input form with the link button appears in both
renderLinkAccountSection()(lines 270-286) andrenderLoggedInAs()(lines 333-349) with only minor styling differences. Extracting this into a reusable helper would improve maintainability.♻️ Example extraction
private renderEmailLinkForm(compact = false): TemplateResult { return html` <div class="relative group ${compact ? '' : 'w-full'}"> <div class="flex gap-2"> <input type="email" .value="${this.email}" @input="${this.handleEmailInput}" class="${compact ? 'w-full' : 'flex-1 min-w-0'} px-4 py-2 bg-black/40 border border-white/10 rounded-lg text-white text-sm placeholder-white/30 focus:outline-none focus:border-blue-500 transition-all font-medium" placeholder="${translateText( "account_modal.link_via_email_placeholder", )}" /> <button @click="${this.handleSubmit}" class="px-${compact ? '4' : '5'} py-2 text-sm font-bold text-white uppercase bg-blue-600 hover:bg-blue-500 rounded-lg transition-all ${compact ? '' : 'border border-blue-500/50 hover:shadow-[0_0_15px_rgba(37,99,235,0.3)] shrink-0'}" > ${translateText("account_modal.link_button")} </button> </div> </div> `; }Then use
${this.renderEmailLinkForm(false)}and${this.renderEmailLinkForm(true)}in the respective locations.Also applies to: 292-352
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/client/AccountModal.tssrc/client/Navigation.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/client/Navigation.ts
🧰 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/AccountModal.ts
🧬 Code graph analysis (1)
src/client/AccountModal.ts (2)
src/client/Utils.ts (2)
copyToClipboard(19-36)translateText(147-202)src/client/LangSelector.ts (1)
translateText(248-268)
⏰ 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 (5)
src/client/AccountModal.ts (5)
48-57: Clean implementation of copy-to-clipboard with user feedback.The method correctly uses the
copyToClipboardutility and manages theshowCopiedstate for UI feedback. The early return prevents errors when no ID is available.
71-102: Good handling of loading states and rendering modes.The conditional rendering for loading state provides clear feedback, and the inline vs. modal rendering flexibility is well implemented.
376-520: Login flow implementation looks solid.The Discord and email login options are clearly presented, input handling is correct, and appropriate validation is in place. The use of
alert()for feedback is simple but effective for this use case.
354-363: Data loading and navigation methods are well implemented.The
viewGame,handleLogout, andloadPlayerProfilemethods all handle async operations properly with appropriate error handling and state management. The page reload after logout ensures clean state reset.Also applies to: 554-577
548-552: Clean event dispatching in the close handler.The
onClosehook properly dispatches a custom event with correct flags for DOM event propagation.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/client/components/baseComponents/setting/SettingKeybind.ts (1)
126-155: Cancel listening state and ensure event details match the keydown pattern.When the user clicks Reset or Unbind while the button is still listening, the UI stays in the "..." state. Additionally, these methods omit
prevValueand setkeyto incorrect values:
resetToDefaultsendskey: this.defaultKey(a code string), but the keydown handler sendskey: e.key(the actual key)- Both methods lack
prevValue, making the event detail inconsistent with keydown eventsUpdate both methods to:
- Capture
prevValuebefore changing the value- Set
this.listening = falseto clear the listening state- Use
key: ""to indicate a programmatic action (not a user keypress)Suggested fix
private resetToDefault() { + const prevValue = this.value; this.value = this.defaultKey; + this.listening = false; this.dispatchEvent( new CustomEvent("change", { detail: { action: this.action, value: this.defaultKey, - key: this.defaultKey, + key: "", + prevValue, }, bubbles: true, composed: true, }), ); } private unbindKey() { + const prevValue = this.value; this.value = "Null"; + this.listening = false; this.dispatchEvent( new CustomEvent("change", { detail: { action: this.action, value: "Null", key: "", + prevValue, }, bubbles: true, composed: true, }), ); this.requestUpdate(); }
🤖 Fix all issues with AI agents
In @src/client/components/baseComponents/setting/SettingKeybind.ts:
- Around line 21-55: Replace the interactive <div role="button"> in the render()
of SettingKeybind with a real <button type="button"> element (the element that
currently has the big class string and event handlers), remove the role and
manual tabindex attributes, keep the same class list, aria-label,
@click=${this.startListening}, @blur=${this.handleBlur} and the visual state
logic, and retain @keydown=${this.handleKeydown} only if it’s required for
capturing keys while listening; using a real button ensures Enter/Space
activation and prevents accidental form submission (type="button").
🧹 Nitpick comments (2)
src/client/components/baseComponents/setting/SettingKeybind.ts (2)
86-119: Avoid multiple “change” events from key-repeat, and consider a clearer payload contract.Holding a key can fire repeated
keydownevents and spamchange. Also,detail.keyhere ise.key(label-ish), while other paths set it to code-ish values; that’s easy for parent code to mis-handle.Proposed fix (ignore repeats + normalize event detail)
private handleKeydown(e: KeyboardEvent) { if (!this.listening) return; + if (e.repeat) return; + // Allow Tab and Escape to work normally (don't trap focus) if (e.key === "Tab" || e.key === "Escape") { if (e.key === "Escape") { // Cancel listening on Escape this.listening = false; this.requestUpdate(); } return; } // Prevent default only for keys we're actually capturing e.preventDefault(); const code = e.code; const prevValue = this.value; // Temporarily set the value to the new code for validation in parent this.value = code; const event = new CustomEvent("change", { - detail: { action: this.action, value: code, key: e.key, prevValue }, + detail: { action: this.action, value: code, code, key: e.key, prevValue }, bubbles: true, composed: true, }); this.dispatchEvent(event);
21-27: Please confirm"Null"is the intended “unbound” sentinel everywhere.Right now unbind sets
this.value = "Null", anddisplayKey()will render"Null"(viaformatKeyForDisplayfallback). If the intended UX is “Press a key” (or blank) when unbound, consider using""for unbound and keep"Null"only as a serialization detail (if needed).Also applies to: 141-155
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/client/KeybindsModal.tssrc/client/components/baseComponents/setting/SettingKeybind.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/client/KeybindsModal.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
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.
🧬 Code graph analysis (1)
src/client/components/baseComponents/setting/SettingKeybind.ts (2)
src/client/LangSelector.ts (1)
translateText(248-268)src/client/Utils.ts (2)
translateText(147-202)formatKeyForDisplay(79-98)
⏰ 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
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/client/components/baseComponents/setting/SettingKeybind.ts (1)
130-158: Fix event detail consistency: all change events must include the same properties.The event detail structures are inconsistent across methods and violate the parent component's type contract:
handleKeydown: sends{ action, value, key, prevValue }✓resetToDefault: sends{ action, value }✗ Missingkey(required by type)unbindKey: sends{ action, value, key }✓ but missingprevValue(optional)KeybindsModal.ts expects
key: stringin its type definition (line 100), and the handler destructures it at line 104. Although line 125 has a fallback (key || value), this doesn't fix the type violation.Make all three methods include the same detail structure:
{ action, value, key, prevValue }. ForresetToDefault, addkey: this.defaultKey(or empty string) andprevValue: this.valueto match the signature.Regarding the
"Null"string: this is intentional. It's used throughout the codebase (KeybindsModal, HelpModal, InputHandler) as a sentinel value for unbound keys, not as a string representation ofnull.
🧹 Nitpick comments (2)
src/client/components/baseComponents/setting/SettingKeybind.ts (2)
22-24: Consider simplifying the currentValue logic for clarity.The expression
this.value === "" ? "" : this.value || this.defaultKeymixes strict equality with falsy checks. This works but could be clearer.♻️ Optional refactor for explicit intent
- const currentValue = this.value === "" ? "" : this.value || this.defaultKey; + const currentValue = this.value === "" ? "" : (this.value ?? this.defaultKey);Using nullish coalescing (
??) makes it explicit that you're checking for null/undefined rather than all falsy values.
25-27: Extract the rainbow gradient string to improve readability.The 220+ character gradient class string makes the code hard to read and maintain.
♻️ Refactor to extract rainbow class constant
Define the class at the top of the file or in a constant:
+const RAINBOW_GRADIENT_CLASS = + "bg-[linear-gradient(270deg,#990033,#996600,#336600,#008080,#1c3f99,#5e0099,#990033)] " + + "bg-[length:1400%_1400%] animate-rainbow-bg text-white " + + "hover:bg-[linear-gradient(270deg,#990033,#996600,#336600,#008080,#1c3f99,#5e0099,#990033)]"; +Then in render:
- const rainbowClass = this.easter - ? "bg-[linear-gradient(270deg,#990033,#996600,#336600,#008080,#1c3f99,#5e0099,#990033)] bg-[length:1400%_1400%] animate-rainbow-bg text-white hover:bg-[linear-gradient(270deg,#990033,#996600,#336600,#008080,#1c3f99,#5e0099,#990033)]" - : ""; + const rainbowClass = this.easter ? RAINBOW_GRADIENT_CLASS : "";
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/client/components/baseComponents/setting/SettingKeybind.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
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.
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.
🧬 Code graph analysis (1)
src/client/components/baseComponents/setting/SettingKeybind.ts (2)
src/client/LangSelector.ts (1)
translateText(248-268)src/client/Utils.ts (2)
translateText(147-202)formatKeyForDisplay(79-98)
⏰ 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 (5)
src/client/components/baseComponents/setting/SettingKeybind.ts (5)
3-3: Good use of the utility function.Using
formatKeyForDisplayensures consistent key formatting across the application.
29-77: Excellent accessibility improvements.The refactored render includes:
- Proper
role="button"for the keybind controlaria-labelwith translated texttabindex="0"for keyboard navigation- Visual feedback for listening state
These changes make the component much more accessible.
80-83: Clean implementation of key display logic.Using the utility function and translation system keeps the display logic simple and maintainable.
125-128: Good UX addition.Stopping the listening state when focus leaves prevents confusion if the user clicks away.
90-123: Tab and Escape handling for accessibility is good. Allowing Tab to traverse focus normally and Escape to cancel listening prevents focus traps, which matters for keyboard navigation.Parent component (KeybindsModal.ts) correctly handles the updated event detail structure with
prevValueandkeyfields—both are properly destructured and used (key for display formatting, prevValue for value restoration on rejection).
|
Resolves #2652 #2291 (apparently, see #2840 (comment)) |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/client/TerritoryPatternsModal.ts:
- Around line 305-318: In the open method, the current guard uses typeof options
=== "object" which also matches arrays; tighten it by ensuring options is a
non-null plain object (e.g., options !== null && !Array.isArray(options) &&
typeof options === "object") or check for the expected property (e.g.,
'affiliateCode' in options) before reading options. Update the conditional that
assigns this.affiliateCode and this.showOnlyOwned so it only runs when options
is a non-array object or when the expected keys exist, leaving the other
branches unchanged.
- Around line 52-56: The event listener for "pattern-selected" is added inline
in connectedCallback and never removed, causing a leak; refactor by creating a
class property handler (e.g., this._onPatternSelected) that wraps the current
arrow function (calling this.updateFromSettings() and this.refresh()), use
window.addEventListener("pattern-selected", this._onPatternSelected) in
connectedCallback, and implement disconnectedCallback to call
window.removeEventListener("pattern-selected", this._onPatternSelected) to
ensure proper cleanup.
🧹 Nitpick comments (5)
src/client/TerritoryPatternsModal.ts (3)
352-375: Consider extracting the name formatting logic.The manual string formatting (splitting on underscore, capitalizing) at lines 356-364 could be fragile if pattern name formats change. Extract this to a utility function or use translation keys for pattern display names if available.
Example utility function
// In Utils.ts or similar export function formatPatternDisplayName(pattern: PlayerPattern | null): string { if (!pattern || !pattern.name) { return translateText("territory_patterns.pattern.default"); } let name = pattern.name .split("_") .map((w) => w.charAt(0).toUpperCase() + w.slice(1)) .join(" "); if (pattern.colorPalette?.name) { name += ` (${pattern.colorPalette.name})`; } return name; } // Then in TerritoryPatternsModal.ts private showSkinSelectedPopup() { const skinName = formatPatternDisplayName(this.selectedPattern); window.dispatchEvent( new CustomEvent("show-message", { detail: { message: `${skinName} ${translateText("territory_patterns.selected")}`, duration: 2000, }, }), ); }
399-429: Remove redundant operations in refresh.Two minor inefficiencies:
- Line 400 calls
requestUpdate()before any state changes or external rendering, which seems unnecessary.- Lines 408 and 418 both check if
previewButtonis in the DOM, duplicating the check.♻️ Streamlined version
public async refresh() { - this.requestUpdate(); - const preview = this.selectedColor ? this.renderColorPreview(this.selectedColor, 48, 48) : renderPatternPreview(this.selectedPattern ?? null, 48, 48); - if ( - this.previewButton === null || - !document.body.contains(this.previewButton) - ) { + if (this.previewButton === null) { this.previewButton = document.getElementById( "territory-patterns-input-preview-button", ); } - if (this.previewButton === null) return; - // Check if the element is still in the DOM to avoid lit-html errors - if (!document.body.contains(this.previewButton)) { + if (this.previewButton === null || !document.body.contains(this.previewButton)) { console.warn( "TerritoryPatternsModal: previewButton is disconnected from DOM, skipping render", ); return; } // Clear and re-render using Lit render(preview, this.previewButton); this.previewButton.style.padding = "4px"; this.requestUpdate(); }
142-194: Consider extracting helper methods for pattern grid logic.The pattern grid rendering at lines 148-194 mixes several concerns: relationship determination, ownership filtering, and selection state calculation. Extract these into helper methods to improve readability.
Example refactor
private isPatternSelected( pattern: Pattern | null, colorPalette: ColorPalette | null ): boolean { const isDefaultPattern = pattern === null; return ( (isDefaultPattern && this.selectedPattern === null) || (!isDefaultPattern && this.selectedPattern && this.selectedPattern.name === pattern?.name && (this.selectedPattern.colorPalette?.name ?? null) === (colorPalette?.name ?? null)) ); } private shouldShowPatternInGrid( relationship: "owned" | "purchasable" | "blocked" ): boolean { if (relationship === "blocked") return false; if (this.showOnlyOwned) { return relationship === "owned"; } else { // Store mode: hide owned items return relationship !== "owned"; } } private renderPatternGrid(): TemplateResult { const buttons: TemplateResult[] = []; const patterns: (Pattern | null)[] = [ null, ...Object.values(this.cosmetics?.patterns ?? {}), ]; for (const pattern of patterns) { const colorPalettes = pattern ? [...(pattern.colorPalettes ?? []), null] : [null]; for (const colorPalette of colorPalettes) { let rel = "owned"; if (pattern) { rel = patternRelationship( pattern, colorPalette, this.userMeResponse, this.affiliateCode, ); } if (!this.shouldShowPatternInGrid(rel)) continue; const isSelected = this.isPatternSelected(pattern, colorPalette); buttons.push(html` <pattern-button .pattern=${pattern} .colorPalette=${this.cosmetics?.colorPalettes?.[ colorPalette?.name ?? "" ] ?? null} .requiresPurchase=${rel === "purchasable"} .selected=${isSelected} .onSelect=${(p: PlayerPattern | null) => this.selectPattern(p)} .onPurchase=${(p: Pattern, colorPalette: ColorPalette | null) => handlePurchase(p, colorPalette)} ></pattern-button> `); } } return html` <div class="flex flex-col gap-4"> <div class="flex justify-center"> ${hasLinkedAccount(this.userMeResponse) ? this.renderMySkinsButton() : html``} </div> ${!this.showOnlyOwned && buttons.length === 0 ? html`<div class="text-white/40 text-sm font-bold uppercase tracking-wider text-center py-8" > ${translateText("territory_patterns.all_owned")} </div>` : html` <div class="flex flex-wrap gap-4 p-2 justify-center items-stretch content-start" > ${buttons} </div> `} </div> `; }src/client/Main.ts (2)
236-255: Timing hack: 100ms delay is fragile.The
setTimeoutwith 100ms delay aftercustomElements.whenDefined()is a timing-based workaround that could fail if the component renders slowly. The selector also tightly couples to the component's internal structure.Consider having the
flag-inputcomponent dispatch a ready event or expose a public method to register this click handler, which would be more reliable than timing-based rendering assumptions.Alternative approach
In the FlagInput component, dispatch a ready event:
// In flag-input component after render this.dispatchEvent(new CustomEvent('ready', { bubbles: true }));Then in Main.ts:
- // Wait for the flag-input component to be fully ready - customElements.whenDefined("flag-input").then(() => { - // Use a small delay to ensure the component has rendered - setTimeout(() => { - const flagButton = document.querySelector( - "#flag-input-component #flag-input_", - ); - if (!flagButton) { - console.warn("Flag button not found inside component"); - return; - } - flagButton.addEventListener("click", (e) => { - e.preventDefault(); - e.stopPropagation(); - if (flagInputModal && flagInputModal instanceof FlagInputModal) { - flagInputModal.open(); - } - }); - }, 100); + const flagInputComponent = document.querySelector("flag-input"); + flagInputComponent?.addEventListener("ready", () => { + const flagButton = document.querySelector( + "#flag-input-component #flag-input_", + ); + if (!flagButton) { + console.warn("Flag button not found inside component"); + return; + } + flagButton.addEventListener("click", (e) => { + e.preventDefault(); + e.stopPropagation(); + if (flagInputModal && flagInputModal instanceof FlagInputModal) { + flagInputModal.open(); + } + }); });
273-309: Refactor: Move responsive logic to CSS.The responsive button repositioning logic manipulates long Tailwind class strings directly in JavaScript, which is fragile and hard to maintain. The escaped selector
.lg\\:col-span-9.flexis particularly brittle.Consider using CSS media queries or data attributes to control styling instead of JavaScript class manipulation. This would be more maintainable and performant.
CSS-based alternative
Instead of moving the element and changing classes in JS, use CSS to control visibility and positioning:
- // Move button to desktop wrapper on large screens - const desktopWrapper = document.getElementById( - "territory-patterns-preview-desktop-wrapper", - ); - if (desktopWrapper && patternButton) { - const moveButtonBasedOnScreenSize = () => { - if (window.innerWidth >= 1024) { - // Desktop: move to wrapper - if ( - patternButton.parentElement?.id !== - "territory-patterns-preview-desktop-wrapper" - ) { - patternButton.className = - "w-full h-[60px] border border-white/20 bg-white/5 hover:bg-white/10 active:bg-white/20 rounded-lg cursor-pointer focus:outline-none transition-all duration-200 hover:scale-105 overflow-hidden"; - patternButton.style.backgroundSize = "auto 100%"; - patternButton.style.backgroundRepeat = "repeat-x"; - desktopWrapper.appendChild(patternButton); - } - } else { - // Mobile: move back to bar - const mobileParent = document.querySelector(".lg\\:col-span-9.flex"); - if ( - mobileParent && - patternButton.parentElement?.id === - "territory-patterns-preview-desktop-wrapper" - ) { - patternButton.className = - "aspect-square h-[40px] sm:h-[50px] lg:hidden border border-white/20 bg-white/5 hover:bg-white/10 active:bg-white/20 rounded-lg cursor-pointer focus:outline-none transition-all duration-200 hover:scale-105 overflow-hidden shrink-0"; - patternButton.style.backgroundSize = ""; - patternButton.style.backgroundRepeat = ""; - mobileParent.appendChild(patternButton); - } - } - }; - moveButtonBasedOnScreenSize(); - window.addEventListener("resize", moveButtonBasedOnScreenSize); - } + // Apply base responsive classes in HTML instead + if (patternButton) { + patternButton.className = + "territory-pattern-button aspect-square h-[40px] sm:h-[50px] lg:w-full lg:h-[60px] border border-white/20 bg-white/5 hover:bg-white/10 active:bg-white/20 rounded-lg cursor-pointer focus:outline-none transition-all duration-200 hover:scale-105 overflow-hidden shrink-0 lg:shrink"; + }Then in CSS:
@media (min-width: 1024px) { .territory-pattern-button { background-size: auto 100%; background-repeat: repeat-x; } }This eliminates the resize listener, DOM manipulation, and hard-coded class strings.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/client/Main.tssrc/client/Navigation.tssrc/client/TerritoryPatternsModal.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/client/Navigation.ts
🧰 Additional context used
🧠 Learnings (7)
📚 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.tssrc/client/TerritoryPatternsModal.ts
📚 Learning: 2026-01-08T13:52:00.939Z
Learnt from: deshack
Repo: openfrontio/OpenFrontIO PR: 2777
File: src/client/Main.ts:374-378
Timestamp: 2026-01-08T13:52:00.939Z
Learning: In src/client/Main.ts, ensure the browser back/forward navigation ordering is correctly handled: when popstate fires before hashchange during a navigation rollback (e.g., user cancels leaving an active game), use a preventHashUpdate-like flag to suppress the hashchange listener (onHashUpdate) from running in that rollback scenario. This avoids applying an unintended hash update after a rollback and keeps the UI/game state consistent. Document the flag’s usage and the exact conditions under which it is set/reset to prevent subtle regressions.
Applied to files:
src/client/Main.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/Main.tssrc/client/TerritoryPatternsModal.ts
📚 Learning: 2025-06-16T03:03:59.778Z
Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 1192
File: src/client/graphics/layers/RadialMenuElements.ts:312-314
Timestamp: 2025-06-16T03:03:59.778Z
Learning: In RadialMenuElements.ts, when fixing undefined params errors in subMenu functions, use explicit checks like `if (params === undefined || params.selected === null)` rather than optional chaining, as it makes the intent clearer and matches the specific error scenarios where params can be undefined during spawn phase operations.
Applied to files:
src/client/Main.tssrc/client/TerritoryPatternsModal.ts
📚 Learning: 2025-06-20T20:11:00.965Z
Learnt from: devalnor
Repo: openfrontio/OpenFrontIO PR: 1195
File: src/client/graphics/layers/AlertFrame.ts:18-18
Timestamp: 2025-06-20T20:11:00.965Z
Learning: In the OpenFrontIO codebase, UserSettings instances are created directly with `new UserSettings()` in each component that needs them. This pattern is used consistently across at least 12+ files including OptionsMenu, EventsDisplay, DarkModeButton, Main, UserSettingModal, UsernameInput, NameLayer, AlertFrame, UILayer, InputHandler, ClientGameRunner, and GameView. This is the established architectural pattern and should be followed for consistency.
Applied to files:
src/client/Main.ts
📚 Learning: 2026-01-02T18:11:15.132Z
Learnt from: ryanbarlow97
Repo: openfrontio/OpenFrontIO PR: 2740
File: src/client/HostLobbyModal.ts:821-821
Timestamp: 2026-01-02T18:11:15.132Z
Learning: In src/client/HostLobbyModal.ts, the `?s=xxxxx` URL suffix in lobby URLs is purely for cache-busting embed previews on platforms like Discord, WhatsApp, and x.com. The suffix value is ignored by the join logic (any value works), so regenerating it on config changes via `updateUrlWithSuffix()` doesn't break existing shared URLs - it only forces platforms to re-fetch updated preview metadata.
Applied to files:
src/client/Main.ts
📚 Learning: 2025-06-22T05:48:19.241Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 786
File: src/client/TerritoryPatternsModal.ts:337-338
Timestamp: 2025-06-22T05:48:19.241Z
Learning: In src/client/TerritoryPatternsModal.ts, the bit shifting operators (<<) used in coordinate calculations with decoder.getScale() are intentional and should not be changed to multiplication. The user scottanderson confirmed this is functioning as intended.
Applied to files:
src/client/TerritoryPatternsModal.ts
🧬 Code graph analysis (2)
src/client/Main.ts (4)
src/client/TerritoryPatternsModal.ts (1)
onUserMe(66-77)src/core/ApiSchemas.ts (1)
UserMeResponse(78-78)src/client/Layout.ts (1)
initLayout(1-81)src/client/Navigation.ts (1)
initNavigation(1-77)
src/client/TerritoryPatternsModal.ts (4)
src/client/Utils.ts (1)
translateText(147-202)src/client/Api.ts (1)
hasLinkedAccount(137-145)src/core/CosmeticSchemas.ts (1)
Pattern(7-7)src/client/components/PatternButton.ts (2)
renderPatternPreview(161-174)render(70-158)
⏰ 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 (13)
src/client/Main.ts (13)
17-18: LGTM: Clean integration of new modules.The imports for
FlagInputModal,KeybindsModal,Layout, andNavigationare properly structured. TheshowPageextension to theWindowinterface is correctly typed as optional.Also applies to: 26-32, 83-83
132-143: LGTM: Robust version display handling.The refactor to support multiple version display elements using
querySelectorAllis well-implemented. The defensive check and version normalization with "v" prefix are appropriate.
196-210: Good UX improvement: User feedback for validation errors.The integration with
window.showPageand the addition of validation error feedback improves the user experience. The error message properly usesthis.usernameInput?.validationError.
220-227: LGTM: Proper defensive handling.Making the help button optional with appropriate null checks follows good defensive programming practices.
321-332: Minor timing dependency in refresh logic.The 50ms
setTimeoutin theshowPagelistener is a timing-based workaround, likely needed for DOM updates to settle. While functional, consider whether the refresh could be triggered by a more deterministic event if available.
366-398: LGTM: Clean authentication-aware UI state.The matchmaking button logic properly handles logged-in vs logged-out states, validates username input, and provides clear user feedback. The separation of concerns is well-executed.
401-409: LGTM: Improved authentication state detection.The refined check for "logged in" now correctly distinguishes between users with just a
publicIdvs. those with actual authentication (Discord or email). This is more accurate for features requiring real accounts.
460-472: LGTM: Consistent validation and navigation pattern.Both host and join lobby buttons follow the same pattern: validate username, provide feedback, and use
window.showPagefor navigation. The logic is consistent and well-structured.Also applies to: 488-498
511-534: LGTM: Correct implementation of navigation rollback handling.The
preventHashUpdateflag correctly prevents thehashchangelistener from running afterpopstateduring navigation rollback scenarios. This matches the pattern documented in the learnings and prevents subtle UI/game state inconsistencies.Based on learnings, this addresses the browser back/forward navigation ordering issue correctly.
562-563: LGTM: Consistent navigation through showPage.The URL handling correctly uses
window.showPagefor navigation while maintaining the subsequent modal operations. The optional chaining provides appropriate safety.Also applies to: 641-642, 650-650
664-664: LGTM: Clean game state class management.The
in-gameclass is properly added when the game starts and removed when stopping or leaving. The lifecycle management is symmetric and clear.Also applies to: 756-756, 775-775
859-869: LGTM: Robust initialization sequence.The bootstrap function properly sequences initialization: Layout → Client → Navigation. The
readyStatecheck ensures the code runs correctly whether the script loads early or late.
334-345: Remove redundantclassList.remove("hidden")call.The
showPagefunction in Navigation.ts already removes the"hidden"class from the target element. The manualskinStoreModal.classList.remove("hidden")is unnecessary duplication that contradicts the pattern used elsewhere in the codebase (see othershowPagecalls at lines 562 and 641).Delete line 340 to rely on
showPagefor visibility management:Diff
patternButton.addEventListener("click", () => { window.showPage?.("page-item-store"); const skinStoreModal = document.getElementById( "page-item-store", ) as HTMLElement & { open?: (opts: any) => void }; if (skinStoreModal) { - skinStoreModal.classList.remove("hidden"); if (typeof skinStoreModal.open === "function") { skinStoreModal.open({ showOnlyOwned: true }); } } });⛔ Skipped due to 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.
It may fix some issues, I fixed a few but there are still localisation issues, e.g. french keyboard W -> Z and Z -> Q (an issue on main) |
Ok, that may be the thing in this other Issue #1071 (not 100% sure) in which this comment from 2 days ago contains outcome of someone's investigation: #1071 (comment) |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/client/TerritoryPatternsModal.ts (4)
312-333: Consider simplifying the options parsing.The type guards on lines 320-322 include redundant checks. Since TypeScript narrows the type at line 316, the remaining logic can be cleaner.
♻️ Simpler options parsing
public async open( options?: string | { affiliateCode?: string; showOnlyOwned?: boolean }, ) { this.isActive = true; - if (typeof options === "string") { - this.affiliateCode = options; - this.showOnlyOwned = false; - } else if ( - options !== null && - typeof options === "object" && - !Array.isArray(options) - ) { - this.affiliateCode = options.affiliateCode ?? null; - this.showOnlyOwned = options.showOnlyOwned ?? false; - } else { - this.affiliateCode = null; - this.showOnlyOwned = false; - } + if (typeof options === "string") { + this.affiliateCode = options; + this.showOnlyOwned = false; + } else if (options) { + this.affiliateCode = options.affiliateCode ?? null; + this.showOnlyOwned = options.showOnlyOwned ?? false; + } else { + this.affiliateCode = null; + this.showOnlyOwned = false; + } await this.refresh(); super.open(); }
363-386: Nitpick: Display name formatting could be a utility.The pattern name formatting (lines 367-376) transforms snake_case to Title Case. If this formatting is needed elsewhere, consider extracting it to Utils.ts.
Example extraction:
// In Utils.ts export function formatPatternDisplayName(patternName: string): string { return patternName .split("_") .map((w) => w.charAt(0).toUpperCase() + w.slice(1)) .join(" "); }
417-434: Simplify redundant DOM checks.Line 426 checks for
null, but lines 429-434 perform the same check withdocument.body.contains. The firstnullcheck is redundant.♻️ Remove redundant null check
if ( this.previewButton === null || !document.body.contains(this.previewButton) ) { this.previewButton = document.getElementById( "territory-patterns-input-preview-button", ); } - if (this.previewButton === null) return; - // Check if the element is still in the DOM to avoid lit-html errors if (!document.body.contains(this.previewButton)) { console.warn( "TerritoryPatternsModal: previewButton is disconnected from DOM, skipping render", ); return; }Note:
document.body.contains(null)returnsfalse, so the second check handles both cases.
179-186: Consider extracting selection matching logic.The
isSelectedcomputation (lines 180-186) has nested conditions that could be clearer as a helper method. This would improve readability and make the logic testable.Example:
private isPatternSelected( pattern: Pattern | null, colorPalette: ColorPalette | null, ): boolean { if (pattern === null) { return this.selectedPattern === null; } if (!this.selectedPattern) { return false; } return ( this.selectedPattern.name === pattern.name && (this.selectedPattern.colorPalette?.name ?? null) === (colorPalette?.name ?? null) ); }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/client/TerritoryPatternsModal.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
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.
📚 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/TerritoryPatternsModal.ts
📚 Learning: 2025-06-22T05:48:19.241Z
Learnt from: scottanderson
Repo: openfrontio/OpenFrontIO PR: 786
File: src/client/TerritoryPatternsModal.ts:337-338
Timestamp: 2025-06-22T05:48:19.241Z
Learning: In src/client/TerritoryPatternsModal.ts, the bit shifting operators (<<) used in coordinate calculations with decoder.getScale() are intentional and should not be changed to multiplication. The user scottanderson confirmed this is functioning as intended.
Applied to files:
src/client/TerritoryPatternsModal.ts
📚 Learning: 2025-06-16T03:03:59.778Z
Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 1192
File: src/client/graphics/layers/RadialMenuElements.ts:312-314
Timestamp: 2025-06-16T03:03:59.778Z
Learning: In RadialMenuElements.ts, when fixing undefined params errors in subMenu functions, use explicit checks like `if (params === undefined || params.selected === null)` rather than optional chaining, as it makes the intent clearer and matches the specific error scenarios where params can be undefined during spawn phase operations.
Applied to files:
src/client/TerritoryPatternsModal.ts
🧬 Code graph analysis (1)
src/client/TerritoryPatternsModal.ts (4)
src/client/Utils.ts (1)
translateText(147-202)src/client/Api.ts (1)
hasLinkedAccount(137-145)src/core/CosmeticSchemas.ts (1)
Pattern(7-7)src/client/Cosmetics.ts (1)
patternRelationship(51-94)
⏰ 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 (2)
src/client/TerritoryPatternsModal.ts (2)
40-63: LGTM! Clean event listener lifecycle.The arrow function property pattern ensures proper cleanup in
disconnectedCallback, preventing memory leaks.
277-310: LGTM! Clean inline rendering support.The content template is correctly reused for both inline and modal modes, with appropriate flags to hide default modal chrome.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/client/components/PatternButton.ts:
- Around line 74-79: The root div in PatternButton.ts currently only uses visual
cues via this.selected; add an accessible selection state by giving the element
an appropriate role (e.g., role="option") and an aria-selected attribute bound
to the component state (use aria-selected={this.selected} or stringified
'true'/'false') so screen readers announce selection; update the root div that
contains this.selected styling to include role and aria-selected attributes and
ensure the binding reflects changes to this.selected.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
resources/lang/en.jsonsrc/client/components/PatternButton.ts
🧰 Additional context used
🧠 Learnings (6)
📚 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-08-16T10:52:08.292Z
Learnt from: TheGiraffe3
Repo: openfrontio/OpenFrontIO PR: 884
File: resources/lang/en.json:456-461
Timestamp: 2025-08-16T10:52:08.292Z
Learning: In OpenFrontIO, translation files in resources/lang/*.json (except en.json) should not be updated in regular PRs. Only dedicated translation PRs titled "mls" and made by Aotumori should update non-English locale files. Regular PRs should only update en.json when adding or modifying translation keys.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-07-12T08:41:35.101Z
Learnt from: Aotumuri
Repo: openfrontio/OpenFrontIO PR: 1357
File: resources/lang/de.json:523-540
Timestamp: 2025-07-12T08:41:35.101Z
Learning: In OpenFrontIO project localization files, always check the en.json source file before flagging potential spelling errors in other language files, as some keys may intentionally use non-standard spellings that need to be consistent across all translations.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-08-27T08:12:19.610Z
Learnt from: mokizzz
Repo: openfrontio/OpenFrontIO PR: 1940
File: resources/lang/en.json:763-766
Timestamp: 2025-08-27T08:12:19.610Z
Learning: In OpenFrontIO, some country entries in src/client/data/countries.json may have similar names but different codes (e.g., "Empire of Japan" vs "Empire of Japan1"). Each unique code requires its own translation key in resources/lang/en.json after normalization. Always verify against countries.json before suggesting removal of translation keys.
Applied to files:
resources/lang/en.json
📚 Learning: 2025-10-26T15:37:07.732Z
Learnt from: GlacialDrift
Repo: openfrontio/OpenFrontIO PR: 2298
File: src/client/graphics/layers/TerritoryLayer.ts:200-210
Timestamp: 2025-10-26T15:37:07.732Z
Learning: In GameImpl.ts lines 124-139, team assignment logic varies by number of teams: when numPlayerTeams < 8, teams are assigned ColoredTeams values (Red, Blue, Yellow, Green, Purple, Orange, Teal); when numPlayerTeams >= 8, teams are assigned generic string identifiers like "Team 1", "Team 2", etc., which are not members of ColoredTeams.
Applied to files:
resources/lang/en.json
📚 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:
resources/lang/en.json
🧬 Code graph analysis (1)
src/client/components/PatternButton.ts (1)
src/client/LangSelector.ts (1)
translateText(248-268)
⏰ 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 (12)
src/client/components/PatternButton.ts (5)
18-19: LGTM! Clean property declaration.The
selectedproperty follows standard Lit patterns with proper decorator and type annotation.
123-138: LGTM! Clean preview rendering.The aspect-square container with hover effects looks good. Type safety with
satisfies PlayerPatternis a nice touch.
140-155: LGTM! Purchase section with layout stability.The dedicated purchase area with a spacer fallback prevents layout shifts. The fixed height
h-[34px]ensures consistent spacing across all pattern buttons.
176-199: LGTM! Responsive blank preview design.The responsive approach shows a visual grid on mobile and a helpful text hint on desktop. Good UX differentiation based on screen size.
86-121: No action needed. The code gracefully handles missing translation keys via fallback formatting. ThetranslateCosmeticfunction attempts to fetch${prefix}.${name}from translations, but if the key is not found, it formats the name by splitting on underscores, capitalizing each word, and joining with spaces (e.g.,red_patternbecomesRed Pattern). This prevents runtime warnings and is intentional design. The required keyterritory_patterns.pattern.defaultis confirmed to exist in en.json.Likely an incorrect or invalid review comment.
resources/lang/en.json (7)
10-23: LGTM!New common keys (
back,none,copied,click_to_copy) follow the existing snake_case naming pattern and provide clear, reusable UI strings.
32-51: LGTM!Key renames (
create,join,solo) are concise and appropriate for button labels. New navigation keys align with the sidebar menu overhaul.
180-192: LGTM!Account modal keys are well-organized and support the new linking flow. Naming follows project conventions.
342-345: LGTM!Clan tag validation keys (
tag,tag_too_short,tag_invalid_chars) are clear and provide actionable error messages for users.
928-933: LGTM!New
matchmaking_buttonsection provides clear UI strings. Having bothlogin_required(short prompt) andmust_login(detailed message) is appropriate for different UI contexts.
918-927: LGTM!Consistent use of "Solo" (matching
single_modal.title) and addition ofno_statsfor empty state handling. Based on learnings, onlyen.jsonshould be updated in regular PRs, which this PR correctly follows.
382-383: Remove unused translation keyteams_Humans Vs Nations.The code in HostLobbyModal.ts branches to
public_lobby.teams_hvnfor HumansVsNations mode, nothost_modal.teams_Humans Vs Nations. This key is never used and should be deleted.🔧 Remove unused key
"teams_Quads": "Quads (teams of 4)", - "teams_Humans Vs Nations": "Humans vs Nations"⛔ Skipped due to learnings
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.Learnt from: FloPinguin Repo: openfrontio/OpenFrontIO PR: 2689 File: src/client/PublicLobby.ts:245-245 Timestamp: 2025-12-26T22:21:21.904Z Learning: In public lobbies with HumansVsNations mode in src/client/PublicLobby.ts, maxPlayers represents only human player slots (already halved in DefaultConfig.ts). The nation NPCs are added automatically server-side and don't count toward maxPlayers. Therefore, getTeamSize correctly returns maxPlayers directly for HumansVsNations to display the proper team size (e.g., maxPlayers=5 yields "5 Humans vs 5 Nations").Learnt from: mokizzz Repo: openfrontio/OpenFrontIO PR: 1940 File: resources/lang/en.json:763-766 Timestamp: 2025-08-27T08:12:19.610Z Learning: In OpenFrontIO, some country entries in src/client/data/countries.json may have similar names but different codes (e.g., "Empire of Japan" vs "Empire of Japan1"). Each unique code requires its own translation key in resources/lang/en.json after normalization. Always verify against countries.json before suggesting removal of translation keys.Learnt from: ryanbarlow97 Repo: openfrontio/OpenFrontIO PR: 2740 File: src/client/HostLobbyModal.ts:821-821 Timestamp: 2026-01-02T18:11:15.132Z Learning: In src/client/HostLobbyModal.ts, the `?s=xxxxx` URL suffix in lobby URLs is purely for cache-busting embed previews on platforms like Discord, WhatsApp, and x.com. The suffix value is ignored by the join logic (any value works), so regenerating it on config changes via `updateUrlWithSuffix()` doesn't break existing shared URLs - it only forces platforms to re-fetch updated preview metadata.
Description:
Overhauls the Main Menu UI, visit https://menu.openfront.dev to see everything.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
w.o.n