Skip to content

fix(leaderboard): drop clan tags from 1v1 ranked and align the pinned… - #4713

Merged
Celant merged 5 commits into
mainfrom
ranked-leaderboard-no-clan-tag
Jul 28, 2026
Merged

fix(leaderboard): drop clan tags from 1v1 ranked and align the pinned…#4713
Celant merged 5 commits into
mainfrom
ranked-leaderboard-no-clan-tag

Conversation

@ryanbarlow97

@ryanbarlow97 ryanbarlow97 commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Description:

Two changes to the 1v1 Ranked leaderboard.

Clan tags removed

1v1 ranked is an individual ladder, and the tag the API returns is whichever one the player last happened to use in any game mode — it says nothing about the ranked results being listed. Dropped from the rows, from the pinned "your ranking" bar, and from the view model (PlayerLeaderboardEntry, which is used only by this component).

The pinned bar now lines up with the table

It was an overlay laid out by hand — flex boxes with px-6 / ml-4 / w-20 — instead of following the table it sits under. Measured in a headless browser at 1600px wide:

Before After
Player name vs Player column 25px to the left aligned
Bar height vs row height 73px vs 65px 65px vs 65px
Elo value far right, under the Win/Loss header under Elo

Hand-picked padding cannot fix this: the table is table-fixed w-full, so the resolved column widths grow with the modal and no fixed offset matches at more than one width.

It is now the table's own sticky <tfoot>, mirroring the <thead> that is already sticky. Sharing the table means the columns cannot drift, and the row follows horizontal scrolling — which matters below the colgroup's 34rem total, where the table scrolls sideways and an overlay outside .scroll-container did not move with it. At 420px wide, scrolling fully right desynced the pinned row by 126px and pushed its Win/Loss cell to x=545 against a container edge of 419, where it could not be reached at all. Thanks to the Codex bot for catching that one.

Being in flow also removes the pb-16 spacer that existed only to keep the overlay off the last row.

Verified in a headless browser: columns align to the pixel at 420 / 700 / 1600px wide, both unscrolled and scrolled fully right; the row stays pinned to the container bottom mid-scroll and at the end; and it is exactly one row tall.

The container's bottom border is dropped so the pinned row sits flush against that edge. That border is unchanged from main, but the row is now solid bg-blue-600 instead of bg-blue-600/90 over a backdrop blur — backdrop-filter on a table row is unreliable and a sticky row has to be opaque — and against the brighter blue the dark hairline read as a stray outline. Its bottom inset against the container measures 0px now, against 1px before.

One judgment call worth a look: now that the columns line up, the bar also fills the Games and Win/Loss cells. Aligning the columns and then leaving two of them blank looked worse than filling them, and every value is already in currentUserEntry. Easy to blank them again if you'd rather.

before:
had a double hbar?
image

columns didnt align:
image

after:

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

No new user-facing strings — the bar reuses the existing leaderboard_modal.your_ranking, leaderboard_modal.elo and leaderboard_modal.ratio keys, so en.json is unchanged.

Please put your Discord username so you can be contacted if a bug or regression is found:

w.o.n

@ryanbarlow97 ryanbarlow97 added this to the v33 milestone Jul 24, 2026
@ryanbarlow97 ryanbarlow97 self-assigned this Jul 24, 2026
Copilot AI review requested due to automatic review settings July 24, 2026 23:05
@ryanbarlow97
ryanbarlow97 requested a review from a team as a code owner July 24, 2026 23:05
@ryanbarlow97 ryanbarlow97 added the UI/UX UI/UX changes including assets, menus, QoL, etc. label Jul 24, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@coderabbitai

coderabbitai Bot commented Jul 24, 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

The ranked 1v1 leaderboard no longer exposes or displays clan tags. Its sticky ranking entry is now rendered as a pinned table-footer row, with related overlay and spacing logic removed. Tests verify that API-provided clan tags are discarded.

Changes

Leaderboard update

Layer / File(s) Summary
Remove clan tags from leaderboard data
src/core/ApiSchemas.ts, src/client/components/leaderboard/LeaderboardPlayerList.ts, tests/client/LeaderboardModal.test.ts
The leaderboard schema and client mapping omit clanTag; tests verify that API-provided tags are absent from mapped data and rendered text.
Render the sticky ranking row in the table
src/client/components/leaderboard/LeaderboardPlayerList.ts
The current user entry is rendered in a <tfoot> row, while the former overlay, clan-tag badge, and related bottom padding are removed.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

