Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
WalkthroughAdded a new Clan UI: a Lit-based Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds a new “Clans” destination to the client navigation and introduces a new clan-modal page to present a clan/browse/manage UI (currently using mock data), along with required English translations.
Changes:
- Added a new “Clans” nav item to both mobile and desktop nav bars that navigates to
page-clan. - Introduced
ClanModal(<clan-modal>) and wired it into the client entrypoint +index.htmlas a new inline page. - Added
main.clansandclan_modal.*translation keys toresources/lang/en.json.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/client/components/MobileNavBar.ts | Adds a new nav button targeting page-clan. |
| src/client/components/DesktopNavBar.ts | Adds a new nav button targeting page-clan. |
| src/client/Main.ts | Imports the new ClanModal so the custom element is registered. |
| src/client/ClanModal.ts | Adds the new clans UI/modal implementation (currently backed by fake data). |
| resources/lang/en.json | Adds “Clans” nav label and clan modal translation keys. |
| index.html | Adds the new inline <clan-modal id="page-clan"> page container. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <button | ||
| @click=${() => (this.manageIsOpen = !this.manageIsOpen)} | ||
| class="relative w-12 h-7 rounded-full transition-all ${this | ||
| .manageIsOpen | ||
| ? "bg-blue-500" | ||
| : "bg-white/20"}" | ||
| > | ||
| <div | ||
| class="absolute top-1 w-5 h-5 rounded-full bg-white shadow transition-all ${this | ||
| .manageIsOpen | ||
| ? "left-6" | ||
| : "left-1"}" | ||
| ></div> | ||
| </button> |
There was a problem hiding this comment.
The open/closed toggle is implemented as a plain <button> with only visual state. For accessibility, expose state via aria-pressed (or role="switch" + aria-checked) and provide an aria-label so screen readers can identify what is being toggled.
|
|
||
| private renderMemberPagination(totalMembers: number) { | ||
| const totalPages = Math.ceil(totalMembers / this.membersPerPage); | ||
| if (totalMembers <= this.perPageOptions[0]) return html``; |
There was a problem hiding this comment.
renderMemberPagination() hides pagination based on totalMembers <= 10, which is unrelated to the current membersPerPage selection and can still render pagination when totalPages === 1 (e.g., 30 members with 50 per page). Use totalPages <= 1 (or totalMembers <= this.membersPerPage) instead.
| if (totalMembers <= this.perPageOptions[0]) return html``; | |
| if (totalPages <= 1) return html``; |
| const FAKE_MEMBER_NAMES = [ | ||
| "GrandMaster42", | ||
| "Lieutenant_X", | ||
| "BattleHawk", | ||
| "NovaStar", | ||
| "IronWill", | ||
| "StormChaser", | ||
| "PhantomBlade", | ||
| "CrimsonTide", | ||
| "FrostNova", | ||
| "ShadowFang", | ||
| "ThunderStrike", | ||
| "VoidWalker", | ||
| "SkyRaider", | ||
| "IronClad", | ||
| "BlazeFury", | ||
| "NightHawk", | ||
| "StormBreaker", | ||
| "DarkMatter", | ||
| "SolarFlare", | ||
| "GhostReaper", | ||
| "WarHound", | ||
| "DeathStroke", | ||
| "IcePhoenix", | ||
| "FireDragon", | ||
| "WolfBane", | ||
| "SteelViper", | ||
| "ThunderBolt", | ||
| "ShadowKnight", | ||
| "CrimsonWolf", | ||
| "BladeRunner", | ||
| "StarFall", | ||
| "VenomStrike", | ||
| "TitanForge", | ||
| "RogueAgent", | ||
| "ViperKing", | ||
| "StormRider", | ||
| "FrostBite", | ||
| "DarkStar", | ||
| "NeonReaper", | ||
| "CyberWolf", | ||
| "BlitzFire", | ||
| "ToxicShark", | ||
| "SilverArrow", | ||
| "RavenClaw", | ||
| "CosmicDust", | ||
| "ArcticFox", | ||
| "PyroKnight", | ||
| ]; |
There was a problem hiding this comment.
This file ships large hardcoded "fake" datasets (names, clans, members, join requests) directly in the production client bundle, which will noticeably increase bundle size and parse time. Consider moving mock data behind a dev-only flag, loading it dynamically, or replacing it with real API-backed data before merging.
| const FAKE_MEMBER_NAMES = [ | |
| "GrandMaster42", | |
| "Lieutenant_X", | |
| "BattleHawk", | |
| "NovaStar", | |
| "IronWill", | |
| "StormChaser", | |
| "PhantomBlade", | |
| "CrimsonTide", | |
| "FrostNova", | |
| "ShadowFang", | |
| "ThunderStrike", | |
| "VoidWalker", | |
| "SkyRaider", | |
| "IronClad", | |
| "BlazeFury", | |
| "NightHawk", | |
| "StormBreaker", | |
| "DarkMatter", | |
| "SolarFlare", | |
| "GhostReaper", | |
| "WarHound", | |
| "DeathStroke", | |
| "IcePhoenix", | |
| "FireDragon", | |
| "WolfBane", | |
| "SteelViper", | |
| "ThunderBolt", | |
| "ShadowKnight", | |
| "CrimsonWolf", | |
| "BladeRunner", | |
| "StarFall", | |
| "VenomStrike", | |
| "TitanForge", | |
| "RogueAgent", | |
| "ViperKing", | |
| "StormRider", | |
| "FrostBite", | |
| "DarkStar", | |
| "NeonReaper", | |
| "CyberWolf", | |
| "BlitzFire", | |
| "ToxicShark", | |
| "SilverArrow", | |
| "RavenClaw", | |
| "CosmicDust", | |
| "ArcticFox", | |
| "PyroKnight", | |
| ]; | |
| const FAKE_MEMBER_NAMES = Array.from({ length: 50 }, (_, index) => `Member${index + 1}`); |
src/client/ClanModal.ts
Outdated
| @customElement("clan-modal") | ||
| export class ClanModal extends BaseModal { | ||
| @state() private activeTab: Tab = "my-clans"; | ||
| @state() private view: View = "list"; | ||
| @state() private selectedClan: FakeClan | null = null; | ||
| @state() private searchQuery = ""; | ||
| @state() private createName = ""; | ||
| @state() private createTag = ""; | ||
| @state() private createDescription = ""; | ||
| @state() private createIsOpen = true; |
There was a problem hiding this comment.
There are existing client-side modal tests (e.g., tests/client/LeaderboardModal.test.ts), but this new modal introduces substantial UI/state logic without any automated coverage. Adding at least a minimal test for key flows (tab switching, selecting a clan, pagination/search state) would help prevent regressions.
| : ""} | ||
| </div> | ||
| <div class="flex items-center gap-4 mt-1 text-xs text-white/40"> | ||
| <span>${clan.members} members</span> |
There was a problem hiding this comment.
This line hardcodes the user-visible word "members" instead of going through the translation system (and it won't pluralize correctly). Use a translated string (ideally ICU pluralization, e.g. "{count, plural, one {...} other {...}}") and pass clan.members as a param.
| <span>${clan.members} members</span> | |
| <span>${translateText("clan_modal.members_count", { count: clan.members })}</span> |
| <div | ||
| @click=${() => (this.transferTarget = m.publicId)} | ||
| class="flex items-center gap-3 py-3 border-b border-white/5 last:border-0 cursor-pointer rounded-lg px-2 transition-all | ||
| ${this.transferTarget === m.publicId | ||
| ? "bg-amber-500/10" | ||
| : "hover:bg-white/5"}" | ||
| > |
There was a problem hiding this comment.
This list item is a clickable <div> without keyboard support. Interactive selection should be a <button> (preferred) or include role, tabindex, and key handlers (Enter/Space) so it’s accessible via keyboard navigation.
src/client/ClanModal.ts
Outdated
| <button | ||
| @click=${() => (this.createIsOpen = !this.createIsOpen)} | ||
| class="relative w-12 h-7 rounded-full transition-all ${this | ||
| .createIsOpen | ||
| ? "bg-blue-500" | ||
| : "bg-white/20"}" | ||
| > | ||
| <div | ||
| class="absolute top-1 w-5 h-5 rounded-full bg-white shadow transition-all ${this | ||
| .createIsOpen | ||
| ? "left-6" | ||
| : "left-1"}" | ||
| ></div> | ||
| </button> |
There was a problem hiding this comment.
The create form's open/closed toggle also needs accessible semantics (e.g., aria-pressed/aria-checked and an aria-label) so screen readers can understand and announce its state.
| const isLeader = myClan?.role === "leader"; | ||
| const isMember = !!myClan; | ||
| const members = FAKE_MEMBERS[clan.id] ?? [ | ||
| { name: this.resolveLeaderName(clan), role: "leader" as const }, |
There was a problem hiding this comment.
members falls back to an object with name (not displayName) and no publicId, but later code assumes FakeMember (uses displayName/publicId in search + rendering). This will break if a clan ID is missing from FAKE_MEMBERS (e.g., newly created clans). Make the fallback conform to FakeMember (include publicId + displayName) or remove the fallback and ensure the data source always returns FakeMember[].
| { name: this.resolveLeaderName(clan), role: "leader" as const }, | |
| { | |
| displayName: this.resolveLeaderName(clan), | |
| publicId: clan.leaderPublicId, | |
| role: "leader" as const, | |
| }, |
| const members = FAKE_MEMBERS[clan.id] ?? [ | ||
| { name: this.resolveLeaderName(clan), role: "leader" as const }, | ||
| ]; |
There was a problem hiding this comment.
Same members fallback issue as in renderClanDetail: the fallback entry doesn't match FakeMember shape (name vs displayName, missing publicId), but downstream code treats it as FakeMember[]. Align the fallback with FakeMember to avoid runtime errors if FAKE_MEMBERS[clan.id] is ever absent.
| class="text-[10px] font-bold uppercase tracking-wider px-2 py-0.5 rounded-full shrink-0 | ||
| ${member.role === "leader" | ||
| ? "bg-amber-500/20 text-amber-400 border border-amber-500/30" | ||
| : member.role === "officer" | ||
| ? "bg-purple-500/20 text-purple-400 border border-purple-500/30" | ||
| : "bg-white/10 text-white/40 border border-white/10"}" | ||
| > | ||
| ${member.role} | ||
| </span> |
There was a problem hiding this comment.
Role labels are rendered directly from internal values (e.g., "leader", "officer", "member"), which makes them non-localizable and couples UI text to internal enums. Map roles to translation keys (e.g. clan_modal.role_leader) and render translated labels instead.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/client/ClanModal.ts`:
- Around line 617-637: The role chips and member count are hard-coded English;
update the UI to use the i18n helper instead: add keys for role labels (e.g.,
"clan.role.leader", "clan.role.member", etc.) and for the members suffix (e.g.,
"clan.members") in resources/lang/en.json, then replace raw role text and the
"${clan.members} members" rendering with translateText(...) calls — ensure the
role rendering logic in the template that checks the role variable still uses
the same classes and only swaps the displayed text via translateText(roleKey)
and use translateText("clan.members", { count: clan.members }) or equivalent
pluralization path; keep resolveLeaderName(clan) unchanged.
- Around line 1099-1149: The candidate rows rendered in the (m) => html`...`
mapping are non-focusable <div>s and must be made keyboard reachable and expose
selected semantics: replace the clickable container with a real focusable
control (e.g., a <button> or an element with tabindex="0") that uses the same
click handler (set this.transferTarget = m.publicId), add keyboard activation
support (handle Enter/Space to trigger selection), and expose selection state
with aria-selected (aria-selected={this.transferTarget === m.publicId}) or
role="option" inside a role="listbox" context; also ensure a visible focus style
is present when focused. Use the existing transferTarget identifier and the
mapping callback where m.publicId and m.role are referenced to locate and update
the element.
- Around line 35-387: This modal currently uses hardcoded test data (FAKE_CLANS,
FAKE_MY_CLANS, FAKE_MEMBERS, FAKE_JOIN_REQUESTS and generateFakeMembers) causing
the clans page to show fake content; change ClanModal so it no longer reads
those constants directly: either gate the component behind a feature flag (check
a feature flag/hook before rendering and return null or a placeholder if
disabled) or replace direct usage with real data hooks/props (e.g., useClans(),
useMyClans(), useClanMembers(clanId), useJoinRequests(clanId) or accept
clans/myClans/members/joinRequests as props) and remove or scope FAKE_* usage to
a local demo-only path. Ensure all references inside ClanModal to FAKE_CLANS,
FAKE_MY_CLANS, FAKE_MEMBERS, FAKE_JOIN_REQUESTS, and generateFakeMembers are
updated to use the chosen real-data hooks/props or are behind the feature-flag
gate before merging.
- Around line 803-846: The clan action buttons render UI but perform no real
work; either wire them to real handlers or disable/hide them until implemented.
Add and call well-named methods (e.g., handleJoinClan, handleRequestInvite,
handleLeaveClan, handleSaveManage, handlePromoteMember, handleDemoteMember,
handleKickMember, handleConfirmTransfer, handleApproveInvite, handleDenyInvite,
handleDisbandClan, handleCreateClan) from the buttons (and where
manageName/manageDescription/manageIsOpen and view = "manage" are set) to
perform the actual state update + API call and then update component state or
dispatch events with results; if backend flow is not ready, replace interactive
buttons with disabled buttons or hide them by rendering nothing or adding
disabled and tooltip explaining "not implemented". Ensure each handler updates
local state (e.g., isMember/isLeader/clan) and emits a custom event (e.g.,
clan-updated) or calls the existing API layer so the UI reflects changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4449223c-dc5b-4004-916c-5682017a45fe
📒 Files selected for processing (6)
index.htmlresources/lang/en.jsonsrc/client/ClanModal.tssrc/client/Main.tssrc/client/components/DesktopNavBar.tssrc/client/components/MobileNavBar.ts
| const FAKE_MEMBER_NAMES = [ | ||
| "GrandMaster42", | ||
| "Lieutenant_X", | ||
| "BattleHawk", | ||
| "NovaStar", | ||
| "IronWill", | ||
| "StormChaser", | ||
| "PhantomBlade", | ||
| "CrimsonTide", | ||
| "FrostNova", | ||
| "ShadowFang", | ||
| "ThunderStrike", | ||
| "VoidWalker", | ||
| "SkyRaider", | ||
| "IronClad", | ||
| "BlazeFury", | ||
| "NightHawk", | ||
| "StormBreaker", | ||
| "DarkMatter", | ||
| "SolarFlare", | ||
| "GhostReaper", | ||
| "WarHound", | ||
| "DeathStroke", | ||
| "IcePhoenix", | ||
| "FireDragon", | ||
| "WolfBane", | ||
| "SteelViper", | ||
| "ThunderBolt", | ||
| "ShadowKnight", | ||
| "CrimsonWolf", | ||
| "BladeRunner", | ||
| "StarFall", | ||
| "VenomStrike", | ||
| "TitanForge", | ||
| "RogueAgent", | ||
| "ViperKing", | ||
| "StormRider", | ||
| "FrostBite", | ||
| "DarkStar", | ||
| "NeonReaper", | ||
| "CyberWolf", | ||
| "BlitzFire", | ||
| "ToxicShark", | ||
| "SilverArrow", | ||
| "RavenClaw", | ||
| "CosmicDust", | ||
| "ArcticFox", | ||
| "PyroKnight", | ||
| ]; | ||
|
|
||
| function generateFakeMembers(count: number, names: string[]): FakeMember[] { | ||
| return names.slice(0, count).map((name, i) => ({ | ||
| publicId: `p_${i.toString(16).padStart(6, "0")}`, | ||
| displayName: name, | ||
| role: (i === 0 | ||
| ? "leader" | ||
| : i < 3 | ||
| ? "officer" | ||
| : "member") as FakeMember["role"], | ||
| })); | ||
| } | ||
|
|
||
| const FAKE_MEMBERS: Record<string, FakeMember[]> = { | ||
| c1: generateFakeMembers(47, FAKE_MEMBER_NAMES), | ||
| c2: generateFakeMembers(32, [ | ||
| "FrostByte", | ||
| "ArcticWind", | ||
| "PolarBear", | ||
| "IceBreaker", | ||
| "SnowDrift", | ||
| "TundraWolf", | ||
| "GlacierKing", | ||
| "Permafrost", | ||
| "BlizzardX", | ||
| "NorthStar", | ||
| "ColdFront", | ||
| "WhiteOut", | ||
| "FreezeFrame", | ||
| "Avalanche", | ||
| "IcePick", | ||
| "WinterSolstice", | ||
| "SubZero", | ||
| "HailStorm", | ||
| "FrostGiant", | ||
| "ChillFactor", | ||
| "SnowBlind", | ||
| "FrozenSolid", | ||
| "IceCap", | ||
| "PolarVortex", | ||
| "FrostFire", | ||
| "ColdSnap", | ||
| "Wintermute", | ||
| "SleetStorm", | ||
| "GlacialPace", | ||
| "IceAge", | ||
| "FrozenThorn", | ||
| "NorthWind", | ||
| ]), | ||
| c3: generateFakeMembers(50, [ | ||
| "DarkSovereign", | ||
| "ShadowLord", | ||
| "NightBlade", | ||
| "VoidPrince", | ||
| "DuskFall", | ||
| "MidnightSun", | ||
| "EclipseKing", | ||
| "PhantomRule", | ||
| "GrimReaper", | ||
| "OnyxKnight", | ||
| "BlackMamba", | ||
| "DarkMatter", | ||
| "SilentDeath", | ||
| "NightCrawler", | ||
| "ShadowStep", | ||
| "Oblivion", | ||
| "DarkVeil", | ||
| "TwilightZone", | ||
| "AbyssWalker", | ||
| "NullVoid", | ||
| "DarkForge", | ||
| "CryptKeeper", | ||
| "DeathMarch", | ||
| "NightShade", | ||
| "Penumbra", | ||
| "UmbraKing", | ||
| "DarkFlame", | ||
| "EternalNight", | ||
| "VoidHunter", | ||
| "ShadowCast", | ||
| "DimReaper", | ||
| "BlackIce", | ||
| "NightFury", | ||
| "DarkPulse", | ||
| "Netherworld", | ||
| "GhostFace", | ||
| "DarkSide", | ||
| "VeilStrike", | ||
| "ShadowFire", | ||
| "Blackout", | ||
| "ChaosVoid", | ||
| "DoomBringer", | ||
| "GrimDark", | ||
| "NightTerror", | ||
| "VoidBorn", | ||
| "OnyxBlade", | ||
| "ShadowHeart", | ||
| "DarkTide", | ||
| "NullField", | ||
| "UmbraSoul", | ||
| ]), | ||
| c4: generateFakeMembers(25, [ | ||
| "SentinelPrime", | ||
| "GuardianAngel", | ||
| "Watchman", | ||
| "Bulwark", | ||
| "Bastion", | ||
| "Rampart", | ||
| "Phalanx", | ||
| "Citadel", | ||
| "Fortify", | ||
| "Aegis", | ||
| "IronGate", | ||
| "StoneWall", | ||
| "ShieldBearer", | ||
| "WardKeeper", | ||
| "HoldFast", | ||
| "LastStand", | ||
| "IronCurtain", | ||
| "DefenseGrid", | ||
| "Safeguard", | ||
| "BunkerDown", | ||
| "CastleKeep", | ||
| "WallBreaker", | ||
| "Garrison", | ||
| "TowerShield", | ||
| "You", | ||
| ]), | ||
| c5: generateFakeMembers(12, [ | ||
| "You", | ||
| "AlphaWolf", | ||
| "BravoSix", | ||
| "CharlieHorse", | ||
| "DeltaForce", | ||
| "EchoStrike", | ||
| "FoxtrotDancer", | ||
| "GolfSwing", | ||
| "HotelCalifornia", | ||
| "IndigoChild", | ||
| "JulietRose", | ||
| "KiloWatt", | ||
| ]), | ||
| c6: [ | ||
| { | ||
| publicId: "p_000000", | ||
| displayName: "IronCommander", | ||
| role: "leader", | ||
| }, | ||
| { | ||
| publicId: "p_000001", | ||
| displayName: "You", | ||
| role: "officer", | ||
| }, | ||
| { | ||
| publicId: "p_000002", | ||
| displayName: "SteelTitan", | ||
| role: "officer", | ||
| }, | ||
| ...[ | ||
| "HeavyMetal", | ||
| "ArmorPlate", | ||
| "TankBuster", | ||
| "WarMachine", | ||
| "BulletProof", | ||
| "Juggernaut", | ||
| "Ironside", | ||
| "ShieldWall", | ||
| "FortressX", | ||
| "ChromeHeart", | ||
| "MetalStorm", | ||
| "CopperKing", | ||
| "BronzeAge", | ||
| "TinSoldier", | ||
| "GoldRush", | ||
| "SilverLining", | ||
| "PlatinumEdge", | ||
| ].map((name, i) => ({ | ||
| publicId: `p_${(i + 3).toString(16).padStart(6, "0")}`, | ||
| displayName: name, | ||
| role: "member" as const, | ||
| })), | ||
| ], | ||
| }; | ||
|
|
||
| const FAKE_JOIN_REQUESTS: Record<string, FakeJoinRequest[]> = { | ||
| c6: [ | ||
| { | ||
| publicId: "p_req_010", | ||
| displayName: "RocketFuel", | ||
| requestedAt: "2026-03-23T10:00:00Z", | ||
| }, | ||
| { | ||
| publicId: "p_req_011", | ||
| displayName: "LaserBeam", | ||
| requestedAt: "2026-03-24T07:45:00Z", | ||
| }, | ||
| { | ||
| publicId: "p_req_012", | ||
| displayName: "TitanFall", | ||
| requestedAt: "2026-03-24T12:30:00Z", | ||
| }, | ||
| ], | ||
| c5: [ | ||
| { | ||
| publicId: "p_req_001", | ||
| displayName: "NeonSamurai", | ||
| requestedAt: "2026-03-22T14:30:00Z", | ||
| }, | ||
| { | ||
| publicId: "p_req_002", | ||
| displayName: "QuantumLeap", | ||
| requestedAt: "2026-03-22T18:15:00Z", | ||
| }, | ||
| { | ||
| publicId: "p_req_003", | ||
| displayName: "ZeroGravity", | ||
| requestedAt: "2026-03-23T09:45:00Z", | ||
| }, | ||
| { | ||
| publicId: "p_req_004", | ||
| displayName: "PixelStorm", | ||
| requestedAt: "2026-03-23T11:20:00Z", | ||
| }, | ||
| { | ||
| publicId: "p_req_005", | ||
| displayName: "TurboCharged", | ||
| requestedAt: "2026-03-23T16:00:00Z", | ||
| }, | ||
| { | ||
| publicId: "p_req_006", | ||
| displayName: "HyperNova", | ||
| requestedAt: "2026-03-24T01:10:00Z", | ||
| }, | ||
| { | ||
| publicId: "p_req_007", | ||
| displayName: "CrypticWolf", | ||
| requestedAt: "2026-03-24T08:30:00Z", | ||
| }, | ||
| ], | ||
| }; | ||
|
|
||
| const FAKE_CLANS: FakeClan[] = [ | ||
| { | ||
| id: "c1", | ||
| name: "Order of the Phoenix", | ||
| tag: "OPX", | ||
| leaderPublicId: "p_000000", | ||
| members: 47, | ||
| isOpen: true, | ||
| description: "Veteran alliance forged in fire. All skill levels welcome.", | ||
| }, | ||
| { | ||
| id: "c2", | ||
| name: "Arctic Wolves", | ||
| tag: "AWF", | ||
| leaderPublicId: "p_000000", | ||
| members: 32, | ||
| isOpen: true, | ||
| description: "Northern hemisphere domination. Active daily players only.", | ||
| }, | ||
| { | ||
| id: "c3", | ||
| name: "Shadow Empire", | ||
| tag: "SHD", | ||
| leaderPublicId: "p_000000", | ||
| members: 50, | ||
| isOpen: false, | ||
| description: "Invite only. Top 100 players.", | ||
| }, | ||
| { | ||
| id: "c4", | ||
| name: "Eternal Guard", | ||
| tag: "ETG", | ||
| leaderPublicId: "p_000000", | ||
| members: 25, | ||
| isOpen: false, | ||
| description: "Elite defensive strategists. Application required.", | ||
| }, | ||
| { | ||
| id: "c5", | ||
| name: "Vanguard Protocol", | ||
| tag: "VGP", | ||
| leaderPublicId: "p_000000", | ||
| members: 12, | ||
| isOpen: false, | ||
| description: "Closed strike team. Recruitment by request only.", | ||
| }, | ||
| { | ||
| id: "c6", | ||
| name: "Iron Legion", | ||
| tag: "IRON", | ||
| leaderPublicId: "p_000000", | ||
| members: 20, | ||
| isOpen: false, | ||
| description: "Heavy hitters only. Coordinated warfare division.", | ||
| }, | ||
| ]; | ||
|
|
||
| const FAKE_MY_CLANS: FakeMyClan[] = [ | ||
| { clan: FAKE_CLANS[0], role: "leader" }, | ||
| { clan: FAKE_CLANS[3], role: "member" }, | ||
| { clan: FAKE_CLANS[4], role: "leader" }, | ||
| { clan: FAKE_CLANS[5], role: "officer" }, | ||
| ]; |
There was a problem hiding this comment.
Don't ship the clans page against FAKE_* data.
This modal reads from FAKE_CLANS, FAKE_MY_CLANS, FAKE_MEMBERS, and FAKE_JOIN_REQUESTS only, so every user will see invented clans, invented membership, and invented requests. With the new nav and page-clan mount already wired in, this lands as a demo screen instead of a working feature. Please gate the page behind a feature flag or hook it up to real clan data before merge.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/client/ClanModal.ts` around lines 35 - 387, This modal currently uses
hardcoded test data (FAKE_CLANS, FAKE_MY_CLANS, FAKE_MEMBERS, FAKE_JOIN_REQUESTS
and generateFakeMembers) causing the clans page to show fake content; change
ClanModal so it no longer reads those constants directly: either gate the
component behind a feature flag (check a feature flag/hook before rendering and
return null or a placeholder if disabled) or replace direct usage with real data
hooks/props (e.g., useClans(), useMyClans(), useClanMembers(clanId),
useJoinRequests(clanId) or accept clans/myClans/members/joinRequests as props)
and remove or scope FAKE_* usage to a local demo-only path. Ensure all
references inside ClanModal to FAKE_CLANS, FAKE_MY_CLANS, FAKE_MEMBERS,
FAKE_JOIN_REQUESTS, and generateFakeMembers are updated to use the chosen
real-data hooks/props or are behind the feature-flag gate before merging.
| ${role | ||
| ? html`<span | ||
| class="text-[10px] font-bold uppercase tracking-wider px-2 py-0.5 rounded-full | ||
| ${role === "leader" | ||
| ? "bg-amber-500/20 text-amber-400 border border-amber-500/30" | ||
| : "bg-blue-500/20 text-blue-400 border border-blue-500/30"}" | ||
| > | ||
| ${role} | ||
| </span>` | ||
| : ""} | ||
| ${!clan.isOpen | ||
| ? html`<span | ||
| class="text-[10px] font-bold uppercase tracking-wider px-2 py-0.5 rounded-full bg-red-500/20 text-red-400 border border-red-500/30" | ||
| > | ||
| ${translateText("clan_modal.invite_only")} | ||
| </span>` | ||
| : ""} | ||
| </div> | ||
| <div class="flex items-center gap-4 mt-1 text-xs text-white/40"> | ||
| <span>${clan.members} members</span> | ||
| <span>${this.resolveLeaderName(clan)}</span> |
There was a problem hiding this comment.
Some clan labels still bypass i18n.
The role chips and the "members" suffix are rendered as raw English strings instead of translateText(...), so this page will be only partly translated. Please move these labels into resources/lang/en.json and render them through the existing i18n path.
Also applies to: 1031-1040, 1398-1406
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/client/ClanModal.ts` around lines 617 - 637, The role chips and member
count are hard-coded English; update the UI to use the i18n helper instead: add
keys for role labels (e.g., "clan.role.leader", "clan.role.member", etc.) and
for the members suffix (e.g., "clan.members") in resources/lang/en.json, then
replace raw role text and the "${clan.members} members" rendering with
translateText(...) calls — ensure the role rendering logic in the template that
checks the role variable still uses the same classes and only swaps the
displayed text via translateText(roleKey) and use translateText("clan.members",
{ count: clan.members }) or equivalent pluralization path; keep
resolveLeaderName(clan) unchanged.
| (m) => html` | ||
| <div | ||
| @click=${() => (this.transferTarget = m.publicId)} | ||
| class="flex items-center gap-3 py-3 border-b border-white/5 last:border-0 cursor-pointer rounded-lg px-2 transition-all | ||
| ${this.transferTarget === m.publicId | ||
| ? "bg-amber-500/10" | ||
| : "hover:bg-white/5"}" | ||
| > | ||
| <div | ||
| class="w-8 h-8 rounded-full bg-white/10 flex items-center justify-center text-white/50 text-xs font-bold shrink-0" | ||
| > | ||
| ${m.displayName.charAt(0)} | ||
| </div> | ||
| <div class="flex-1 min-w-0"> | ||
| <span | ||
| class="text-white text-sm font-medium truncate block" | ||
| >${m.displayName}</span | ||
| > | ||
| <copy-button | ||
| compact | ||
| .copyText=${m.publicId} | ||
| .displayText=${m.publicId} | ||
| .showVisibilityToggle=${false} | ||
| .showCopyIcon=${false} | ||
| ></copy-button> | ||
| </div> | ||
| <span | ||
| class="text-[10px] font-bold uppercase tracking-wider px-2 py-0.5 rounded-full shrink-0 | ||
| ${m.role === "officer" | ||
| ? "bg-purple-500/20 text-purple-400 border border-purple-500/30" | ||
| : "bg-white/10 text-white/40 border border-white/10"}" | ||
| > | ||
| ${m.role} | ||
| </span> | ||
| ${this.transferTarget === m.publicId | ||
| ? html`<svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| class="w-5 h-5 text-amber-400 shrink-0" | ||
| fill="none" | ||
| viewBox="0 0 24 24" | ||
| stroke="currentColor" | ||
| stroke-width="2" | ||
| > | ||
| <path | ||
| stroke-linecap="round" | ||
| stroke-linejoin="round" | ||
| d="M5 13l4 4L19 7" | ||
| /> | ||
| </svg>` | ||
| : ""} | ||
| </div> |
There was a problem hiding this comment.
Make leader selection keyboard reachable.
The transfer candidates are clickable div rows with no focus or selected semantics, so a keyboard-only user cannot choose a new leader here. Use a real focusable control for each candidate and expose the selected state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/client/ClanModal.ts` around lines 1099 - 1149, The candidate rows
rendered in the (m) => html`...` mapping are non-focusable <div>s and must be
made keyboard reachable and expose selected semantics: replace the clickable
container with a real focusable control (e.g., a <button> or an element with
tabindex="0") that uses the same click handler (set this.transferTarget =
m.publicId), add keyboard activation support (handle Enter/Space to trigger
selection), and expose selection state with aria-selected
(aria-selected={this.transferTarget === m.publicId}) or role="option" inside a
role="listbox" context; also ensure a visible focus style is present when
focused. Use the existing transferTarget identifier and the mapping callback
where m.publicId and m.role are referenced to locate and update the element.
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
If this PR fixes an issue, link it below. If not, delete these two lines.
Resolves #(issue number)
Description:
Describe the PR.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
DISCORD_USERNAME