Add Public Achivements UI#3530
Open
Aotumuri wants to merge 7 commits into
Open
Conversation
|
|
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds player achievements: new English translations and metadata, API schema changes to support mixed achievement arrays, a new Changes
Sequence DiagramsequenceDiagram
participant User as User
participant Modal as AccountModal
participant Component as PlayerAchievements
participant Metadata as PlayerAchievementMetadata
participant Formatter as Intl.DateTimeFormat
User->>Modal: Open account modal
Modal->>Modal: Read userMeResponse.player.achievements
Modal->>Component: Pass achievementGroups
Component->>Component: Filter for type == "player" and flatten unlocked items
Component->>Component: Merge keys with metadata and sort by achievedAt (desc)
Component->>Metadata: Lookup achievement metadata (difficulty, desc)
Metadata-->>Component: Return metadata (or missing)
Component->>Formatter: Format achievedAt timestamp
Formatter-->>Component: Return formatted date (or raw)
Component-->>User: Render achievement cards or empty state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/client/AccountModal.ts (1)
179-186: Consider adding an emoji for visual consistency with other sections.The Stats Overview section uses
📊and Recent Games uses🎮. The Achievements section heading lacks an icon, which creates visual inconsistency.✨ Suggested enhancement
<div class="bg-white/5 rounded-xl border border-white/10 p-6"> - <h3 class="text-lg font-bold text-white mb-4"> + <h3 + class="text-lg font-bold text-white mb-4 flex items-center gap-2" + > + <span class="text-blue-400">🏆</span> ${translateText("account_modal.achievements")} </h3>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/AccountModal.ts` around lines 179 - 186, Add an emoji to the Achievements header to match other sections: update the string passed to translateText("account_modal.achievements") (or prepend the emoji when rendering in the h3) so the heading includes an icon (e.g., "🏆") and keep the rest of the markup unchanged (the h3 element containing translateText and the <player-achievements .achievementGroups=${achievements}> component).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@resources/lang/en.json`:
- Around line 245-248: Rename the misspelled translation object "achivements" to
"achievements" in en.json and update all lookup sites to use the corrected key;
specifically change the en.json top-level key from "achivements" to
"achievements" and update the translation lookups in PlayerAchievements.ts (the
lines that build the translation key using achievementKey) from
"achivements.${achievementKey}" to "achievements.${achievementKey}" so the i18n
lookup uses the corrected path.
In `@src/client/components/baseComponents/stats/PlayerAchievements.ts`:
- Around line 97-107: In resolveTitle and resolveDescription change the
translation key prefix from "achivements." to the correct "achievements." (i.e.,
update the template strings in resolveTitle(achievementKey) and
resolveDescription(achievementKey)); also update the corresponding keys in the
en.json translations to use "achievements" instead of "achivements" so the
translateText lookups resolve correctly.
---
Nitpick comments:
In `@src/client/AccountModal.ts`:
- Around line 179-186: Add an emoji to the Achievements header to match other
sections: update the string passed to
translateText("account_modal.achievements") (or prepend the emoji when rendering
in the h3) so the heading includes an icon (e.g., "🏆") and keep the rest of the
markup unchanged (the h3 element containing translateText and the
<player-achievements .achievementGroups=${achievements}> component).
🪄 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: ba849722-0822-4752-a33d-f23a7b060909
📒 Files selected for processing (5)
resources/lang/en.jsonresources/playerAchievementMetadata.jsonsrc/client/AccountModal.tssrc/client/components/baseComponents/stats/PlayerAchievements.tssrc/core/ApiSchemas.ts
github-project-automation
Bot
moved this from Triage
to Development
in OpenFront Release Management
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/components/baseComponents/stats/PlayerAchievements.ts`:
- Around line 25-31: Remove the dev-only fabricated achievement from the shipped
UI by deleting the MOCK_UNLOCKED_TEST_ACHIEVEMENT constant and any code that
injects it (or alternatively filter out entries where achievement === "test") in
PlayerAchievements.ts; specifically, remove uses that merge or default API
results with MOCK_UNLOCKED_TEST_ACHIEVEMENT (also at the other instances noted
around lines 46-54 and 69-75) so the component only displays real
playerAchievement data from the API or filtered metadata.
🪄 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: f3349f0e-8bb5-411d-8337-6c0b4e534599
📒 Files selected for processing (3)
resources/lang/en.jsonresources/playerAchievementMetadata.jsonsrc/client/components/baseComponents/stats/PlayerAchievements.ts
✅ Files skipped from review due to trivial changes (1)
- resources/lang/en.json
🚧 Files skipped from review as they are similar to previous changes (1)
- resources/playerAchievementMetadata.json
Aotumuri
force-pushed
the
public-achivements-ui
branch
from
d3952c0 to
3c4d1f9
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/client/components/baseComponents/stats/PlayerAchievements.ts (1)
25-31:⚠️ Potential issue | 🔴 CriticalRemove the shipped
testachievement from the public UI.This component still invents a
testunlock and seeds cards from every metadata key, so the public view can show data the API never returned. Please drop the mock and filtertesthere until it is removed from metadata.🧹 Minimal fix
-const MOCK_UNLOCKED_TEST_ACHIEVEMENT = { - playerId: "0", - achievement: "test", - achievedAt: "2025-01-01T00:00:00.000Z", - gameId: "0", - game: "ui-test", -} satisfies PlayerAchievementJson; @@ private get unlockedAchievements(): PlayerAchievementJson[] { - const unlockedAchievements = this.achievementGroups + return this.achievementGroups .flatMap((group) => (group.type === "player" ? group.data : [])) - .slice(); - - if ( - !unlockedAchievements.some( - (achievement) => - achievement.achievement === - MOCK_UNLOCKED_TEST_ACHIEVEMENT.achievement, - ) - ) { - unlockedAchievements.push(MOCK_UNLOCKED_TEST_ACHIEVEMENT); - } - - return unlockedAchievements.sort( + .filter((achievement) => achievement.achievement !== "test") + .sort( (a, b) => new Date(b.achievedAt).getTime() - new Date(a.achievedAt).getTime(), - ); + ); } @@ - const knownKeys = Object.keys(playerAchievementMetadata); + const knownKeys = Object.keys(playerAchievementMetadata).filter( + (achievement) => achievement !== "test", + );Also applies to: 46-54, 69-75
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/components/baseComponents/stats/PlayerAchievements.ts` around lines 25 - 31, Remove the shipped test achievement and ensure it isn't surfaced in the UI by deleting the MOCK_UNLOCKED_TEST_ACHIEVEMENT constant and any code that seeds cards from it; in the PlayerAchievements component remove usages of MOCK_UNLOCKED_TEST_ACHIEVEMENT and when iterating metadata keys (the code that maps over achievement metadata) explicitly filter out entries where key === "test" so only real API-returned achievements are rendered.
🧹 Nitpick comments (1)
src/client/components/baseComponents/stats/PlayerAchievements.ts (1)
62-103: Add a small test matrix aroundPlayerAchievements.achievements.This getter controls the whole card order. Please cover unlocked before locked, newest unlocked first, unknown unlocked keys, and empty input.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/client/components/baseComponents/stats/PlayerAchievements.ts` around lines 62 - 103, Add unit tests for the PlayerAchievements.achievements getter to cover the ordering behaviors: verify unlocked cards appear before locked ones and that among unlocked entries newest (highest achievedAt) comes first, ensure unknown achievement keys present in unlockedAchievements are included and ordered using originalOrder fallback from playerAchievementMetadata, and include a test for empty unlockedAchievements returning metadata-only ordered list; reference the PlayerAchievements.achievements getter, the unlockedAchievements array, playerAchievementMetadata, and the resulting PlayerAchievementCard objects when asserting ordering and isUnlocked/achievedAt values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/client/components/baseComponents/stats/PlayerAchievements.ts`:
- Around line 25-31: Remove the shipped test achievement and ensure it isn't
surfaced in the UI by deleting the MOCK_UNLOCKED_TEST_ACHIEVEMENT constant and
any code that seeds cards from it; in the PlayerAchievements component remove
usages of MOCK_UNLOCKED_TEST_ACHIEVEMENT and when iterating metadata keys (the
code that maps over achievement metadata) explicitly filter out entries where
key === "test" so only real API-returned achievements are rendered.
---
Nitpick comments:
In `@src/client/components/baseComponents/stats/PlayerAchievements.ts`:
- Around line 62-103: Add unit tests for the PlayerAchievements.achievements
getter to cover the ordering behaviors: verify unlocked cards appear before
locked ones and that among unlocked entries newest (highest achievedAt) comes
first, ensure unknown achievement keys present in unlockedAchievements are
included and ordered using originalOrder fallback from
playerAchievementMetadata, and include a test for empty unlockedAchievements
returning metadata-only ordered list; reference the
PlayerAchievements.achievements getter, the unlockedAchievements array,
playerAchievementMetadata, and the resulting PlayerAchievementCard objects when
asserting ordering and isUnlocked/achievedAt values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cbae4f6d-b906-4921-86e8-7c351a466a53
📒 Files selected for processing (3)
resources/lang/en.jsonresources/playerAchievementMetadata.jsonsrc/client/components/baseComponents/stats/PlayerAchievements.ts
✅ Files skipped from review due to trivial changes (2)
- resources/playerAchievementMetadata.json
- resources/lang/en.json
Aotumuri
force-pushed
the
public-achivements-ui
branch
from
d235f11 to
f6bc47b
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #3144
Description:
Adds a new achievements UI for the player profile.
Please complete the following:
Please put your Discord username so you can be contacted if a bug or regression is found:
aotumuri