Skip to content

Conversation

@evanpelle
Copy link
Collaborator

@evanpelle evanpelle commented Oct 30, 2025

Description:

  • Create CREDITS.md
  • add all link to it on loading page
Screenshot 2025-10-30 at 6 24 07 PM

Please complete the following:

  • I have added screenshots for all UI updates
  • I process any text displayed to the user through translateText() and I've added it to the en.json file
  • I have added relevant tests to the test directory
  • I confirm I have thoroughly tested these changes and take full responsibility for any bugs introduced

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

evan

@evanpelle evanpelle requested a review from a team as a code owner October 30, 2025 23:40
@evanpelle evanpelle changed the base branch from main to v26 October 30, 2025 23:41
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 30, 2025

Walkthrough

GameStartingModal styling and content were updated: paragraph and link CSS rules added; loading/title display adjusted; Credits link added and rendered before code_license. A new CREDITS.md file was introduced documenting code, map data, and icon attributions. resources/lang/en.json replaced desc with a credits key.

Changes

Cohort / File(s) Summary
GameStartingModal styling and rendering updates
src/client/GameStartingModal.ts
Added CSS rules for .modal p, .modal .loading, .copyright, and .modal a (spacing, font sizes, hover effects). Reordered rendered content: added a Credits link to CREDITS.md, retained code_license paragraph, and replaced the previous title display with a loading-styled paragraph.
Credits documentation
CREDITS.md
New file documenting licenses and sources for code, map data (OpenStreetMap, Natural Earth, Bedmap3 Antarctica Dataset) and icons. No runtime logic changes.
Localization update
resources/lang/en.json
Removed game_starting_modal.desc and added game_starting_modal.credits; code_license remains. Adjusted translation keys to match modal rendering changes.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Check CSS specificity and interactions with existing modal styles.
  • Verify the Credits link href/relative path and localization key usage.
  • Confirm resources/lang/en.json key rename doesn't break other references.

Possibly related PRs

Poem

A modal trimmed and given light,
A Credits link to set things right,
Small styles hum and text aligns,
New file thanks the source designs,
The UI waits — now polished, bright.

Pre-merge checks

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title "add credits link to starting modal" directly and accurately describes the primary user-facing change in this pull request. The changeset adds a new credits link to the GameStartingModal component that points to a new CREDITS.md file, with corresponding styling and internationalization updates. The title is concise, uses clear language without unnecessary noise, and a teammate reviewing the git history would immediately understand that this PR introduces credits functionality to the starting modal.
Description Check ✅ Passed The pull request description is related to and describes the changeset. It explicitly mentions creating CREDITS.md and adding a link to it on the loading page, which matches the actual changes in the PR. The included screenshot provides visual evidence of the UI update, and the completed checklist items demonstrate the author followed project conventions including adding translations to en.json and running tests. The description is brief but sufficiently specific about what was changed.

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b3ff553 and da73573.

