Skip to content

Conversation

@antigrid
Copy link
Contributor

@antigrid antigrid commented Jan 9, 2026

Description:

Added a clickable toggle to the ControlPanel's troop display, allowing users to switch between absolute troop counts and a percentage of max troops. The preference is persisted in localStorage, and the percentage display uses adaptive decimal precision (1 decimal place for values below 5% or above 95%, and whole numbers otherwise).

SCR-20260109-uacv SCR-20260109-ubgi

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:

webdev.js

@antigrid antigrid requested a review from a team as a code owner January 9, 2026 21:30
@CLAassistant
Copy link

CLAassistant commented Jan 9, 2026

CLA assistant check
All committers have signed the CLA.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 9, 2026

Walkthrough

Adds a toggle button to the ControlPanel troops display so users can switch between absolute counts and percentage-of-max; initializes mode from localStorage, persists changes, and formats percentages with one decimal near 0%/100%.

Changes

Cohort / File(s) Summary
Troops percentage mode toggle
src/client/graphics/layers/ControlPanel.ts
Adds private _troopsPercentageMode, reads localStorage["settings.troopsDisplayMode"] on init, wraps troops display in a toggle button that updates aria-pressed and persists "percentage"/"absolute". Renders percentage of maxTroops with one decimal for ≤5% or ≥95%, otherwise rounded; preserves absolute display and other fields.

Sequence Diagram(s)

sequenceDiagram
  participant Game as GameState
  participant Panel as ControlPanel
  participant Browser as Browser (localStorage)
  participant DOM as UI

  Browser->>Panel: read "settings.troopsDisplayMode" on init
  Game->>Panel: provide `_troops` and `_maxTroops` on render/tick
  Panel->>DOM: render troops button (shows absolute or percentage)
  DOM->>Panel: user clicks toggle
  Panel->>Browser: write "settings.troopsDisplayMode" ("percentage"/"absolute")
  Panel->>DOM: re-render updated display
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Suggested labels

Feature - Frontend

Suggested reviewers

  • evanpelle
  • scottanderson

Poem

A tiny switch on the panel gleams,
Numbers or percent — choose how it seems.
Saved in storage, steady and bright,
Troops now shown in day or night.
A small change, a clearer sight. 🎖️

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding a toggleable troop display mode that switches between absolute and percentage views.
Description check ✅ Passed The description is directly related to the changeset, providing clear details about the feature, UI updates, persistence mechanism, and decimal precision logic implemented in the code.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4a14421 and 991eec5.

📒 Files selected for processing (1)
  • src/client/graphics/layers/ControlPanel.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/client/graphics/layers/ControlPanel.ts

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: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)
src/client/graphics/layers/ControlPanel.ts (2)

45-76: Harden localStorage read (invalid values / storage exceptions).
In some environments, localStorage access can throw; also treat unexpected stored values as default.

