Restyle game rank modal #2918
Restyle game rank modal #2918
Conversation
WalkthroughAdded two translation keys for trade types and updated UI styling across modal and ranking components (backdrop opacity, header background, scrollbar utilities, ranking row, controls, and header text localization). Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/ranking/PlayerRow.ts (1)
190-195: Add empty alt text for decorative coin icons.These are decorative, so
alt=""(andaria-hidden="true") avoids noisy screen readers.🛠️ Suggested fix
- <img src="/images/GoldCoinIcon.svg" class="size-3.5 sm:size-5 m-auto" /> + <img src="/images/GoldCoinIcon.svg" alt="" aria-hidden="true" class="size-3.5 sm:size-5 m-auto" />- <img src="/images/GoldCoinIcon.svg" class="w-5 size-3.5 sm:size-5" /> + <img src="/images/GoldCoinIcon.svg" alt="" aria-hidden="true" class="w-5 size-3.5 sm:size-5" />Also applies to: 213-214
🤖 Fix all issues with AI agents
In `@src/client/components/baseComponents/ranking/PlayerRow.ts`:
- Around line 30-36: In PlayerRow's class string, the fixed trailing
"border-white/5" is overriding the conditional border classes (e.g.,
"border-yellow-500" when player.winner is true), and the bg ternary is
redundant; update the class assembly in PlayerRow to remove the trailing border
color and consolidate the background logic (replace class="${player.winner ?
"bg-black/20" : "bg-black/20"} ..." with a single bg class) and ensure the
conditional border classes (using player.winner and visibleBorder) are the final
border-related classes so the winner/transparent styles take effect.
🧹 Nitpick comments (1)
src/client/components/baseComponents/ranking/PlayerRow.ts (1)
209-210: Conflicting width utilities reduce layout clarity.
w-50andw-fullfight each other. Pick one to avoid surprises.🛠️ Suggested cleanup
- <div class="flex gap-2 w-50 justify-between items-center w-full"> + <div class="flex gap-2 justify-between items-center w-full">
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
resources/lang/en.jsonsrc/client/GameInfoModal.tssrc/client/components/baseComponents/Modal.tssrc/client/components/baseComponents/ranking/PlayerRow.tssrc/client/components/baseComponents/ranking/RankingControls.tssrc/client/components/baseComponents/ranking/RankingHeader.ts
🧰 Additional context used
🧠 Learnings (11)
📓 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-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-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-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/GameInfoModal.tssrc/client/components/baseComponents/ranking/PlayerRow.ts
📚 Learning: 2026-01-12T21:37:01.156Z
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2874
File: src/server/MapLandTiles.ts:7-11
Timestamp: 2026-01-12T21:37:01.156Z
Learning: In this repository's OpenFrontIO deployment, inter-service HTTP calls to the master should target http://localhost:3000 (master at port 3000) as the canonical address. Apply this as the standard for all server-side TypeScript code that communicates with the master. Avoid hardcoding non-master URLs; centralize the master address (e.g., via config or env) when possible, and ensure internal service communication uses localhost:3000 in this architecture.
Applied to files:
src/client/GameInfoModal.tssrc/client/components/baseComponents/Modal.tssrc/client/components/baseComponents/ranking/PlayerRow.tssrc/client/components/baseComponents/ranking/RankingControls.tssrc/client/components/baseComponents/ranking/RankingHeader.ts
📚 Learning: 2026-01-13T20:16:05.535Z
Learnt from: FloPinguin
Repo: openfrontio/OpenFrontIO PR: 2887
File: src/core/execution/NukeExecution.ts:118-122
Timestamp: 2026-01-13T20:16:05.535Z
Learning: In code paths that return a Player-like object, prefer returning a union type (Player | TerraNullius) instead of undefined. When a function may fail to find a player, return TerraNullius for the 'not found' case and a Player for valid IDs, and check .isPlayer() (or equivalent) directly on the result instead of guarding with undefined or optional chaining. This should be enforced in Game, GameImpl, and GameView (and similar accessors) to avoid undefined checks and simplify null-safety handling.
Applied to files:
src/client/GameInfoModal.ts
📚 Learning: 2025-06-10T09:56:44.473Z
Learnt from: Ble4Ch
Repo: openfrontio/OpenFrontIO PR: 1063
File: src/core/configuration/PastelThemeDark.ts:53-53
Timestamp: 2025-06-10T09:56:44.473Z
Learning: In ColorAllocator class in src/core/configuration/Colors.ts, the correct method names are assignColor(id: string): Colord for general color assignment and assignTeamColor(team: Team): Colord for team-specific colors. There are no assignPlayerColor() or assignBotColor() methods.
Applied to files:
src/client/components/baseComponents/ranking/PlayerRow.ts
📚 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:
src/client/components/baseComponents/ranking/PlayerRow.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/components/baseComponents/ranking/PlayerRow.ts
📚 Learning: 2025-06-06T15:36:31.739Z
Learnt from: Ble4Ch
Repo: openfrontio/OpenFrontIO PR: 1063
File: src/core/configuration/Colors.ts:114-330
Timestamp: 2025-06-06T15:36:31.739Z
Learning: In the OpenFrontIO project, color improvements are implemented incrementally. The current focus for player colors is ensuring sufficient unique color availability rather than optimizing for visual distinction or accessibility features.
Applied to files:
src/client/components/baseComponents/ranking/PlayerRow.ts
🔇 Additional comments (15)
resources/lang/en.json (1)
227-228: LGTM — new keys fit the header labels.Good to see only en.json updated here; keep other locales for dedicated translation PRs. Based on learnings, please continue separating translation content updates from feature PRs.
src/client/components/baseComponents/Modal.ts (1)
63-67: LGTM — softer backdrop fits the new theme.src/client/GameInfoModal.ts (3)
52-54: LGTM — scrollbar styling is clear and consistent.
113-113: LGTM — header background matches the darker theme.
145-145: LGTM — clean list markup.src/client/components/baseComponents/ranking/RankingControls.ts (2)
60-62: LGTM — main button styling looks consistent.
116-117: LGTM — subbutton styles match the new palette.src/client/components/baseComponents/ranking/RankingHeader.ts (3)
17-17: LGTM — header styling is consistent with the new theme.
63-71: LGTM — localized trade labels are now consistent.
92-94: LGTM — active class toggle stays simple.src/client/components/baseComponents/ranking/PlayerRow.ts (5)
94-95: Score badge styling looks consistent.Nice, clean update to the badge look.
121-123: Multi-score highlight styling looks good.Clearer emphasis between active and inactive values.
224-236: Name/tag styling updates look consistent.The tracking and opacity choices read clean and match the new theme.
107-111: No action required.w-(--width)is a valid, documented Tailwind shorthand for CSS custom properties. This syntax is fully supported and equivalent tow-[var(--width)]. The bar width will apply correctly as written.Likely an incorrect or invalid review comment.
158-165: BigInt → Number cast can lose precision for large trade totals.
gold[...]values arebigint. If totals can exceedNumber.MAX_SAFE_INTEGER, display will be wrong. Please confirm bounds or keep abigintpath (e.g., typed unionnumber | bigintand format accordingly).
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
2 participants
Description:
The game rank modal was still using the old style, which clashes strongly with the new one.
This PR changes changes the modal style to be consistent with the new one:
Old
New
Tagged as
v29to have a consistent style in the same version.Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
IngloriousTom