Feature: Elo being displayed on the PlayPage (#3029) #3048
Conversation
…eature/3029-elo-display # Conflicts: # resources/lang/en.json # src/client/components/PlayPage.ts
WalkthroughThe pull request adds ELO rating display functionality to the main play page. A new translation key is added, and the PlayPage component is enhanced with state management to listen for user data events, extract the ELO value, and render it in the matchmaking button UI. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Pull request overview
Adds visibility of the player’s ranked ELO directly on the Play page’s “Ranked Matchmaking” button (addresses #3029).
Changes:
- Listen for
userMeResponseonPlayPageand store the player’s 1v1 ELO in reactive state. - Render an “ELO” line (label + value) beneath the Ranked Matchmaking button text.
- Add a new English translation key for the ELO label.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/client/components/PlayPage.ts | Subscribes to userMeResponse and displays the user’s ELO on the matchmaking button. |
| resources/lang/en.json | Adds matchmaking_button.elo label text for the new UI line. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <span> | ||
| <span | ||
| class="relative z-10 text-xs font-medium text-purple-100 opacity-90 group-hover:opacity-100 transition-opacity" | ||
| data-i18n="matchmaking_button.elo" | ||
| ></span> |
There was a problem hiding this comment.
The ELO label/value is split across two s and the translation string includes formatting ("ELO: ") to create spacing. This makes localization harder (languages may need to reorder or format the whole "ELO: {value}" differently) and relies on trailing whitespace in the translation.
Consider rendering a single localized string with an ICU placeholder (similar to matchmaking_modal.elo in src/client/Matchmaking.ts) and passing the current elo as a param, or otherwise avoid encoding spacing/punctuation in the translation.
| }, | ||
| "matchmaking_button": { | ||
| "play_ranked": "1v1 Ranked Matchmaking", | ||
| "elo": "ELO: ", |
There was a problem hiding this comment.
matchmaking_button.elo has a trailing space ("ELO: "). Trailing whitespace in translations is fragile and makes it harder for translators/tools to manage spacing/punctuation.
Prefer removing the trailing space and handling spacing in markup/CSS, or switch this string to a placeholder format (e.g., "ELO: {elo}") and render it as a single translated string.
| "elo": "ELO: ", | |
| "elo": "ELO:", |
There was a problem hiding this comment.
please fix, instead use vairables "ELO: {elo}"
| @state() private elo: string = "..."; | ||
| private _onUserMeResponse: (e: Event) => void; |
There was a problem hiding this comment.
PR description indicates relevant tests were added, but this change set only updates PlayPage.ts and en.json with no corresponding test changes. If the intent is to add coverage, please include a client/component test that verifies the ELO text updates when a userMeResponse event is dispatched.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/client/components/PlayPage.ts`:
- Around line 10-26: PlayPage.connectedCallback currently only sets this.elo in
the userMeResponse event handler (_onUserMeResponse) and misses the value if the
event already fired; update the flow so PlayPage reads a cached last userMe
response from Main before attaching the listener: add a simple cache accessor in
Main (e.g., store the last response when getUserMe resolves and expose
getLastUserMeResponse or a module-level variable) and in
PlayPage.connectedCallback call that getter (or read the exported cached value)
to initialize this.elo the same way as in _onUserMeResponse (using
event.detail.player.leaderboard.oneVone.elo) as a fallback, then attach
document.addEventListener for future events.
🧹 Nitpick comments (2)
resources/lang/en.json (1)
971-971: Drop the trailing space in the ELO label.Line 971 ends with a space. Trailing spaces are easy to lose and make spacing hard to control. Prefer
ELO:and add the gap in markup.♻️ Proposed change
- "elo": "ELO: ", + "elo": "ELO:"src/client/components/PlayPage.ts (1)
189-199: Add spacing in markup instead of relying on label text.The label and value are separate spans, but spacing currently depends on the translation string. Add a small gap in markup so translators don’t need trailing spaces.
♻️ Proposed change
- <span> + <span class="inline-flex items-center gap-1">
| connectedCallback() { | ||
| super.connectedCallback(); | ||
|
|
||
| this._onUserMeResponse = (e: Event): void => { | ||
| const event = e as CustomEvent<UserMeResponse | false>; | ||
|
|
||
| if (!event.detail) { | ||
| this.elo = "..."; | ||
| return; | ||
| } | ||
|
|
||
| this.elo = | ||
| event.detail?.player?.leaderboard?.oneVone?.elo?.toString() ?? "..."; | ||
| }; | ||
|
|
||
| document.addEventListener("userMeResponse", this._onUserMeResponse); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'userMeResponse' --type ts --type js -C 3Repository: openfrontio/OpenFrontIO
Length of output: 17824
🏁 Script executed:
rg -n 'dispatchEvent' --type ts --type js | grep -i 'usermeresp\|userme'Repository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
fd -e ts -e js | xargs grep -l 'dispatchEvent.*userMeResponse\|new CustomEvent.*[Uu]serMe'Repository: openfrontio/OpenFrontIO
Length of output: 85
🏁 Script executed:
rg -n 'onUserMe\s*=' src/client/Main.ts -A 30 | head -80Repository: openfrontio/OpenFrontIO
Length of output: 1247
🏁 Script executed:
rg -n 'onUserMe\s*\(' src/client/Main.ts -B 5 -A 2Repository: openfrontio/OpenFrontIO
Length of output: 246
🏁 Script executed:
sed -n '440,550p' src/client/Main.tsRepository: openfrontio/OpenFrontIO
Length of output: 3619
🏁 Script executed:
rg -n 'getUserMe' src/client/Main.ts -B 2 -A 5Repository: openfrontio/OpenFrontIO
Length of output: 641
🏁 Script executed:
rg -n 'window.showPage\|page-matchmaking' src/client/Main.ts | head -20Repository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
find src -name "*.ts" -type f | xargs grep -l 'page-matchmaking' | head -5Repository: openfrontio/OpenFrontIO
Length of output: 111
🏁 Script executed:
cat -n src/client/Api.ts | grep -A 10 'export function getUserMe'Repository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
sed -n '1,50p' src/client/PlayPage.tsRepository: openfrontio/OpenFrontIO
Length of output: 132
🏁 Script executed:
find src -name "*PlayPage*" -o -name "*playpage*" -o -name "*play-page*"Repository: openfrontio/OpenFrontIO
Length of output: 100
🏁 Script executed:
rg -l 'class PlayPage' --type tsRepository: openfrontio/OpenFrontIO
Length of output: 100
🏁 Script executed:
rg 'export function getUserMe' -A 15Repository: openfrontio/OpenFrontIO
Length of output: 49
🏁 Script executed:
rg 'getUserMe' src/client/Api.ts -A 15Repository: openfrontio/OpenFrontIO
Length of output: 512
🏁 Script executed:
cat -n src/client/components/PlayPage.ts | head -50Repository: openfrontio/OpenFrontIO
Length of output: 1947
Read ELO from cached state on connect, not just from event listener.
The userMeResponse event is dispatched asynchronously via getUserMe().then(onUserMe) in Main.ts line 510. If PlayPage connects after this event fires, the listener will miss it and elo stays as "...". Store the last userMeResponse in Main.ts and read it in PlayPage's connectedCallback as fallback.
🤖 Prompt for AI Agents
In `@src/client/components/PlayPage.ts` around lines 10 - 26,
PlayPage.connectedCallback currently only sets this.elo in the userMeResponse
event handler (_onUserMeResponse) and misses the value if the event already
fired; update the flow so PlayPage reads a cached last userMe response from Main
before attaching the listener: add a simple cache accessor in Main (e.g., store
the last response when getUserMe resolves and expose getLastUserMeResponse or a
module-level variable) and in PlayPage.connectedCallback call that getter (or
read the exported cached value) to initialize this.elo the same way as in
_onUserMeResponse (using event.detail.player.leaderboard.oneVone.elo) as a
fallback, then attach document.addEventListener for future events.
github-project-automation
bot
moved this from Triage
to Development
in OpenFront Release Management
|
The trailing whitespace after the "ELO:" in en.json is with intent |
| }, | ||
| "matchmaking_button": { | ||
| "play_ranked": "1v1 Ranked Matchmaking", | ||
| "elo": "ELO: ", |
There was a problem hiding this comment.
please fix, instead use vairables "ELO: {elo}"
There was a problem hiding this comment.
I think all this logic should be in Matchmaking.ts
There was a problem hiding this comment.
Fair point. I'll fix it tomorrow. I don't have any time today sadly.
There was a problem hiding this comment.
The button it is attached to is located in PlayPage.ts. I would have to migrate him to Matchmaking first, unless I am not missing out on something here.
github-project-automation
bot
moved this from Development
to Complete
in OpenFront Release Management
2 participants
If this PR fixes an issue, link it below. If not, delete these two lines.
Resolves #3029
Description:
Your ELO is now being displayed on the Matchmaking button.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
@xTonai
UI updates