Poem

Clan tags drift away,
The footer holds rank in place,
Rows share one clear shape,
Clean data fills the table,
Tests guard every space.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the main change: removing clan tags from 1v1 ranked and aligning the pinned leaderboard row.
Description check ✅ Passed The description clearly covers the clan-tag removal and the pinned bar layout change described in the diff.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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.

chatgpt-codex-connector[bot]

This comment was marked as resolved.

coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 24, 2026
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 24, 2026
… row

1v1 ranked is an individual ladder, and the tag the API reports is
whichever one the player last happened to use in any game mode — so it
does not belong on this board. Removed from the rows, from the pinned
"your ranking" bar, and from the view model, which is used only by this
component.

The pinned bar was also an overlay laid out by hand (flex boxes with
px-6/ml-4/w-20) rather than following the table, so it never lined up with
the rows it sits under: the player name was 25px left of the Player column
and the bar was 8px taller than a row, while the Elo value floated at the
far right under the Win/Loss header. Hand-picked padding cannot fix this —
the table is `table-fixed w-full`, so the resolved column widths grow with
the modal and no fixed offset matches at more than one size.

It is now the table's own sticky <tfoot>, mirroring the existing sticky
<thead>. Sharing the table means the columns cannot drift and the row
follows horizontal scrolling: below the colgroup's 34rem the table scrolls
sideways, and an overlay outside .scroll-container did not move with it —
at 420px wide, scrolling right desynced the pinned row by 126px and pushed
its Win/Loss cell to x=545 against a container edge of 419, where it could
not be reached. Being in flow also removes the pb-16 spacer that kept the
overlay off the last row.

The container's bottom border is gone so the pinned row sits flush against
that edge. The border is unchanged from main, but the row is now solid
bg-blue-600 rather than bg-blue-600/90 over a backdrop blur — backdrop
filters on a table row are unreliable and a sticky row has to be opaque —
and the brighter blue turned that dark hairline into an apparent outline.

Measured in a headless browser: columns align to the pixel at 420/700/1600
wide, before and after scrolling fully right; the row stays pinned to the
container bottom mid-scroll and at the end; it is exactly one row tall; and
its bottom inset against the container is now 0px.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
coderabbitai[bot]
coderabbitai Bot previously approved these changes Jul 24, 2026
@ryanbarlow97
ryanbarlow97 marked this pull request as ready for review July 25, 2026 06:04

@Celant Celant left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed the diff and verified the structural claims against main. All 13 checks are green.

The reasoning in the description holds up under checking, which is worth saying explicitly:

  • The schema removal is safe. PlayerLeaderboardEntry really does have no consumer outside this component — the only other file mentioning it is ApiSchemas.ts where it's declared. And removing this one usage doesn't orphan RequiredClanTagSchema; it still has five other usages across ApiSchemas.ts and ClanApiSchemas.ts.
  • The column structure is consistent. colgroup 5 / thead 5 / tbody 5 / tfoot 5, and the colgroup's 4+12+6+6+6 does come to the 34rem quoted in the description. Nothing to drift.
  • Moving to a <tfoot> is the right call for the reason given — table-fixed w-full resolves column widths against the container, so no separately-positioned element can track them across sizes. The horizontal-scroll case is the one that makes it not merely tidier but actually correct.

Two things below: one layout consequence I think is a small regression, and a missing test. Neither is blocking.

Comment thread src/client/components/leaderboard/LeaderboardPlayerList.ts Outdated
Comment thread tests/client/LeaderboardModal.test.ts
Comment thread src/core/ApiSchemas.ts
@github-project-automation github-project-automation Bot moved this from Triage to Final Review in OpenFront Release Management Jul 28, 2026
@Celant
Celant added this pull request to the merge queue Jul 28, 2026
Merged via the queue into main with commit ad3fcbe Jul 28, 2026
13 checks passed
@Celant
Celant deleted the ranked-leaderboard-no-clan-tag branch July 28, 2026 17:57
@github-project-automation github-project-automation Bot moved this from Final Review to Complete in OpenFront Release Management Jul 28, 2026
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.

3 participants