Skip to content

fix(homepage): stop the lobby cards drawing a rim, and let the pills breathe - #5095

Closed
ryanbarlow97 wants to merge 4 commits into
mainfrom
proto/homepage-dashboard
Closed

fix(homepage): stop the lobby cards drawing a rim, and let the pills breathe#5095
ryanbarlow97 wants to merge 4 commits into
mainfrom
proto/homepage-dashboard

Conversation

@ryanbarlow97

@ryanbarlow97 ryanbarlow97 commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Two cosmetic fixes to the homepage lobby cards. One file, one commit — main's layout, card selection and ordering are untouched.

The cards drew a rim along their corners. Three nested radii — the card button, the image wrapper inside it, and the name bar at its foot — plus the bar's backdrop-filter left a bright 1–2px line tracing each corner. The card is now the only rounded, clipping box: the image layer and the name bar are flat and get clipped by it. Found by pixel measurement at DPR 1–8 rather than by eye.

Modifier pills lose their max-w-[65%] cap and sort longest first, so on a small card the labels that say the most are the ones that stay legible, and a long label uses the width the card actually has.

History

This PR previously carried a much larger homepage redesign plus a server-side promotion rotation. Both are out: the ordering work only existed to give the redesign a single ordered queue to render, and with main's per-bucket cards (FFA as the main card, team and special beside it) there is nothing for it to do. Nothing was deleted — if any of it is wanted later it is on these branches:

  • homepage-ui — the full play-surface redesign: hero card, UPCOMING column, full-width SOLO row, PlayPage grid changes.
  • homepage-rotation — the master's promotion rotation and PublicGameInfo.queuePosition, stacked on the above.
  • t3code/a0aa255a — a smaller take on the buttons: full-width SOLO with CREATE / RANKED / JOIN underneath, and a per-card expand control replacing the DETAILED VIEW button.

Testing

npm test — 339 files / 4026 tests, plus 57 / 588 on the server pass, exit 0. tsc, oxlint, eslint and Prettier clean.

The rim itself was measured in a real browser when it was first found. This diff has not had a fresh browser pass: the machine it was written on is arm64 and the pinned headless Chromium is x86-64, so the harness won't run there. Worth a glance at a card's corners and at a card carrying two or three modifier pills before merging.

@coderabbitai

coderabbitai Bot commented Aug 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Changes

The homepage dashboard is replaced by GameModeSelector. It renders identity controls, public lobbies, lobby actions, streaming content, and navigation. Server scheduling now uses one shared rotating queue with pending-operation tracking. Multiplayer gating is shared through MultiplayerGate.

Lobby dashboard and rotation

