feat(profile): add barebones achievements (@jonasiwnl)#7865
feat(profile): add barebones achievements (@jonasiwnl)#7865jonasiwnl wants to merge 3 commits intomonkeytypegame:masterfrom
Conversation
github-actions
Bot
added
the
waiting for review
There was a problem hiding this comment.
Pull request overview
Adds a frontend-only “Achievements” section to profile pages by deriving milestone progress from existing UserProfile data, plus targeted unit tests for the derivation logic.
Changes:
- Add
getAchievements()utility + achievement definitions (tests, streak, typing time, level, PB WPM). - Render an Achievements grid on profile pages via
UserProfile. - Add Vitest coverage for unlock behavior, percent clamping, and PB/level derivation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| frontend/src/ts/utils/achievements.ts | New achievement derivation utility + milestone definitions. |
| frontend/src/ts/components/pages/profile/Achievements.tsx | New UI section to display achievement cards + progress bars. |
| frontend/src/ts/components/pages/profile/UserProfile.tsx | Mount Achievements section in the profile page layout. |
| frontend/tests/utils/achievements.spec.ts | New Vitest tests covering derivation/unlock logic. |
| lbOptOut: false, | ||
| streak: 0, | ||
| maxStreak: 0, | ||
| details: {}, |
There was a problem hiding this comment.
getProfile() sets details: {} but UserProfileDetails requires at least bio and keyboard keys (even if empty). With type-aware lint/typecheck this should fail. Either omit details entirely (since optional on UserProfile) or provide { bio: "", keyboard: "" } (plus any other required keys).
| details: {}, | |
| details: { | |
| bio: "", | |
| keyboard: "", | |
| }, |
| const achievements = () => getAchievements(props.profile); | ||
|
|
||
| return ( | ||
| <div class="grid gap-4 rounded bg-sub-alt p-4"> | ||
| <div class="flex items-center justify-between gap-4"> | ||
| <div> | ||
| <div class="text-lg text-text">Achievements</div> | ||
| <div class="text-sm text-sub"> | ||
| A barebones read-only overview of profile milestones. | ||
| </div> | ||
| </div> | ||
| <div class="text-sm text-sub"> | ||
| {achievements().filter((it) => it.unlocked).length}/ | ||
| {achievements().length} unlocked | ||
| </div> | ||
| </div> | ||
| <div class="grid grid-cols-1 gap-3 md:grid-cols-2 xl:grid-cols-3"> | ||
| <For each={achievements()}> | ||
| {(achievement) => <AchievementCard achievement={achievement} />} |
There was a problem hiding this comment.
achievements() recomputes a new array on every call, and it's called multiple times in the same render (filter/length + list). In Solid this can cause unnecessary work and churn (new array identity for <For>). Use createMemo(() => getAchievements(props.profile)) (and derive unlocked count/total from that memo) so it only recalcs when props.profile changes.
| const rawProgress = definition.getProgress(profile); | ||
| const progress = Math.min(rawProgress, definition.target); | ||
| const unlocked = rawProgress >= definition.target; | ||
|
|
||
| return { | ||
| ...definition, | ||
| progress: rawProgress, | ||
| progressPercent: | ||
| definition.target === 0 ? 100 : (progress / definition.target) * 100, | ||
| unlocked, |
There was a problem hiding this comment.
Achievement.progress is set to rawProgress, but progressPercent is computed from a clamped progress value. This makes the API internally inconsistent (a consumer might assume progress matches the percentage/target). Consider returning progress: Math.min(rawProgress, target) and (if needed) a separate rawProgress field for labels/tooltips.
|
Hi @jonasiwnl, thanks for your contribution. This feature already looks very cool. Few suggestions to discuss. UiCurrently it takes up a lot of space. Especially if we add more archivements in the future. We could have a way to either expand the section to show the full summary (like we do for advanced filters on the account page) or open a modal (like we do with show all personal bests). Archivements
@Miodec what do you think? |
Summary
Testing
Notes