📒 Files selected for processing (3)
  • CREDITS.md (1 hunks)
  • resources/lang/en.json (1 hunks)
  • src/client/GameStartingModal.ts (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • CREDITS.md
🧰 Additional context used
🧠 Learnings (6)
📚 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/GameStartingModal.ts
📚 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:

  • src/client/GameStartingModal.ts
📚 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:

  • src/client/GameStartingModal.ts
📚 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:

  • src/client/GameStartingModal.ts
📚 Learning: 2025-07-12T06:35:10.457Z
Learnt from: Aotumuri
Repo: openfrontio/OpenFrontIO PR: 1357
File: resources/lang/ja.json:0-0
Timestamp: 2025-07-12T06:35:10.457Z
Learning: In OpenFrontIO project, "giantworldmap" is the correct localization key name for the giant world map, used consistently across all language files and TypeScript code. Do not suggest renaming this key.

Applied to files:

  • src/client/GameStartingModal.ts
📚 Learning: 2025-08-21T21:42:45.277Z
Learnt from: plazmaezio
Repo: openfrontio/OpenFrontIO PR: 1883
File: src/client/HostLobbyModal.ts:227-245
Timestamp: 2025-08-21T21:42:45.277Z
Learning: In openfrontio/OpenFrontIO, the en.json file contains difficulty translation keys using the descriptive names ("Relaxed", "Balanced", "Intense", "Impossible") rather than the enum values ("Easy", "Medium", "Hard", "Impossible"). The DifficultyDescription mapping in src/client/components/Difficulties.ts correctly maps enum values to these descriptive translation keys.

Applied to files:

  • resources/lang/en.json
🔇 Additional comments (4)
src/client/GameStartingModal.ts (3)

53-65: LGTM! Clean styling improvements.

The paragraph and loading text styles improve readability and visual hierarchy in the modal.


102-115: LGTM! Good link styling with proper hover states.

The link styles are clean and provide clear visual feedback. Color transition on hover is smooth.


122-129: LGTM! Proper internationalization and security attributes.

All user-facing text correctly uses translateText(), and the link includes rel="noopener noreferrer" for security. The Credits link provides clear attribution.

resources/lang/en.json (1)

265-266: No issues found—removal of "desc" key is safe.

Verification confirms no code references game_starting_modal.desc anywhere. The key removal does not break anything. The "credits" key addition is a clean new feature.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (3)
src/client/GameStartingModal.ts (3)

95-100: Consider reducing the copyright font size.

A 32px font in a 300px-wide modal means the copyright text takes up significant vertical and visual space. Consider reducing to 20-24px for better proportion.

    .copyright {
-      font-size: 32px;
+      font-size: 24px;
      margin-top: 20px;
      margin-bottom: 10px;
      opacity: 1;
    }

122-127: Consider adding an aria-label for better accessibility.

The link properly includes rel="noopener noreferrer" for security. However, screen reader users would benefit from knowing this opens GitHub in a new tab.

        <a
          href="https://github.com/openfrontio/OpenFrontIO/graphs/contributors"
          target="_blank"
          rel="noopener noreferrer"
+          aria-label="View contributors on GitHub (opens in new tab)"
          >Contributors</a
        >

128-131: Optional: Consider using semantic HTML for attributions.

The attribution list could use more semantic structure for better accessibility and maintainability.

-        <p>${translateText("game_starting_modal.code_license")}</p>
-        <p>Icons from The Noun Project</p>
-        <p>© OpenStreetMap contributors</p>
+        <ul class="attributions">
+          <li>${translateText("game_starting_modal.code_license")}</li>
+          <li>${translateText("game_starting_modal.icons_attribution")}</li>
+          <li>${translateText("game_starting_modal.osm_attribution")}</li>
+        </ul>
         <p class="loading">${translateText("game_starting_modal.title")}</p>

Add corresponding CSS:

.modal .attributions {
  list-style: none;
  padding: 0;
  margin: 0;
}

.modal .attributions li {
  margin: 2px 0;
  font-size: 14px;
}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4f00c4a and 36af54b.

📒 Files selected for processing (1)
  • src/client/GameStartingModal.ts (2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.
📚 Learning: 2025-06-09T02:20:43.637Z
Learnt from: VariableVince
Repo: openfrontio/OpenFrontIO PR: 1110
File: src/client/Main.ts:293-295
Timestamp: 2025-06-09T02:20:43.637Z
Learning: In src/client/Main.ts, during game start in the handleJoinLobby callback, UI elements are hidden using direct DOM manipulation with classList.add("hidden") for consistency. This includes modals, buttons, and error divs. The codebase follows this pattern rather than using component APIs for hiding elements during game transitions.

Applied to files:

  • src/client/GameStartingModal.ts
🔇 Additional comments (3)
src/client/GameStartingModal.ts (3)

53-56: LGTM: Paragraph styling is appropriate.

The tight margins and smaller font size work well for the attribution content.


58-65: LGTM: Loading text styling is clear.

The increased font size and spacing properly distinguish the loading message from the attribution content.


102-115: LGTM: Link styling is interactive and accessible.

The color contrast, hover states, and transitions provide good user experience.

@evanpelle evanpelle force-pushed the evan-starting-modal branch from 36af54b to b3ff553 Compare October 31, 2025 01:25
@evanpelle evanpelle changed the title add attributions to starting modal add credits link to starting modal Oct 31, 2025
@evanpelle evanpelle force-pushed the evan-starting-modal branch from b3ff553 to f00c6d2 Compare October 31, 2025 01:28
@evanpelle evanpelle force-pushed the evan-starting-modal branch from f00c6d2 to da73573 Compare October 31, 2025 01:29
@evanpelle evanpelle added this to the v26 milestone Oct 31, 2025
@evanpelle evanpelle merged commit d97184a into v26 Oct 31, 2025
13 of 22 checks passed
@evanpelle evanpelle deleted the evan-starting-modal branch October 31, 2025 02:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants