Skip to content

Use OpenFront font for version#3795

Merged
evanpelle merged 1 commit intomainfrom
font
Apr 29, 2026
Merged

Use OpenFront font for version#3795
evanpelle merged 1 commit intomainfrom
font

Conversation

@evanpelle
Copy link
Copy Markdown
Collaborator

@evanpelle evanpelle commented Apr 29, 2026

Description:

Screenshot 2026-04-29 at 4 18 23 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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 29, 2026

Walkthrough

Loads and registers the "OpenFront" font at runtime, applies it to game version text with fallbacks, and adds a CSS custom property (--font-display) defining the preferred font stack.

Changes

Cohort / File(s) Summary
OpenFront integration
src/client/Main.ts, src/client/styles.css
Adds runtime font loading using the FontFace API for fonts/OpenFront.ttf, assigns fontFamily (preferring "OpenFront") to version display elements, and defines --font-display CSS custom property for the font stack.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A new font tiptoes into place,
OpenFront smiles across the space,
If it stumbles, fallbacks stay,
Version text dons a brighter day,
Small change, neat finish — graceful trace. 🎨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The PR description is a template with placeholder text and a screenshot, but lacks substantive detail about the font implementation changes to Main.ts and styles.css. Provide a clear description explaining what font is being added (OpenFront), why it's being added, and how it improves the game. Describe the changes to Main.ts and styles.css in the description, not just in code.
✅ Passed checks (4 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title 'Use OpenFront font for version' accurately describes the main change: loading and applying the OpenFront font specifically to version display elements.

✏️ 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.

❤️ Share

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

Copy link
Copy Markdown
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 (2)
src/client/Main.ts (2)

275-275: Don’t swallow font-load errors silently

Line 275 hides failures completely. A small warning log helps diagnose CDN/path issues without changing behavior.

Proposed fix
-    openFrontFont.load().catch(() => {});
+    openFrontFont.load().catch((error) => {
+      console.warn("Failed to load OpenFront font, using fallback fonts.", error);
+    });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/client/Main.ts` at line 275, The openFrontFont.load() call currently
swallows errors with .catch(() => {}); change the catch to log a warning that
includes the error details so font CDN/path failures are visible (e.g., replace
the empty catch on openFrontFont.load() with a handler that calls console.warn
or the project's logger and includes a descriptive message and the caught error
object).

286-286: Use the shared theme font token instead of a hardcoded stack

Line 286 duplicates the stack and drops the Arial fallback defined in CSS. Use the shared --font-display token to keep one source of truth.

Proposed fix
-        (el as HTMLElement).style.fontFamily = '"OpenFront", Inter, sans-serif';
+        (el as HTMLElement).style.fontFamily = "var(--font-display)";
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/client/Main.ts` at line 286, The inline font stack on the element is
hardcoded in Main.ts where (el as HTMLElement).style.fontFamily is set; replace
that hardcoded value with the shared CSS token so it uses the single source of
truth (the --font-display token) and preserves fallbacks like Arial defined in
your global CSS; locate the assignment to style.fontFamily in Main.ts and set it
to use the CSS variable (e.g., via style.fontFamily = 'var(--font-display)') so
the component follows the theme token instead of duplicating the stack.
🤖 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/styles.css`:
- Line 9: The CSS custom property --font-display currently uses unquoted
fallback font names which triggers Stylelint value-keyword-case; update the
value for --font-display to quote the named fonts (e.g., "OpenFront", "Inter",
"Arial") while leaving the generic family unquoted (sans-serif) so the
declaration becomes quoted font names followed by sans-serif; change the value
on the --font-display declaration accordingly.

---

Nitpick comments:
In `@src/client/Main.ts`:
- Line 275: The openFrontFont.load() call currently swallows errors with
.catch(() => {}); change the catch to log a warning that includes the error
details so font CDN/path failures are visible (e.g., replace the empty catch on
openFrontFont.load() with a handler that calls console.warn or the project's
logger and includes a descriptive message and the caught error object).
- Line 286: The inline font stack on the element is hardcoded in Main.ts where
(el as HTMLElement).style.fontFamily is set; replace that hardcoded value with
the shared CSS token so it uses the single source of truth (the --font-display
token) and preserves fallbacks like Arial defined in your global CSS; locate the
assignment to style.fontFamily in Main.ts and set it to use the CSS variable
(e.g., via style.fontFamily = 'var(--font-display)') so the component follows
the theme token instead of duplicating the stack.
🪄 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: 8bb9fde3-70f4-442a-9c81-e1083449a9de

📥 Commits

Reviewing files that changed from the base of the PR and between 67a42c2 and 4b524c4.

⛔ Files ignored due to path filters (1)
  • proprietary/font/OpenFront.ttf is excluded by !**/*.ttf
📒 Files selected for processing (2)
  • src/client/Main.ts
  • src/client/styles.css

Comment thread src/client/styles.css
@theme {
--default-ring-width: 3px;
--default-ring-color: var(--color-malibu-blue);
--font-display: "OpenFront", Inter, Arial, sans-serif;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Fix Stylelint failure on font stack casing

Line 9 currently triggers Stylelint (value-keyword-case). Quote fallback font names to keep intent and pass lint.

Proposed fix
-  --font-display: "OpenFront", Inter, Arial, sans-serif;
+  --font-display: "OpenFront", "Inter", "Arial", sans-serif;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
--font-display: "OpenFront", Inter, Arial, sans-serif;
--font-display: "OpenFront", "Inter", "Arial", sans-serif;
🧰 Tools
🪛 Stylelint (17.9.0)

[error] 9-9: Expected "Inter" to be "inter" (value-keyword-case)

(value-keyword-case)


[error] 9-9: Expected "Arial" to be "arial" (value-keyword-case)

(value-keyword-case)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/client/styles.css` at line 9, The CSS custom property --font-display
currently uses unquoted fallback font names which triggers Stylelint
value-keyword-case; update the value for --font-display to quote the named fonts
(e.g., "OpenFront", "Inter", "Arial") while leaving the generic family unquoted
(sans-serif) so the declaration becomes quoted font names followed by
sans-serif; change the value on the --font-display declaration accordingly.

@github-project-automation github-project-automation Bot moved this from Triage to Development in OpenFront Release Management Apr 29, 2026
Copy link
Copy Markdown
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.

🧹 Nitpick comments (1)
src/client/Main.ts (1)

286-286: Use the shared font token instead of duplicating the stack.

Line 286 hard-codes a stack and drops the Arial fallback defined in CSS. Using var(--font-display) keeps TS and CSS in sync.

Proposed fix
-        (el as HTMLElement).style.fontFamily = '"OpenFront", Inter, sans-serif';
+        (el as HTMLElement).style.fontFamily = "var(--font-display)";
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/client/Main.ts` at line 286, Replace the hard-coded font stack used in
the fontFamily assignment with the shared CSS token so TypeScript and CSS stay
in sync: locate the statement in Main.ts where (el as
HTMLElement).style.fontFamily is set to '"OpenFront", Inter, sans-serif' and
change it to use the CSS variable token (e.g., set fontFamily to
"var(--font-display)") so the fallback (including Arial) defined in CSS is
preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/client/Main.ts`:
- Line 286: Replace the hard-coded font stack used in the fontFamily assignment
with the shared CSS token so TypeScript and CSS stay in sync: locate the
statement in Main.ts where (el as HTMLElement).style.fontFamily is set to
'"OpenFront", Inter, sans-serif' and change it to use the CSS variable token
(e.g., set fontFamily to "var(--font-display)") so the fallback (including
Arial) defined in CSS is preserved.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 37dd2421-41cb-488b-81b1-f27fefe0bc0c

📥 Commits

Reviewing files that changed from the base of the PR and between 4b524c4 and 3ea42bb.

⛔ Files ignored due to path filters (1)
  • proprietary/fonts/OpenFront.ttf is excluded by !**/*.ttf
📒 Files selected for processing (2)
  • src/client/Main.ts
  • src/client/styles.css

@evanpelle evanpelle changed the title font Use OpenFront font for version Apr 29, 2026
@evanpelle evanpelle marked this pull request as ready for review April 29, 2026 22:19
@evanpelle evanpelle requested a review from a team as a code owner April 29, 2026 22:19
@evanpelle evanpelle added this to the v31 milestone Apr 29, 2026
@evanpelle evanpelle merged commit 063704f into main Apr 29, 2026
16 of 27 checks passed
@evanpelle evanpelle deleted the font branch April 29, 2026 22:19
@github-project-automation github-project-automation Bot moved this from Development to Complete in OpenFront Release Management Apr 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

1 participant