feat(ui): add mini-hover overlay with in-game toggle#3371
feat(ui): add mini-hover overlay with in-game toggle#3371hkio120 wants to merge 2 commits intoopenfrontio:mainfrom
Conversation
WalkthroughAdds a Mini Hover Overlay toggle persisted in localStorage and applied via a body class; GameRenderer now assigns a uiState to PlayerInfoOverlay; PlayerInfoOverlay gains hover tracking and a desktop-only compact hover overlay rendering troop counts, percentages, and attack previews when enabled. Changes
Sequence DiagramsequenceDiagram
participant User
participant HTML
participant Renderer
participant Overlay
participant DOM
User->>HTML: Click Mini Hover Toggle
HTML->>HTML: Update localStorage (settings.miniHoverOverlay)
HTML->>DOM: Apply/remove mini-hover-overlay-disabled class
HTML->>HTML: Update toggle UI text
Renderer->>Renderer: Initialize uiState
Renderer->>Overlay: Assign uiState property
User->>DOM: Move mouse over unit/player
DOM->>Overlay: Mousemove / hover events
Overlay->>Overlay: Update hoverScreenX/Y
Note over Overlay: Check: desktop viewport, overlay enabled, not own unit
alt Conditions met
Overlay->>Overlay: Compute attack preview & troop percentage
Overlay->>DOM: renderCompactHoverPlayerInfo (troop counts, % , preview)
else Conditions not met
Overlay->>DOM: No compact overlay rendered
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ 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 |
|
can you add screenshots? |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@index.html`:
- Around line 317-321: The Mini Hover button text is hardcoded; replace it with
the i18n lookup used elsewhere and do the same inside applyMiniHoverOverlay() so
both "On" and "Off" states are localized via the same keys. Specifically, update
the element with id "mini-hover-toggle-button" to set its label by calling the
project's i18n function (use the existing i18n path/key pattern, e.g.,
t('miniHover.on')/t('miniHover.off') or the equivalent used in the codebase)
instead of raw strings, and modify applyMiniHoverOverlay() to set the button
text from those same i18n keys when toggling. Ensure you add/choose consistent
keys for both states and remove duplicated raw text in both places.
In `@src/client/graphics/layers/PlayerInfoOverlay.ts`:
- Around line 515-520: The preview multiplies troops by uiState.attackRatio as
if it's fractional but attackRatio is seeded as 20 (percent); in
predictedOutgoingAttackTroops() normalize attackRatio before use (e.g., divide
by 100 or convert values >1 to a fraction), clamp to 0..1, then compute
Math.floor(myPlayer.troops() * normalizedRatio) to return the correct
percent-based preview; update references to this.uiState?.attackRatio and ensure
the final Math.max/Math.floor logic remains.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e03744f7-8086-434e-a301-e0a9404f64ea
📒 Files selected for processing (3)
index.htmlsrc/client/graphics/GameRenderer.tssrc/client/graphics/layers/PlayerInfoOverlay.ts
index.html
Outdated
| <button | ||
| id="mini-hover-toggle-button" | ||
| class="px-3 py-1.5 rounded-md bg-gray-900/80 border border-white/20 text-white text-xs lg:text-sm hover:bg-gray-800/90 transition-colors" | ||
| > | ||
| Mini Hover: On |
There was a problem hiding this comment.
Localize the Mini Hover label.
The button text is hardcoded here and again in applyMiniHoverOverlay(), so it will stay English when the rest of the UI is translated. Please route both states through the existing i18n path instead of assigning raw strings.
Also applies to: 353-357
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@index.html` around lines 317 - 321, The Mini Hover button text is hardcoded;
replace it with the i18n lookup used elsewhere and do the same inside
applyMiniHoverOverlay() so both "On" and "Off" states are localized via the same
keys. Specifically, update the element with id "mini-hover-toggle-button" to set
its label by calling the project's i18n function (use the existing i18n path/key
pattern, e.g., t('miniHover.on')/t('miniHover.off') or the equivalent used in
the codebase) instead of raw strings, and modify applyMiniHoverOverlay() to set
the button text from those same i18n keys when toggling. Ensure you add/choose
consistent keys for both states and remove duplicated raw text in both places.
| private predictedOutgoingAttackTroops(): number { | ||
| const myPlayer = this.game.myPlayer(); | ||
| if (!myPlayer) return 0; | ||
| const ratio = this.uiState?.attackRatio ?? 0; | ||
| return Math.max(0, Math.floor(myPlayer.troops() * ratio)); | ||
| } |
There was a problem hiding this comment.
Normalize attackRatio before calculating preview troops.
uiState.attackRatio is seeded as 20, so this currently predicts myPlayer.troops() * 20 instead of 20% of the army. That makes the new blue-sword preview wildly wrong.
Proposed fix
private predictedOutgoingAttackTroops(): number {
const myPlayer = this.game.myPlayer();
if (!myPlayer) return 0;
- const ratio = this.uiState?.attackRatio ?? 0;
+ const ratio = (this.uiState?.attackRatio ?? 0) / 100;
return Math.max(0, Math.floor(myPlayer.troops() * ratio));
}📝 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.
| private predictedOutgoingAttackTroops(): number { | |
| const myPlayer = this.game.myPlayer(); | |
| if (!myPlayer) return 0; | |
| const ratio = this.uiState?.attackRatio ?? 0; | |
| return Math.max(0, Math.floor(myPlayer.troops() * ratio)); | |
| } | |
| private predictedOutgoingAttackTroops(): number { | |
| const myPlayer = this.game.myPlayer(); | |
| if (!myPlayer) return 0; | |
| const ratio = (this.uiState?.attackRatio ?? 0) / 100; | |
| return Math.max(0, Math.floor(myPlayer.troops() * ratio)); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/client/graphics/layers/PlayerInfoOverlay.ts` around lines 515 - 520, The
preview multiplies troops by uiState.attackRatio as if it's fractional but
attackRatio is seeded as 20 (percent); in predictedOutgoingAttackTroops()
normalize attackRatio before use (e.g., divide by 100 or convert values >1 to a
fraction), clamp to 0..1, then compute Math.floor(myPlayer.troops() *
normalizedRatio) to return the correct percent-based preview; update references
to this.uiState?.attackRatio and ensure the final Math.max/Math.floor logic
remains.
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
|
the option to enable/disable should be in the options menu |
can you move the screenshots to the PR description |
Yeah done 
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/core/game/UserSettings.ts (1)
60-65: Consider using a single storage format forsettings.miniHoverOverlayto simplify maintenance.Currently,
miniHoverOverlay()reads legacy"enabled"/"disabled"values andtoggleMiniHoverOverlay()writes"true"/"false". Whileindex.htmlhandles both formats correctly at startup, this split across read/write creates maintenance risk. When this setting is touched again, the logic becomes harder to follow.Recommendation: either migrate all legacy values to the new format during the next read, or standardize on one encoding before making further changes to this setting.
Also applies to: 126-128
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/game/UserSettings.ts` around lines 60 - 65, The miniHoverOverlay getter currently reads legacy "enabled"/"disabled" strings while toggleMiniHoverOverlay writes "true"/"false"; update the getter to normalize and persist the new format on read so storage uses a single encoding: in miniHoverOverlay() detect legacy "enabled"/"disabled" values, convert them to the canonical boolean/string form you use (e.g., "true"/"false" or boolean true/false), persist the normalized value back to localStorage, and then return the boolean; also ensure toggleMiniHoverOverlay() (and the related code referenced around lines 126-128) writes the same canonical format so both read and write paths are consistent.
🤖 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/graphics/layers/SettingsModal.ts`:
- Around line 516-540: The mini-hover overlay toggle (button using
onToggleMiniHoverOverlayButtonClick and reading userSettings.miniHoverOverlay())
is desktop-only but is rendered on touch devices; modify SettingsModal to detect
hover-capable devices and either hide the entire button or render it disabled
with a short hint on non-hover devices. Implement a small helper (e.g.
isHoverCapable or isTouchDevice) in the SettingsModal component that uses
navigator / matchMedia (hover: none) to determine capability, and wrap the
existing button render in that check (or set disabled/aria-disabled plus a
desktop-only hint text) so mobile/tablet users won't see an active no-op toggle.
Ensure you reference onToggleMiniHoverOverlayButtonClick and
userSettings.miniHoverOverlay() when updating the rendering logic.
---
Nitpick comments:
In `@src/core/game/UserSettings.ts`:
- Around line 60-65: The miniHoverOverlay getter currently reads legacy
"enabled"/"disabled" strings while toggleMiniHoverOverlay writes "true"/"false";
update the getter to normalize and persist the new format on read so storage
uses a single encoding: in miniHoverOverlay() detect legacy "enabled"/"disabled"
values, convert them to the canonical boolean/string form you use (e.g.,
"true"/"false" or boolean true/false), persist the normalized value back to
localStorage, and then return the boolean; also ensure toggleMiniHoverOverlay()
(and the related code referenced around lines 126-128) writes the same canonical
format so both read and write paths are consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e825073f-e87c-4c5f-b05b-b3bfe97449ed
📒 Files selected for processing (4)
index.htmlresources/lang/en.jsonsrc/client/graphics/layers/SettingsModal.tssrc/core/game/UserSettings.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- index.html
| <button | ||
| class="flex gap-3 items-center w-full text-left p-3 hover:bg-slate-700 rounded-sm text-white transition-colors" | ||
| @click="${this.onToggleMiniHoverOverlayButtonClick}" | ||
| > | ||
| <img | ||
| src=${swordIcon} | ||
| alt="miniHoverOverlay" | ||
| width="20" | ||
| height="20" | ||
| style="filter: brightness(0) saturate(100%) invert(72%) sepia(56%) saturate(3204%) hue-rotate(176deg) brightness(99%) contrast(101%)" | ||
| /> | ||
| <div class="flex-1"> | ||
| <div class="font-medium"> | ||
| ${translateText("user_setting.mini_hover_overlay_label")} | ||
| </div> | ||
| <div class="text-sm text-slate-400"> | ||
| ${translateText("user_setting.mini_hover_overlay_desc")} | ||
| </div> | ||
| </div> | ||
| <div class="text-sm text-slate-400"> | ||
| ${this.userSettings.miniHoverOverlay() | ||
| ? translateText("user_setting.on") | ||
| : translateText("user_setting.off")} | ||
| </div> | ||
| </button> |
There was a problem hiding this comment.
Don't show a no-op toggle on touch devices.
The mini-hover overlay is desktop-only in this PR, but this row renders for every client. On mobile/tablet this becomes a saved setting with no visible effect. Please hide it there, or render it disabled with a short desktop-only hint.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/client/graphics/layers/SettingsModal.ts` around lines 516 - 540, The
mini-hover overlay toggle (button using onToggleMiniHoverOverlayButtonClick and
reading userSettings.miniHoverOverlay()) is desktop-only but is rendered on
touch devices; modify SettingsModal to detect hover-capable devices and either
hide the entire button or render it disabled with a short hint on non-hover
devices. Implement a small helper (e.g. isHoverCapable or isTouchDevice) in the
SettingsModal component that uses navigator / matchMedia (hover: none) to
determine capability, and wrap the existing button render in that check (or set
disabled/aria-disabled plus a desktop-only hint text) so mobile/tablet users
won't see an active no-op toggle. Ensure you reference
onToggleMiniHoverOverlayButtonClick and userSettings.miniHoverOverlay() when
updating the rendering logic.
2 participants
Description:
This PR adds a lightweight mini-hover overlay for player tooltips in-game, with a toggle to enable/disable it.
What’s included
• Adds a Mini Hover: On/Off toggle button in-game.
• Persists preference in local storage (settings.miniHoverOverlay).
• Adds a compact hover card near cursor on desktop when hovering other players/bots.
• Compact card shows:
◦ Troops / max troops
◦ Red sword + attacking troops (only if target is attacking)
◦ Blue sword + predicted outgoing troops based on current attack ratio
◦ Your current troop % next to predicted outgoing troops
• Keeps the existing full overlay behavior intact.
Files changed
• index.html
• src/client/graphics/GameRenderer.ts
• src/client/graphics/layers/PlayerInfoOverlay.ts
Notes
• Build/TS check passes locally (npm run -s build-dev).
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
HulKiora
--- Screenshot ---