Proposed diff
   init() {
     this.attackRatio = Number(
       localStorage.getItem("settings.attackRatio") ?? "0.2",
     );
     this.uiState.attackRatio = this.attackRatio;
-    this._troopsPercentageMode =
-      localStorage.getItem("settings.troopsDisplayMode") === "percentage";
+    try {
+      const v = localStorage.getItem(ControlPanel.TROOPS_DISPLAY_MODE_KEY);
+      this._troopsDisplayMode = v === "percentage" || v === "absolute" ? v : "absolute";
+    } catch {
+      this._troopsDisplayMode = "absolute";
+    }
     this.eventBus.on(AttackRatioEvent, (event) => {

178-205: Use a semantic button, guard _maxTroops, clamp percentage, and add keyboard/aria support.

The current code divides by potentially undefined _maxTroops before the first tick() call, producing NaN%. Additionally, using a <span> with click handler instead of <button> breaks keyboard navigation and screen reader accessibility—always use semantic HTML for interactive elements.

Recommended changes
-              <span
-                class="cursor-pointer"
-                @click=${() => {
-                  this._troopsPercentageMode = !this._troopsPercentageMode;
-                  localStorage.setItem(
-                    "settings.troopsDisplayMode",
-                    this._troopsPercentageMode ? "percentage" : "absolute",
-                  );
-                }}
-                >${this._troopsPercentageMode
-                  ? (() => {
-                      const troopsFillPercent =
-                        (this._troops / this._maxTroops) * 100;
-                      return troopsFillPercent <= 5 || troopsFillPercent >= 95
-                        ? `${troopsFillPercent.toFixed(1)}%`
-                        : `${Math.round(troopsFillPercent)}%`;
-                    })()
-                  : renderTroops(this._troops)}
-                / ${renderTroops(this._maxTroops)}</span
-              >
+              <button
+                type="button"
+                class="cursor-pointer"
+                aria-label=${translateText("control_panel.toggle_troops_display_mode")}
+                @click=${() => {
+                  this._troopsPercentageMode = !this._troopsPercentageMode;
+                  try {
+                    localStorage.setItem(
+                      "settings.troopsDisplayMode",
+                      this._troopsPercentageMode ? "percentage" : "absolute",
+                    );
+                  } catch {
+                    // ignore storage errors
+                  }
+                }}
+              >
+                ${(() => {
+                  const troops = this._troops ?? 0;
+                  const maxTroops = this._maxTroops ?? 0;
+                  const percentRaw = maxTroops > 0 ? (troops / maxTroops) * 100 : 0;
+                  const percent = Number.isFinite(percentRaw)
+                    ? Math.min(100, Math.max(0, percentRaw))
+                    : 0;
+                  const shown = this._troopsPercentageMode
+                    ? percent < 5 || percent > 95
+                      ? `${percent.toFixed(1)}%`
+                      : `${Math.round(percent)}%`
+                    : renderTroops(troops);
+                  return html`${shown} / ${renderTroops(maxTroops)}`;
+                })()}
+              </button>
🧹 Nitpick comments (1)
src/client/graphics/layers/ControlPanel.ts (1)

38-40: Use a typed union for display mode (clearer + matches localStorage values).
A boolean hides meaning and diverges from the persisted "absolute" | "percentage" values.

Proposed diff
 export class ControlPanel extends LitElement implements Layer {
+  private static readonly TROOPS_DISPLAY_MODE_KEY = "settings.troopsDisplayMode" as const;
+  type TroopsDisplayMode = "absolute" | "percentage";

   @state()
-  private _troopsPercentageMode: boolean = false;
+  private _troopsDisplayMode: TroopsDisplayMode = "absolute";
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cf1e67c and f819fae.

📒 Files selected for processing (1)
  • src/client/graphics/layers/ControlPanel.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: For the window close confirmation feature in `ClientGameRunner.ts`, the troop count requirement (>10,000 troops) from issue #2137 was intentionally removed because it was arbitrary and troop count can be reported as low despite having significant land. The confirmation now triggers for any alive player regardless of troop count.
📚 Learning: 2025-10-08T17:14:49.369Z
Learnt from: Foorack
Repo: openfrontio/OpenFrontIO PR: 2141
File: src/client/ClientGameRunner.ts:228-234
Timestamp: 2025-10-08T17:14:49.369Z
Learning: For the window close confirmation feature in `ClientGameRunner.ts`, the troop count requirement (>10,000 troops) from issue #2137 was intentionally removed because it was arbitrary and troop count can be reported as low despite having significant land. The confirmation now triggers for any alive player regardless of troop count.

Applied to files:

  • src/client/graphics/layers/ControlPanel.ts
📚 Learning: 2025-05-31T18:15:03.445Z
Learnt from: 1brucben
Repo: openfrontio/OpenFrontIO PR: 977
File: src/core/execution/AttackExecution.ts:123-125
Timestamp: 2025-05-31T18:15:03.445Z
Learning: The removeTroops function in PlayerImpl.ts already prevents negative troop counts by using minInt(this._troops, toInt(troops)) to ensure it never removes more troops than available.

Applied to files:

  • src/client/graphics/layers/ControlPanel.ts
🧬 Code graph analysis (1)
src/client/graphics/layers/ControlPanel.ts (1)
src/client/Utils.ts (1)
  • renderTroops (15-17)

coderabbitai[bot]
coderabbitai bot previously approved these changes Jan 9, 2026
antigrid added 2 commits January 10, 2026 11:49
Added a clickable toggle to the ControlPanel's troop display, allowing users to switch between absolute troop counts and a percentage of max troops. The preference is persisted in localStorage, and the percentage display uses adaptive decimal precision (1 decimal place for values below 5% or above 95%, and whole numbers otherwise).
@antigrid antigrid force-pushed the feat/control-panel-troops-mode branch from 4a14421 to 991eec5 Compare January 10, 2026 09:49
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