Layer / File(s) Summary
Shared lobby rotation and queue metadata
src/server/MasterLobbyService.ts, src/core/Schemas.ts, tests/server/*
Scheduled lobbies use one ffa/team/special rotation with a combined length of 18. Pending creates and promotions are tracked. Queue positions and deterministic ordering are exposed.
Shared multiplayer gating and action wiring
src/client/MultiplayerGate.ts, src/client/components/DetailedGameViewModal.ts, tests/*Gating*, tests/MultiplayerGate.test.ts
Desktop-update blocking is centralized in MultiplayerGate. Modal and selector wiring use the shared gate and updated card controls.
Game mode selector and lobby cards
src/client/GameModeSelector.ts, src/client/components/LobbyCard.ts, src/client/components/PlayPage.ts, src/client/Main.ts, resources/lang/en.json
The new selector replaces the dashboard. Lobby cards use overlay navigation, sibling action buttons, countdowns, capped modifiers, and accessibility attributes.
Persistent streaming panel
src/client/components/StreamingNow.ts, tests/client/components/StreamingNow.test.ts, resources/lang/en.json
The streaming panel remains visible without live streams and shows a translated Discord invitation.

Estimated code review effort: 4 (Complex) | ~60 minutes

Merge Risk: 🔵 Low · up to 66e7e

The homepage change leaves carousel controls without accessible names and retains an English-only streamer invitation, which can make navigation unclear for screen-reader users and leave some users with untranslated copy. The PR is mergeable with explicit owner follow-up on these bounded accessibility and localization issues; no merge-blocking correctness risk is supported.

Sequence Diagram(s)

sequenceDiagram
  participant MasterLobbyService
  participant LobbyWorker
  participant GameModeSelector
  MasterLobbyService->>MasterLobbyService: build the shared rotation queue
  MasterLobbyService->>LobbyWorker: create or promote a lobby
  LobbyWorker-->>MasterLobbyService: report lobby state and startsAt
  MasterLobbyService->>GameModeSelector: provide queuePosition and lobby data
Loading
sequenceDiagram
  participant GameModeSelector
  participant MultiplayerGate
  participant LobbyCard
  participant DetailedGameViewModal
  GameModeSelector->>MultiplayerGate: check multiplayer action
  GameModeSelector->>LobbyCard: render lobby controls
  LobbyCard-->>GameModeSelector: dispatch card selection
  GameModeSelector->>DetailedGameViewModal: open lobby details
Loading

Suggested reviewers: evanpelle

Poem

One queue turns, three modes align,
Cards reveal each lobby line.
Streams stay bright when channels sleep,
Gates guard multiplayer actions.
New game paths now flow.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
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 accurately describes the LobbyCard rim and modifier-pill changes, but it does not cover the larger homepage, client, and server changes in the changeset. It remains related and specific.
Description check ✅ Passed The description directly explains the LobbyCard styling changes and also provides relevant history and testing information. It is related to the changeset.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch

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.

coderabbitai[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

@ryanbarlow97 ryanbarlow97 added Backend Server-side features and systems - lobbies, matchmaking, accounts, APIs, etc. UI/UX UI/UX changes including assets, menus, QoL, etc. labels Aug 23, 2026
coderabbitai[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

@github-actions

Copy link
Copy Markdown

🤖 Claude Code Review

Verdict: Needs changes — 1 functional bug, 2 accessibility regressions. No CLAUDE.md violations found.

Findings by severity: 3 medium, 0 low.


1. src/client/GameModeSelector.ts:322-325 — "See all" button silently no-ops when the username is invalid

The new "See all {count}" button in renderQueueColumn() wires its click to openDetailed (GameModeSelector.ts:478-480):

private openDetailed = () => {
  if (!this.validateUsername()) return;
  window.showPage?.("page-detailed-view");
};

validateUsername() returns false and the handler returns immediately — no toast, no visual feedback — when the username input is invalid. Every other gated action in this file (e.g. openSolo at GameModeSelector.ts:252-253) binds ?disabled=${!this.inputValid} plus opacity-50/pointer-events-none styling so the control visibly reflects the gate. The "See all" button has no such binding, so it always renders fully clickable and silently does nothing when the username is invalid. This also regresses prior behavior: the entry point it replaces (renderSmallActionCard(translateText("main.detailed_view"), this.openDetailedView, ...)) did bind the disabled state.

Worth noting the gate itself may be unnecessary: page-detailed-view (DetailedGameViewModal.ts) already has its own independent username check on the join path (canPlay() / join()), so browsing the lobby list doesn't actually require a valid username — only joining does.

Suggested fix: either drop the validateUsername() gate from openDetailed entirely (navigation is read-only and the modal already gates joins), or add the same ?disabled=${!this.inputValid} + disabled styling used by the other action buttons in this file.


2. src/client/components/LobbyCard.ts:262-267 — Trust-lock icon's native tooltip is unreachable

The rebuilt "Name and mode" bar now carries pointer-events-none:

class="absolute inset-x-0 bottom-0 h-14 flex flex-col justify-center px-3 bg-black/70 pointer-events-none ${trustedOnly ? "pr-10" : ""}"

trustLockIcon(viewerTrusted) is rendered as the first child of this bar and relies on a native title=${label} attribute (unchanged, ~lines 271-283) to show "Trusted accounts only..." on hover. Since the parent has pointer-events-none and nothing re-enables pointer events on the icon's own <span>, the browser never fires hover events there, so the tooltip can no longer appear. Before this PR the whole card was a single <button> with no pointer-events-none ancestor, so the tooltip worked. Note the new full-card overlay <button> (lines 285-291) also paints above this bar in DOM order, so a full fix needs both a pointer-events override on the icon and for it to be stacked above the overlay button (e.g. relative z-10).

Suggested fix: add pointer-events-auto (and appropriate z-index/stacking) to the trust-lock icon's span so it remains hoverable above the overlay button.


3. src/client/components/LobbyCard.ts:179-194, 285-291 — Card content is announced twice to screen readers

The card restructure moved the click target to a separate, empty overlay <button aria-label=${label}> (lines 285-291), where label (lines 179-194) is a manual concatenation of title/subtitle/modifiers/player count/time/trust status. The visible text conveying that same information now lives in sibling elements (<p>, <h3>, modifier <span>s, player-count <span>, and trustLockIcon's own aria-label/title) that are not marked aria-hidden="true". Previously this text lived inside the clickable <button> itself, forming its accessible name naturally with no duplication. Now a screen reader in browse/virtual-cursor mode will read the visible text layers and then hit the overlay button, which re-announces the identical concatenation as its accessible name — for every card on the homepage and in the detailed lobby browser.

Suggested fix: add aria-hidden="true" to the presentational text containers (modifiers/countdown row, player-count span, and the name-bar div), leaving the overlay <button aria-label=${label}> as the sole node exposed to the accessibility tree.


No CLAUDE.md compliance issues were found: all new user-visible strings go through translateText() with matching entries in resources/lang/en.json, no other locale files were touched, and the only src/core change (queuePosition field in Schemas.ts) is deterministic and covered by a new golden-vector test in tests/zbin/golden.test.ts.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 33cbcfae56

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/client/components/LobbyCard.ts Outdated
@github-actions

Copy link
Copy Markdown

🤖 Claude Code Review

Verdict: Solid PR overall (homepage redesign + rotating lobby scheduler); one low-severity, self-correcting logic bug found. No CLAUDE.md violations found.

Findings by severity: High: 0 · Medium: 0 · Low: 1

File: src/server/MasterLobbyService.ts

Low — queuePosition briefly stamped from a stale rotation cursor (lines 259-261, 379-381)

getAllLobbies() stamps each lobbys queuePosition via promotionOrder(result, this.lastPromoted) (line 259), and promotionOrder uses lastPromoted as the cursor for where the next lap of the rotation starts (rotationFrom(lastPromoted), line 68). However, lastPromoted is only updated when a countdown is observed as live (line 350, inside maybePromote) — not at the moment a lobby is actually promoted and handed a countdown (line 379: this.promoting = { gameID, type };).

maybeScheduleLobby/maybePromote runs on a 1000ms poll while broadcastLobbies (which calls getAllLobbies()) runs on a separate 500ms poll (src/server/MasterLobbyService.ts:173-174). This means there is a window — from the moment a lobby is promoted until the next maybePromote tick observes it as live — during which a broadcastLobbies tick can fire with the old lastPromoted value. Example: rotation is ffa to team to special, lastPromoted === "ffa", and maybePromote promotes a team lobby. Until the next tick sees it live, promotionOrder still computes the lap starting from rotationFrom("ffa"), putting another team lobby at queue position 1 instead of the special lobby that will actually go live next. This briefly mis-orders the "up next" queue card on the homepage (GameModeSelector.allLobbies()) and in the Detailed View (DetailedGameViewFilters.queuePositions()).

Position 0 (the live/counting-down lobby) is always correct — only positions 1+ can be briefly wrong. The window is bounded to at most one polling cycle (about 0.5-1s) and self-corrects once maybePromote next runs.

Suggested fix: set this.lastPromoted = type alongside this.promoting = { gameID, type } at line 379-380, so the cursor advances at promotion time rather than waiting for the lobby to be observed live. This is safe: line 350 re-derives lastPromoted from the live lobby on every subsequent poll anyway, and the existing fallback at line 367 (this.lastPromoted = this.promoting.type;) would simply become a no-op in the case it currently handles.


No other issues found. CLAUDE.md compliance (i18n via translateText()/en.json, src/core determinism and test coverage for the new queuePosition wire field) was checked and is satisfied.

@ryanbarlow97

Copy link
Copy Markdown
Contributor Author

Latest review round addressed — the stale-cursor finding included.

queuePosition stamped from a stale rotation cursor. Confirmed, and fixed as suggested: lastPromoted now advances at promotion time, alongside promoting = { gameID, type }, rather than waiting for the countdown to be observed. promoting is what prevents a second lobby being promoted while one is in flight, so the cursor no longer has to do that job as well. The gone without being reported live branch keeps clearing promoting but no longer needs to move the cursor.

Verified on a running server after the change — three consecutive broadcasts, live lobby first:

0:ffa*     1:team 2:special 3:ffa 4:team 5:special ...
0:team*    1:special 2:ffa 3:team 4:special 5:ffa ...
0:special* 1:ffa 2:team 3:special 4:ffa 5:team ...

Position 1 tracks the next type across the handover, which is what was briefly wrong.

Also in this round, from the inline threads: the queue tooltip stays inside the viewport on phones and is wired to its trigger with aria-describedby; the trust lock keeps its tooltip and passes clicks back to the card; StreamingNow treats the first poll response as loaded, so a failed or stale feed shows the invite instead of leaving the column blank; and GameServer.phase() no longer computes warmupOver from an undefined startsAt.

npm test — 333 files / 3931 tests plus 55 / 576 on the server, exit 0. tsc, oxlint, eslint and Prettier clean.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 02a8b878fa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/server/MasterLobbyService.ts Outdated
Comment on lines +384 to +385
this.lastPromoted = type;
this.promoting = { gameID, type };

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep the in-flight promotion at the front

When a scheduling poll selects a lobby, this advances lastPromoted before the worker has reported the new startsAt; if the independent 500 ms broadcast runs in that window, getAllLobbies() calls promotionOrder(result, this.lastPromoted), which starts after the newly selected type and assigns position 0 to a different lobby. The homepage therefore briefly presents a queued lobby as the hero with no countdown while the actual promoted lobby is elsewhere in the queue, and a user can join the wrong card. Account for promoting.gameID as the live/front lobby until its worker report arrives.

Useful? React with 👍 / 👎.

@github-actions

Copy link
Copy Markdown

🤖 Claude Code Review

Verdict: Needs a fix before merge — one confirmed high-severity logic bug in the new lobby-promotion rotation. No CLAUDE.md violations found.

Findings by severity: 🔴 High: 1 · 🟡 Medium: 0 · 🟢 Low: 0

src/server/MasterLobbyService.ts

🔴 High — maybePromote advances the rotation cursor before the promoted lobby's startsAt is known, so every promotion broadcasts the wrong hero lobby for at least one cycle

// The cursor moves now, not when the countdown is reported: broadcasts run
// on their own poll, and one landing in between would stamp queuePosition
// from the previous lap and put the wrong lobby up next. `promoting` is what
// stops a second lobby being promoted while this one is in flight.
const gameID = queued(type)[0].gameID;
this.lastPromoted = type;
this.promoting = { gameID, type };
this.sendCountdown(gameID);

In maybePromote, this.lastPromoted = type (L383) is set immediately when a lobby is chosen for promotion — before the worker has actually reported back that the lobby's startsAt is set (sendCountdown only sends an updateLobby message to the worker; it doesn't mutate the master's local InternalGameInfo.startsAt). The worker only reports the new startsAt in response to a subsequent lobbiesBroadcast, so the very first broadcast issued after a promotion necessarily goes out before that report lands.

promotionOrder() sorts unstamped (still-startsAt === undefined) lobbies using rotationFrom(lastPromoted), which starts the lap at the type after the cursor — not at the cursor itself. Since lastPromoted was just advanced to the newly-promoted lobby's type, that lobby's own type now sorts toward the back of the first lap, while some other, not-yet-promoted type's lobby is computed as queuePosition === 0.

Concrete trace (rotation ffa → team → special, lastPromoted === "ffa" after ffa's lobby started):

  1. Broadcast before promotion: rotationFrom("ffa") = [team, special, ffa] → position 0 = team-0. Hero card = team-0 (correct, it's about to be promoted).
  2. maybePromote promotes team-0, sets lastPromoted = "team".
  3. Next broadcast (fires before the worker's report of team-0.startsAt arrives): rotationFrom("team") = [special, ffa, team] → position 0 = special-0. Hero card flips to special-0, which has no countdown running, while team-0 (the lobby actually counting down) is pushed to position 2.
  4. Once the report lands, team-0 has startsAt set and moves into the ordered (stamped-first) branch, so position 0 becomes team-0 again.

This isn't a rare race — because the worker can only learn about the new startsAt via a report triggered by a broadcast, the broadcast carrying the stale/wrong order is unavoidable and happens on every single promotion cycle. On the client, GameModeSelector.allLobbies() sorts strictly by queuePosition and treats position 0 as the featured/hero lobby, so this produces a visible, deterministic flicker to the wrong lobby each rotation.

Note the code comment directly above (L378-381) argues the opposite risk ("one [broadcast] landing in between would stamp queuePosition from the previous lap and put the wrong lobby up next") — but the previous lap (i.e., not yet advancing the cursor) is actually the one that ranks the just-promoted lobby correctly at position 0; advancing early is what introduces the bad window.

Suggested fix: don't advance this.lastPromoted until the promoted lobby's countdown is actually observed (i.e., in the existing live !== undefined branch, which already does this.lastPromoted = live.publicGameType), or have promotionOrder special-case this.promoting so the in-flight lobby is treated as if it were already stamped at position 0 while its report is pending.


No other issues found. CLAUDE.md compliance (i18n via translateText() + en.json, src/core determinism/no-deps/test-coverage) was checked independently by two reviewers and both came back clean, and a second independent bug-scan pass found no additional high-confidence issues.

@ryanbarlow97
ryanbarlow97 force-pushed the proto/homepage-dashboard branch from 02a8b87 to 655a4b1 Compare August 31, 2026 13:49
@ryanbarlow97 ryanbarlow97 changed the title feat(homepage): redesign the play surface, and rotate the featured lobby feat(homepage): redesign the play surface Aug 31, 2026
@ryanbarlow97

Copy link
Copy Markdown
Contributor Author

Force-pushed: this PR is now UI only (9 files, client only — no src/server or src/core). Previous head was 02a8b878f.

The server half — the promotion rotation, PublicGameInfo.queuePosition, its golden wire vector, the lobby browser's queue numbering, and the GameServer warmupOver NaN fix — is unchanged and now sits on homepage-rotation, stacked on this branch. Nothing was dropped; I haven't opened a PR for it yet.

One behavioural note for reviewers of this PR: allLobbies() previously ordered the UPCOMING list by the master's queuePosition, which doesn't exist without the server half. It now orders by startsAt — countdowns first, soonest first, then waiting lobbies by gameID. On current main each scheduled type runs its own countdown, so the hero card is still the next game to go live; the list just isn't in true promotion order until the rotation lands.

The two caveats that were flagged in the old description both belonged to the server half and have moved with it: the start-rate change (one game per interval instead of three) and the zbin deploy window for the appended field. Neither applies to this diff.

Both branches: tsc, oxlint, eslint, Prettier clean; full suite green (this branch 339 files / 4030 tests, plus 57 / 588 server; homepage-rotation 4034 and 588).

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 655a4b179a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/client/GameModeSelector.ts Outdated
Comment on lines +197 to +198
const queue = this.allLobbies(this.lobbies);
const [featured, ...rest] = queue;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Rotate the schedule before choosing a single hero

In the inspected production scheduler, MasterLobbyService.maybeScheduleLobby() still iterates over every scheduled type and assigns each bucket's first lobby a countdown in the same poll (src/server/MasterLobbyService.ts:251-264). Therefore this selection does not rotate the featured type: it chooses one of three concurrently counting-down lobbies based on tiny timestamp differences or gameID, while the other live countdowns appear under “Upcoming.” Introduce a single rotating promotion in the scheduler, or keep presenting the independently scheduled buckets rather than treating the first sorted lobby as the sole featured game.

Useful? React with 👍 / 👎.

@github-actions

Copy link
Copy Markdown

🤖 Claude Code Review

Verdict: No issues found — 0 findings (0 critical, 0 major, 0 minor).

Reviewed the diff (resources/lang/en.json, src/client/GameModeSelector.ts, src/client/components/LobbyCard.ts, src/client/components/PlayPage.ts, src/client/components/StreamingNow.ts, and 4 test files) across four independent passes: two focused on CLAUDE.md compliance (i18n/translateText() usage, translation-file scope, src/core purity, testing patterns) and two focused on bugs/logic/security in the introduced code (the allLobbies sort comparator, the StreamingNow loaded/deliveries gating logic, the trust-lock click-forwarding/z-index wiring, and removed-key/removed-event cross-references).

No clear, unambiguous CLAUDE.md violations and no high-confidence bugs were found. All new user-visible strings go through translateText() with matching entries added to en.json; only en.json was touched. The i18n keys removed in this PR (main.detailed_view, mode_selector.teams_count/teams_of/teams_title) were verified to have no other call sites in the repo — their only usages were in the exact GameModeSelector.ts code this PR deletes. public_lobby.starting_game, still used by DetailedGameViewModal.ts, was correctly left in place.

One non-blocking observation, not filed as a finding since no current caller triggers it: LobbyCardOptions.subtitle is typed string | TemplateResult, but the new aria-label builder in LobbyCard.ts does .join(", ") over an array containing it — if a future caller ever passed a TemplateResult subtitle, it would stringify to [object Object] in the accessible name. Both current call sites pass a plain string, so this is worth keeping in mind rather than acting on now.

@ryanbarlow97 ryanbarlow97 removed the Backend Server-side features and systems - lobbies, matchmaking, accounts, APIs, etc. label Aug 31, 2026
@github-actions

Copy link
Copy Markdown

🤖 Claude Code Review

Verdict: No high-signal issues found — approve.

Findings by severity: Critical: 0 · High: 0 · Medium: 0 · Low: 0

Reviewed for CLAUDE.md compliance (i18n via translateText()/en.json, src/core purity, translation-file scope) and for bugs/security issues in the diff (lobby sort comparator, countdown/timer logic, XSS via Lit template bindings, card click-target plumbing, StreamingNow loaded-state tracking, removed i18n keys/event listeners for orphaned references). Two independent CLAUDE.md passes and two independent bug passes all came back clean against the strict bar (compile/parse failures, guaranteed-wrong logic, or unambiguous CLAUDE.md rule violations).

One sub-bar observation surfaced independently by both bug-focused passes, noted here for awareness only (not a finding): in src/client/components/StreamingNow.ts, the matchMedia("(min-width: 1024px)") guard was removed from connectedCallback so streams are now subscribed at all widths, but src/client/components/PlayPage.ts still renders the component with class="hidden lg:flex ...", so on mobile the component polls the streams API while never being visible. Not a correctness bug — just a possibly-unintended minor inefficiency — so it wasn't raised as a finding.

No issues found. Checked for bugs and CLAUDE.md compliance.

…breathe

Three nested radii — the card, the image wrapper inside it and the name bar
at its foot — plus the bar's backdrop-filter were drawing a bright 1-2px
line along the card's corners. The card is now the only rounded, clipping
box, so nothing draws that line. Found by pixel measurement at DPR 1-8.

Modifier pills lose their 65% cap and sort longest first, so on a small card
the labels that say the most are the ones that stay legible.
@ryanbarlow97

Copy link
Copy Markdown
Contributor Author

Cut this back to cosmetics only: one file, one commit, +8/-11. Previous head was e45f510a3.

Dropped from the PR — the redesign, the UPCOMING queue column, the PlayPage grid changes, and the server-side promotion rotation with PublicGameInfo.queuePosition. main's card selection and ordering (FFA as the main card, team and special beside it) are untouched, so none of the wire, master or lobby-browser changes are needed any more. All of it is preserved on homepage-ui, homepage-rotation and t3code/a0aa255a; see the description for what's where.

What remains is the corner-rim fix and the modifier pills. Both are the changes that stood on their own.

@github-actions

Copy link
Copy Markdown

🤖 Claude Code Review

Verdict: No issues found — this is a clean, well-scoped CSS fix. Findings: 0 blocking, 0 high, 0 medium, 0 low.

Review scope

Single file, src/client/components/LobbyCard.ts: (1) moves overflow-hidden/rounded-2xl clipping onto the card <button> and removes the now-redundant radius from the image wrapper and bottom bar, eliminating the double-radius corner rim; (2) reverses the modifier-pill sort to longest-first and drops the max-w-[65%] cap.

What was checked

  • CLAUDE.md compliance — no violations. No user-visible text was added/changed (the i18n rule doesn't apply), and the change is confined to src/client, which carries no test-coverage mandate (unlike src/core).
  • Clipping regression risk — verified that the player-count pill (absolute bottom-full right-2) and the trust-lock icon (absolute bottom-2 right-2), both descendants of the button that now has overflow-hidden, stay well within the button's bounding box at all card heights used by callers (h-44 mobile, homepage grid cell, modal slot). Neither gets clipped.
  • Own box-shadowhover:shadow-[var(--shadow-lobby-card-hover)] is set on the button itself, so the new overflow-hidden on that same element doesn't clip it (overflow only clips descendant content, not an element's own shadow).
  • Sort mutation safetygetModifierLabels() returns a fresh array each render, so the in-place .sort() reversal doesn't mutate shared state.
  • Removed max-w-[65%] — the modifier-pill column keeps min-w-0 and the timer pill is shrink-0, so an unusually long label wraps within the flex layout rather than overflowing or overlapping the timer.

No syntax, type, or logic errors in the diff.

One non-blocking note already called out by the author in the PR description: this diff hasn't had a fresh browser pass (arm64 dev machine, x86-64 headless Chromium), so the corner-rim fix and multi-pill layout are worth a quick visual check before merge — consistent with what's stated in the PR body already.

The two pills on a lobby card's top row carried the same padding, font
size, weight and tracking, but rendered about 4px apart in height.

The difference was the box model, not the classes. The modifier labels
are children of a flex column, so they blockify and take their height
from the line box: 16px for text-xs, plus 8px of padding, 24px total.
The countdown span is a plain inline inside its wrapper, and an inline
box paints its background over the font's content area rather than the
line box. OpenFront.ttf declares ascent 800 and descent -200 against an
upem of 1000, exactly 1.0em, so the countdown pill painted 12px plus
padding, 20px total -- and would have changed height again on the
fallback font, whose content area is nearer 1.2em.

Give both pills one shared class string so they can't drift apart, and
make it inline-block, which takes the countdown's height from the line
box like the modifiers' and stops it depending on which font loaded. The
mt-[2px] nudge on the modifier column went with it: it was compensating
for the inline pill's background bleeding above its wrapper, and with
both boxes blockified items-start aligns them on its own. The pill glow
is dropped from the modifiers rather than added to the countdown, so the
two are identical.

The uppercase toggle on the countdown stays: renderDuration emits
letters, so forcing it would read "1MIN 30S".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions

This comment was marked as resolved.

ryanbarlow97 and others added 2 commits August 31, 2026 16:29
The pills matched in height after the last commit but not in position:
the countdown sat two pixels lower than the modifier labels.

The modifier pills are direct children of the top row's flex container,
so they are blockified and items-start puts their top edge on the row's
top edge. The countdown was wrapped in a block div, which put it on a
line box instead, and the strut of that line -- sized by the card's
larger inherited font, not by text-xs -- has more ascent than the pill
needs, so it pushed the pill down.

Make the countdown span a flex item too, carrying shrink-0 itself, so
both pills are laid out by the same rule.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three lines of CSS trivia for two class strings. Keep the constraint and
what breaks if it is ignored, drop the explanation of why.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

UI/UX UI/UX changes including assets, menus, QoL, etc.

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

2 participants