Promo card overlay on game selector #4575
Conversation
|
|
WalkthroughAdds configurable promotional overlays to lobby slots. Overlay settings are schema-validated, fetched with a local fallback, scheduled from lobby changes, animated through video phases, and dismissible from the rendered card. ChangesLobby Card Overlays
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GameModeSelector
participant getLobbyCardOverlays
participant lobby-card-overlays.json
participant LobbySlot
GameModeSelector->>getLobbyCardOverlays: Load overlay definitions
getLobbyCardOverlays->>lobby-card-overlays.json: Fetch configuration
lobby-card-overlays.json-->>getLobbyCardOverlays: Return overlay data
getLobbyCardOverlays-->>GameModeSelector: Return validated overlays
LobbySlot->>GameModeSelector: Report changed lobby gameID
GameModeSelector->>GameModeSelector: Evaluate interval and trigger overlay
GameModeSelector-->>LobbySlot: Render overlay card
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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: 5
🧹 Nitpick comments (2)
src/core/ApiSchemas.ts (1)
353-371: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTighten numeric field constraints.
slot,interval,offset, andttlaccept any number, including negative or fractional values.slotis used as an array index andinterval/offsetfeed a modulo check incheckOverlayTriggers(GameModeSelector.ts) — fractional or negative values there could make promotions never trigger or trigger unpredictably.♻️ Suggested constraint tightening
- slot: z.number(), - interval: z.number(), - offset: z.number().default(0), - ttl: z.number(), + slot: z.number().int().min(0), + interval: z.number().int().min(0), + offset: z.number().int().min(0).default(0), + ttl: z.number().int().min(0),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/ApiSchemas.ts` around lines 353 - 371, The numeric fields in LobbyCardOverlaySchema are too permissive. Constrain slot, interval, offset, and ttl to non-negative integers, while preserving offset’s default of 0; use the resulting validated values unchanged by checkOverlayTriggers.src/client/GameModeSelector.ts (1)
167-201: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConsider tests for the overlay scheduling state machine.
checkOverlayTriggers/triggerOverlayimplement a fairly intricate two-pass counting + phase-timer state machine (four phases, three timer maps). The untracked-timer bug found nearby is a good example of the kind of regression tests would catch.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/client/GameModeSelector.ts` around lines 167 - 201, add tests for the overlay scheduling state machine centered on checkOverlayTriggers and triggerOverlay, covering the two-pass slot counting, interval/offset phase behavior across all four phases, active-overlay suppression, and timer tracking/cleanup including the untracked-timer regression case.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/client/GameModeSelector.ts`:
- Around line 112-119: Track the 800ms handleVideoReady fallback timer created
by triggerOverlay in a dedicated timer map or the existing overlay timer
tracking, then clear and remove it in both disconnectedCallback and
dismissOverlay alongside the video-length and dismiss timers. Ensure the
fallback cannot invoke setActiveOverlay after the overlay is dismissed or the
component disconnects.
- Around line 598-684: Update the overlay markup in GameModeSelector so the
sibling container holding the count badge and subtitle is rendered only when
showReveal is true, matching the existing title/image block. Keep the count and
subtitle content unchanged and ensure neither reveal block appears during
entering or video phases.
- Around line 341-368: The responsive branches in the GameModeSelector render
duplicate cards, causing overlay videos to mount and play in hidden copies.
Update the rendering around renderCard and the special/ffa/teams slot branches
so an overlay card is created only once for the active viewport, or ensure
hidden branches do not assign video src or autoplay; preserve the existing
desktop and mobile layout behavior.
In `@src/core/ApiSchemas.ts`:
- Around line 352-372: Add tests for LobbyCardOverlaySchema using safeParse,
covering a valid payload and invalid payloads with missing required fields and
incorrect types. Ensure the tests also verify the offset default behavior and
exercise the exported LobbyCardOverlay schema contract.
- Around line 353-371: Restrict the linkTo field in LobbyCardOverlaySchema to
URLs using only the http or https protocols, replacing the unrestricted z.url()
validation. Update src/core/ApiSchemas.ts lines 353-371;
src/client/GameModeSelector.ts lines 592-597 requires no direct change because
the schema validation provides the protection.
---
Nitpick comments:
In `@src/client/GameModeSelector.ts`:
- Around line 167-201: add tests for the overlay scheduling state machine
centered on checkOverlayTriggers and triggerOverlay, covering the two-pass slot
counting, interval/offset phase behavior across all four phases, active-overlay
suppression, and timer tracking/cleanup including the untracked-timer regression
case.
In `@src/core/ApiSchemas.ts`:
- Around line 353-371: The numeric fields in LobbyCardOverlaySchema are too
permissive. Constrain slot, interval, offset, and ttl to non-negative integers,
while preserving offset’s default of 0; use the resulting validated values
unchanged by checkOverlayTriggers.
🪄 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: 264facca-43b4-4a40-981a-bcdfe9a90464
⛔ Files ignored due to path filters (2)
resources/images/promo/trophy.pngis excluded by!**/*.pngresources/video/promo/ofm_card_intro.mp4is excluded by!**/*.mp4
📒 Files selected for processing (4)
resources/lobby-card-overlays.jsonsrc/client/Api.tssrc/client/GameModeSelector.tssrc/core/ApiSchemas.ts
| for (const timer of this.overlayTimers.values()) { | ||
| clearTimeout(timer); | ||
| } | ||
| this.overlayTimers.clear(); | ||
| for (const timer of this.overlayDismissTimers.values()) { | ||
| clearTimeout(timer); | ||
| } | ||
| this.overlayDismissTimers.clear(); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Untracked setTimeout for handleVideoReady — never cleared.
The 800ms fallback timer in triggerOverlay isn't stored anywhere, unlike the video-length timer (via setOverlayTimer) and the dismiss timer (via overlayDismissTimers) set right next to it. disconnectedCallback (lines 112-119) and dismissOverlay only clear those two maps, so this timer keeps running after disconnect/dismissal and can call setActiveOverlay on a torn-down or unrelated overlay state later.
🔧 Suggested fix: track the ready-fallback timer like the others
+ private overlayReadyTimers: Map<number, ReturnType<typeof setTimeout>> = new Map();
+
private triggerOverlay(overlay: LobbyCardOverlay) {
const slot = overlay.slot;
this.setActiveOverlay(slot, { slot, overlay, phase: "entering" });
- setTimeout(() => this.handleVideoReady(slot), 800);
+ const existingReady = this.overlayReadyTimers.get(slot);
+ if (existingReady) clearTimeout(existingReady);
+ this.overlayReadyTimers.set(
+ slot,
+ setTimeout(() => this.handleVideoReady(slot), 800),
+ );
this.setOverlayTimer(slot, (overlay.video.videoLength + 2) * 1000, () =>
this.advanceOverlayToFading(slot),
);Then clear it alongside the other maps in disconnectedCallback and in dismissOverlay.
Also applies to: 227-240
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/client/GameModeSelector.ts` around lines 112 - 119, Track the 800ms
handleVideoReady fallback timer created by triggerOverlay in a dedicated timer
map or the existing overlay timer tracking, then clear and remove it in both
disconnectedCallback and dismissOverlay alongside the video-length and dismiss
timers. Ensure the fallback cannot invoke setActiveOverlay after the overlay is
dismissed or the component disconnects.
| ${this.renderCard(SLOT_FFA, ffa)} | ||
| </div>` | ||
| : nothing} | ||
|
|
||
| <!-- Right col: special + teams (desktop only) --> | ||
| <div class="hidden sm:flex sm:flex-col sm:gap-4"> | ||
| ${special | ||
| ? html`<div class="flex-1 min-h-0"> | ||
| ${this.renderSpecialLobbyCard(special)} | ||
| ${this.renderCard(SLOT_SPECIAL, special)} | ||
| </div>` | ||
| : nothing} | ||
| ${teams | ||
| ? html`<div class="flex-1 min-h-0"> | ||
| ${this.renderLobbyCard(teams, this.getLobbyTitle(teams))} | ||
| ${this.renderCard(SLOT_TEAM, teams)} | ||
| </div>` | ||
| : nothing} | ||
| </div> | ||
|
|
||
| <!-- Mobile: special, ffa, teams inline --> | ||
| <div class="sm:hidden"> | ||
| ${special ? this.renderSpecialLobbyCard(special) : nothing} | ||
| ${special ? this.renderCard(SLOT_SPECIAL, special) : nothing} | ||
| </div> | ||
| <div class="sm:hidden"> | ||
| ${ffa | ||
| ? this.renderLobbyCard(ffa, this.getLobbyTitle(ffa)) | ||
| : nothing} | ||
| ${ffa ? this.renderCard(SLOT_FFA, ffa) : nothing} | ||
| </div> | ||
| <div class="sm:hidden"> | ||
| ${teams | ||
| ? this.renderLobbyCard(teams, this.getLobbyTitle(teams)) | ||
| : nothing} | ||
| ${teams ? this.renderCard(SLOT_TEAM, teams) : nothing} | ||
| </div> |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the selector and the card renderer around the referenced lines.
ast-grep outline src/client/GameModeSelector.ts --view expanded
# Read the relevant sections with line numbers.
sed -n '300,760p' src/client/GameModeSelector.ts | cat -nRepository: openfrontio/OpenFrontIO
Length of output: 25573
🌐 Web query:
display:none video autoplay still loads decodes browser behavior hidden video element muted autoplay
💡 Result:
The behavior of a hidden video element (e.g., set to display:none) regarding loading, decoding, and autoplay is not uniform across all browsers, though several key patterns exist. Generally, setting a video element to display:none does not prevent the browser from loading or buffering the video resource [1][2][3]. The browser treats the video element as part of the document regardless of its CSS display property, meaning the network request for the video file will typically still occur unless other factors (such as the preload attribute) intervene [1][4][5]. Regarding autoplay, browser behavior has become increasingly restrictive to save resources and improve user experience: 1. Loading and Decoding: Applying display:none does not inherently stop the browser from downloading or decoding the media [1][2]. If the browser's preload strategy (e.g., preload="auto") is active, the video will still be fetched [4][6]. 2. Autoplay Restrictions: Modern browsers implement strict policies regarding autoplay [7]. While muted autoplay is generally permitted when visible, many browsers have introduced "invisible autoplay" restrictions [8][9]. These policies may pause or prevent playback if a video element is not currently visible within the viewport or is hidden using CSS like display:none [8][10][9]. 3. Implementation Differences: - Safari (WebKit): Historically, Safari on iOS and macOS has been more aggressive in pausing or preventing autoplay for elements that are not visible or are outside the viewport to save battery and data [1][8][9]. - Chrome/Blink: Chrome also enforces strict autoplay policies, and while it may not always block the initial network load simply due to display:none, it may opt to pause or throttle playback for hidden elements to optimize performance [10][7]. - Standards: The HTML specification does not mandate that display:none must prevent media from loading or playing, which leads to the variation in implementation [4][11]. To reliably prevent a video from loading or playing when it is not needed (e.g., on mobile devices), the most robust approach is not to rely on CSS, but to use JavaScript to conditionally inject the video element into the DOM or set its src attribute only when needed [6][2][5][3].
Citations:
- 1: https://stackoverflow.com/questions/41058916/does-video-autoplay-pause-when-element-not-visible-in-chrome
- 2: https://stackoverflow.com/questions/50476549/does-displaynone-prevent-content-being-loaded
- 3: https://stackoverflow.com/questions/4099637/does-displaynone-keep-elements-from-loading
- 4: https://html.spec.whatwg.org/multipage/media.html
- 5: https://stackoverflow.com/questions/37591489/prevent-load-video-that-included-in-video-tag-using-css
- 6: https://stackoverflow.com/questions/76063835/will-a-hidden-html5-video-actually-load
- 7: https://developer.chrome.com/blog/autoplay
- 8: Interrupt autoplay when video element becomes invisible whatwg/html#2324
- 9: WebKit/WebKit@0146d52
- 10: https://groups.google.com/a/chromium.org/g/blink-dev/c/_HuO-G0rn1E/m/93tZTHzlDgAJ
- 11: https://bugzilla.mozilla.org/show_bug.cgi?id=697651
Avoid mounting the overlay video in both responsive branches
The sm:hidden / hidden sm:* split keeps both DOM copies alive. When a slot is on the overlay card, the hidden copy still contains an autoplaying <video>, which can still fetch and decode media in the background. Render the active overlay once, or gate src/autoplay behind a viewport check so only the visible branch creates the video.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/client/GameModeSelector.ts` around lines 341 - 368, The responsive
branches in the GameModeSelector render duplicate cards, causing overlay videos
to mount and play in hidden copies. Update the rendering around renderCard and
the special/ffa/teams slot branches so an overlay card is created only once for
the active viewport, or ensure hidden branches do not assign video src or
autoplay; preserve the existing desktop and mobile layout behavior.
| <div class="flex-1 min-h-0 flex items-center gap-3 px-3 py-2"> | ||
| ${showReveal | ||
| ? html` | ||
| <h3 | ||
| class="flex-1 min-w-0 text-base sm:text-xl font-bold uppercase tracking-wider leading-snug" | ||
| > | ||
| ${overlay.displayInfo.title} | ||
| </h3> | ||
| ${overlay.image.url | ||
| ? html`<img | ||
| src="${overlay.image.url}" | ||
| alt="" | ||
| class="shrink-0 w-14 h-14 sm:w-20 sm:h-20 object-contain" | ||
| />` | ||
| : nothing} | ||
| ` | ||
| : nothing} | ||
| </div> | ||
| <div | ||
| class="relative z-10 shrink-0 flex flex-col px-3 py-2 bg-black/55 backdrop-blur-sm rounded-b-2xl" | ||
| style="overflow: visible;" | ||
| > | ||
| ${overlay.displayInfo.count !== undefined | ||
| ? html`<span | ||
| class="absolute bottom-full right-2 mb-1 flex items-center gap-1 text-xs font-bold tracking-widest bg-black/70 backdrop-blur-sm px-2 py-0.5 rounded" | ||
| > | ||
| ${overlay.displayInfo.count} | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| class="h-4 w-4 shrink-0" | ||
| viewBox="0 0 512 512" | ||
| fill="currentColor" | ||
| > | ||
| <path | ||
| d="M430.337,231.065H81.674c-29.701,0-53.858,24.16-53.858,53.862v49.884v15.976l15.806,2.262c9.135,1.31,16.03,9.258,16.03,18.483c0,9.225-6.891,17.173-16.022,18.482l-15.814,2.262v15.978v49.892c0,29.693,24.157,53.854,53.858,53.854h348.663c29.701,0,53.862-24.161,53.862-53.854v-49.558V391l-17.571-0.822c-9.982-0.463-17.808-8.655-17.808-18.645c0-9.982,7.826-18.174,17.815-18.646l17.564-0.83v-17.58v-49.55C484.199,255.225,460.038,231.065,430.337,231.065z M465.765,334.477c-19.686,0.936-35.371,17.14-35.371,37.056c0,19.923,15.685,36.135,35.371,37.055v49.558c0,19.565-15.864,35.428-35.428,35.428H81.674c-19.569,0-35.432-15.863-35.432-35.428v-49.892c17.991-2.579,31.836-18.011,31.836-36.722c0-18.703-13.846-34.135-31.836-36.721v-49.884c0-19.573,15.863-35.436,35.432-35.436h348.663c19.564,0,35.428,15.863,35.428,35.436V334.477z" | ||
| ></path> | ||
| <rect | ||
| x="133.621" | ||
| y="439.419" | ||
| width="12.19" | ||
| height="31.8" | ||
| ></rect> | ||
| <rect | ||
| x="133.621" | ||
| y="383.564" | ||
| width="12.19" | ||
| height="31.792" | ||
| ></rect> | ||
| <rect | ||
| x="133.621" | ||
| y="327.7" | ||
| width="12.19" | ||
| height="31.8" | ||
| ></rect> | ||
| <rect | ||
| x="133.621" | ||
| y="271.846" | ||
| width="12.19" | ||
| height="31.799" | ||
| ></rect> | ||
| <polygon | ||
| points="111.245,180.758 100.592,186.68 116.053,214.461 126.702,208.539" | ||
| ></polygon> | ||
| <path | ||
| d="M497.524,179.025l-24.095-43.311l-8.558-15.36l-15.749,7.826c-8.948,4.442-19.768,1.09-24.617-7.639c-4.865-8.721-2.001-19.687,6.492-24.95l14.952-9.266l-8.558-15.368l-24.088-43.294C398.863,1.714,366.006-7.658,340.047,6.79L35.374,176.299c-25.955,14.44-35.318,47.305-20.878,73.256l0.875,1.578c3.27-6.394,7.43-12.243,12.324-17.409c-4.803-15.643,1.762-33.044,16.636-41.326l304.681-169.51c17.1-9.518,38.674-3.368,48.192,13.732l24.088,43.302c-16.751,10.38-22.575,32.182-12.895,49.582c9.681,17.401,31.271,23.942,48.925,15.172l24.095,43.312c7.273,13.056,5.337,28.692-3.571,39.601c4.776,3.961,8.989,8.558,12.65,13.569C505.4,224.524,508.979,199.615,497.524,179.025z" | ||
| ></path> | ||
| </svg> | ||
| </span>` | ||
| : nothing} | ||
| <p | ||
| class="flex justify-end items-center gap-1 text-xs text-white/70 uppercase tracking-wider truncate" | ||
| > | ||
| ${overlay.displayInfo.subtitle} | ||
| <svg | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| class="h-3 w-3 shrink-0" | ||
| viewBox="0 0 20 20" | ||
| fill="currentColor" | ||
| > | ||
| <path | ||
| fill-rule="evenodd" | ||
| d="M12.293 5.293a1 1 0 011.414 0l4 4a1 1 0 010 1.414l-4 4a1 1 0 01-1.414-1.414L14.586 11H3a1 1 0 110-2h11.586l-2.293-2.293a1 1 0 010-1.414z" | ||
| clip-rule="evenodd" | ||
| ></path> | ||
| </svg> | ||
| </p> | ||
| </div> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Count badge / subtitle aren't gated by showReveal, unlike the title/image block.
The title+image block (599-614) only renders when showReveal is true (fading/card phases), but the sibling div holding the count badge and subtitle (616-684) is unconditional — it renders during entering/video too, undercutting the intended reveal effect described for this feature.
🐛 Suggested fix: gate both blocks together
- <div class="flex-1 min-h-0 flex items-center gap-3 px-3 py-2">
- ${showReveal
- ? html`
- <h3 ...>${overlay.displayInfo.title}</h3>
- ${overlay.image.url ? html`<img ... />` : nothing}
- `
- : nothing}
- </div>
- <div class="relative z-10 shrink-0 flex flex-col px-3 py-2 bg-black/55 backdrop-blur-sm rounded-b-2xl" style="overflow: visible;">
- ${overlay.displayInfo.count !== undefined ? html`...` : nothing}
- <p>...${overlay.displayInfo.subtitle}...</p>
- </div>
+ ${showReveal
+ ? html`
+ <div class="flex-1 min-h-0 flex items-center gap-3 px-3 py-2">
+ <h3 ...>${overlay.displayInfo.title}</h3>
+ ${overlay.image.url ? html`<img ... />` : nothing}
+ </div>
+ <div class="relative z-10 shrink-0 flex flex-col px-3 py-2 bg-black/55 backdrop-blur-sm rounded-b-2xl" style="overflow: visible;">
+ ${overlay.displayInfo.count !== undefined ? html`...` : nothing}
+ <p>...${overlay.displayInfo.subtitle}...</p>
+ </div>
+ `
+ : nothing}📝 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.
| <div class="flex-1 min-h-0 flex items-center gap-3 px-3 py-2"> | |
| ${showReveal | |
| ? html` | |
| <h3 | |
| class="flex-1 min-w-0 text-base sm:text-xl font-bold uppercase tracking-wider leading-snug" | |
| > | |
| ${overlay.displayInfo.title} | |
| </h3> | |
| ${overlay.image.url | |
| ? html`<img | |
| src="${overlay.image.url}" | |
| alt="" | |
| class="shrink-0 w-14 h-14 sm:w-20 sm:h-20 object-contain" | |
| />` | |
| : nothing} | |
| ` | |
| : nothing} | |
| </div> | |
| <div | |
| class="relative z-10 shrink-0 flex flex-col px-3 py-2 bg-black/55 backdrop-blur-sm rounded-b-2xl" | |
| style="overflow: visible;" | |
| > | |
| ${overlay.displayInfo.count !== undefined | |
| ? html`<span | |
| class="absolute bottom-full right-2 mb-1 flex items-center gap-1 text-xs font-bold tracking-widest bg-black/70 backdrop-blur-sm px-2 py-0.5 rounded" | |
| > | |
| ${overlay.displayInfo.count} | |
| <svg | |
| xmlns="http://www.w3.org/2000/svg" | |
| class="h-4 w-4 shrink-0" | |
| viewBox="0 0 512 512" | |
| fill="currentColor" | |
| > | |
| <path | |
| d="M430.337,231.065H81.674c-29.701,0-53.858,24.16-53.858,53.862v49.884v15.976l15.806,2.262c9.135,1.31,16.03,9.258,16.03,18.483c0,9.225-6.891,17.173-16.022,18.482l-15.814,2.262v15.978v49.892c0,29.693,24.157,53.854,53.858,53.854h348.663c29.701,0,53.862-24.161,53.862-53.854v-49.558V391l-17.571-0.822c-9.982-0.463-17.808-8.655-17.808-18.645c0-9.982,7.826-18.174,17.815-18.646l17.564-0.83v-17.58v-49.55C484.199,255.225,460.038,231.065,430.337,231.065z M465.765,334.477c-19.686,0.936-35.371,17.14-35.371,37.056c0,19.923,15.685,36.135,35.371,37.055v49.558c0,19.565-15.864,35.428-35.428,35.428H81.674c-19.569,0-35.432-15.863-35.432-35.428v-49.892c17.991-2.579,31.836-18.011,31.836-36.722c0-18.703-13.846-34.135-31.836-36.721v-49.884c0-19.573,15.863-35.436,35.432-35.436h348.663c19.564,0,35.428,15.863,35.428,35.436V334.477z" | |
| ></path> | |
| <rect | |
| x="133.621" | |
| y="439.419" | |
| width="12.19" | |
| height="31.8" | |
| ></rect> | |
| <rect | |
| x="133.621" | |
| y="383.564" | |
| width="12.19" | |
| height="31.792" | |
| ></rect> | |
| <rect | |
| x="133.621" | |
| y="327.7" | |
| width="12.19" | |
| height="31.8" | |
| ></rect> | |
| <rect | |
| x="133.621" | |
| y="271.846" | |
| width="12.19" | |
| height="31.799" | |
| ></rect> | |
| <polygon | |
| points="111.245,180.758 100.592,186.68 116.053,214.461 126.702,208.539" | |
| ></polygon> | |
| <path | |
| d="M497.524,179.025l-24.095-43.311l-8.558-15.36l-15.749,7.826c-8.948,4.442-19.768,1.09-24.617-7.639c-4.865-8.721-2.001-19.687,6.492-24.95l14.952-9.266l-8.558-15.368l-24.088-43.294C398.863,1.714,366.006-7.658,340.047,6.79L35.374,176.299c-25.955,14.44-35.318,47.305-20.878,73.256l0.875,1.578c3.27-6.394,7.43-12.243,12.324-17.409c-4.803-15.643,1.762-33.044,16.636-41.326l304.681-169.51c17.1-9.518,38.674-3.368,48.192,13.732l24.088,43.302c-16.751,10.38-22.575,32.182-12.895,49.582c9.681,17.401,31.271,23.942,48.925,15.172l24.095,43.312c7.273,13.056,5.337,28.692-3.571,39.601c4.776,3.961,8.989,8.558,12.65,13.569C505.4,224.524,508.979,199.615,497.524,179.025z" | |
| ></path> | |
| </svg> | |
| </span>` | |
| : nothing} | |
| <p | |
| class="flex justify-end items-center gap-1 text-xs text-white/70 uppercase tracking-wider truncate" | |
| > | |
| ${overlay.displayInfo.subtitle} | |
| <svg | |
| xmlns="http://www.w3.org/2000/svg" | |
| class="h-3 w-3 shrink-0" | |
| viewBox="0 0 20 20" | |
| fill="currentColor" | |
| > | |
| <path | |
| fill-rule="evenodd" | |
| d="M12.293 5.293a1 1 0 011.414 0l4 4a1 1 0 010 1.414l-4 4a1 1 0 01-1.414-1.414L14.586 11H3a1 1 0 110-2h11.586l-2.293-2.293a1 1 0 010-1.414z" | |
| clip-rule="evenodd" | |
| ></path> | |
| </svg> | |
| </p> | |
| </div> | |
| ${showReveal | |
| ? html` | |
| <div class="flex-1 min-h-0 flex items-center gap-3 px-3 py-2"> | |
| <h3 | |
| class="flex-1 min-w-0 text-base sm:text-xl font-bold uppercase tracking-wider leading-snug" | |
| > | |
| ${overlay.displayInfo.title} | |
| </h3> | |
| ${overlay.image.url | |
| ? html`<img | |
| src="${overlay.image.url}" | |
| alt="" | |
| class="shrink-0 w-14 h-14 sm:w-20 sm:h-20 object-contain" | |
| />` | |
| : nothing} | |
| </div> | |
| <div | |
| class="relative z-10 shrink-0 flex flex-col px-3 py-2 bg-black/55 backdrop-blur-sm rounded-b-2xl" | |
| style="overflow: visible;" | |
| > | |
| ${overlay.displayInfo.count !== undefined | |
| ? html`<span | |
| class="absolute bottom-full right-2 mb-1 flex items-center gap-1 text-xs font-bold tracking-widest bg-black/70 backdrop-blur-sm px-2 py-0.5 rounded" | |
| > | |
| ${overlay.displayInfo.count} | |
| <svg | |
| xmlns="http://www.w3.org/2000/svg" | |
| class="h-4 w-4 shrink-0" | |
| viewBox="0 0 512 512" | |
| fill="currentColor" | |
| > | |
| <path | |
| d="M430.337,231.065H81.674c-29.701,0-53.858,24.16-53.858,53.862v49.884v15.976l15.806,2.262c9.135,1.31,16.03,9.258,16.03,18.483c0,9.225-6.891,17.173-16.022,18.482l-15.814,2.262v15.978v49.892c0,29.693,24.157,53.854,53.858,53.854h348.663c29.701,0,53.862-24.161,53.862-53.854v-49.558V391l-17.571-0.822c-9.982-0.463-17.808-8.655-17.808-18.645c0-9.982,7.826-18.174,17.815-18.646l17.564-0.83v-17.58v-49.55C484.199,255.225,460.038,231.065,430.337,231.065z M465.765,334.477c-19.686,0.936-35.371,17.14-35.371,37.056c0,19.923,15.685,36.135,35.371,37.055v49.558c0,19.565-15.864,35.428-35.428,35.428H81.674c-19.569,0-35.432-15.863-35.432-35.428v-49.892c17.991-2.579,31.836-18.011,31.836-36.722c0-18.703-13.846-34.135-31.836-36.721v-49.884c0-19.573,15.863-35.436,35.432-35.436h348.663c19.564,0,35.428,15.863,35.428,35.436V334.477z" | |
| ></path> | |
| <rect | |
| x="133.621" | |
| y="439.419" | |
| width="12.19" | |
| height="31.8" | |
| ></rect> | |
| <rect | |
| x="133.621" | |
| y="383.564" | |
| width="12.19" | |
| height="31.792" | |
| ></rect> | |
| <rect | |
| x="133.621" | |
| y="327.7" | |
| width="12.19" | |
| height="31.8" | |
| ></rect> | |
| <rect | |
| x="133.621" | |
| y="271.846" | |
| width="12.19" | |
| height="31.799" | |
| ></rect> | |
| <polygon | |
| points="111.245,180.758 100.592,186.68 116.053,214.461 126.702,208.539" | |
| ></polygon> | |
| <path | |
| d="M497.524,179.025l-24.095-43.311l-8.558-15.36l-15.749,7.826c-8.948,4.442-19.768,1.09-24.617-7.639c-4.865-8.721-2.001-19.687,6.492-24.95l14.952-9.266l-8.558-15.368l-24.088-43.294C398.863,1.714,366.006-7.658,340.047,6.79L35.374,176.299c-25.955,14.44-35.318,47.305-20.878,73.256l0.875,1.578c3.27-6.394,7.43-12.243,12.324-17.409c-4.803-15.643,1.762-33.044,16.636-41.326l304.681-169.51c17.1-9.518,38.674-3.368,48.192,13.732l24.088,43.302c-16.751,10.38-22.575,32.182-12.895,49.582c9.681,17.401,31.271,23.942,48.925,15.172l24.095,43.312c7.273,13.056,5.337,28.692-3.571,39.601c4.776,3.961,8.989,8.558,12.65,13.569C505.4,224.524,508.979,199.615,497.524,179.025z" | |
| ></path> | |
| </svg> | |
| </span>` | |
| : nothing} | |
| <p | |
| class="flex justify-end items-center gap-1 text-xs text-white/70 uppercase tracking-wider truncate" | |
| > | |
| ${overlay.displayInfo.subtitle} | |
| <svg | |
| xmlns="http://www.w3.org/2000/svg" | |
| class="h-3 w-3 shrink-0" | |
| viewBox="0 0 20 20" | |
| fill="currentColor" | |
| > | |
| <path | |
| fill-rule="evenodd" | |
| d="M12.293 5.293a1 1 0 011.414 0l4 4a1 1 0 010 1.414l-4 4a1 1 0 01-1.414-1.414L14.586 11H3a1 1 0 110-2h11.586l-2.293-2.293a1 1 0 010-1.414z" | |
| clip-rule="evenodd" | |
| ></path> | |
| </svg> | |
| </p> | |
| </div> | |
| ` | |
| : nothing} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/client/GameModeSelector.ts` around lines 598 - 684, Update the overlay
markup in GameModeSelector so the sibling container holding the count badge and
subtitle is rendered only when showReveal is true, matching the existing
title/image block. Keep the count and subtitle content unchanged and ensure
neither reveal block appears during entering or video phases.
|
|
||
| export const LobbyCardOverlaySchema = z.object({ | ||
| slot: z.number(), | ||
| interval: z.number(), | ||
| offset: z.number().default(0), | ||
| ttl: z.number(), | ||
| video: z.object({ | ||
| url: z.string(), | ||
| videoLength: z.number(), | ||
| }), | ||
| image: z.object({ | ||
| url: z.string(), | ||
| }), | ||
| displayInfo: z.object({ | ||
| title: z.string(), | ||
| subtitle: z.string(), | ||
| count: z.number().optional(), | ||
| }), | ||
| linkTo: z.url(), | ||
| }); | ||
| export type LobbyCardOverlay = z.infer<typeof LobbyCardOverlaySchema>; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add tests for the new schema.
No tests accompany this new LobbyCardOverlaySchema/LobbyCardOverlay export.
As per coding guidelines, "All changes to src/core/ must include tests." A simple safeParse test asserting valid/invalid payloads (missing fields, wrong types) would satisfy this.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/ApiSchemas.ts` around lines 352 - 372, Add tests for
LobbyCardOverlaySchema using safeParse, covering a valid payload and invalid
payloads with missing required fields and incorrect types. Ensure the tests also
verify the offset default behavior and exercise the exported LobbyCardOverlay
schema contract.
Source: Coding guidelines
| export const LobbyCardOverlaySchema = z.object({ | ||
| slot: z.number(), | ||
| interval: z.number(), | ||
| offset: z.number().default(0), | ||
| ttl: z.number(), | ||
| video: z.object({ | ||
| url: z.string(), | ||
| videoLength: z.number(), | ||
| }), | ||
| image: z.object({ | ||
| url: z.string(), | ||
| }), | ||
| displayInfo: z.object({ | ||
| title: z.string(), | ||
| subtitle: z.string(), | ||
| count: z.number().optional(), | ||
| }), | ||
| linkTo: z.url(), | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== package metadata ==\n'
git ls-files package.json pnpm-lock.yaml yarn.lock package-lock.json 2>/dev/null || true
for f in package.json pnpm-lock.yaml yarn.lock package-lock.json; do
if [ -f "$f" ]; then
printf '\n-- %s --\n' "$f"
sed -n '1,220p' "$f"
fi
done
printf '\n== locate relevant files ==\n'
git ls-files 'src/core/ApiSchemas.ts' 'src/client/GameModeSelector.ts'
printf '\n== source excerpts ==\n'
sed -n '330,390p' src/core/ApiSchemas.ts
printf '\n---\n'
sed -n '560,620p' src/client/GameModeSelector.ts
printf '\n== zod url semantics probe ==\n'
node - <<'JS'
let zod;
try {
zod = require('zod');
} catch (e) {
console.log('require-zod-failed:', e.message);
process.exit(0);
}
const schema = zod.z ? zod.z.url() : zod.url();
const samples = [
'https://example.com',
'http://example.com',
'mailto:test@example.com',
'javascript:alert(1)',
'data:text/plain,hi',
'//example.com',
'/relative/path',
];
for (const s of samples) {
const res = schema.safeParse(s);
console.log(JSON.stringify({ input: s, success: res.success, error: res.success ? null : res.error.issues.map(i => i.message) }));
}
JSRepository: openfrontio/OpenFrontIO
Length of output: 17072
Restrict linkTo to http(s) z.url() accepts javascript: and data: URLs, and GameModeSelector.ts renders this value directly into <a href target="_blank">. Narrow the schema here so a bad overlay cannot become an unsafe clickable link.
📍 Affects 2 files
src/core/ApiSchemas.ts#L353-L371(this comment)src/client/GameModeSelector.ts#L592-L597
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/ApiSchemas.ts` around lines 353 - 371, Restrict the linkTo field in
LobbyCardOverlaySchema to URLs using only the http or https protocols, replacing
the unrestricted z.url() validation. Update src/core/ApiSchemas.ts lines
353-371; src/client/GameModeSelector.ts lines 592-597 requires no direct change
because the schema validation provides the protection.
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
2 participants
Add approved & assigned issue number here:
Resolves #4573
Description:
Adds promo cards read from a json file overlay to specified game lobby select slots.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
troshujin