Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds an Achievements section to the SvelteKit UI, including a new achievements route, reusable achievement display components, and supporting API/types and navigation updates.
Changes:
- Add
/achievementspage that loads the current user and renders an achievements UI. - Introduce achievement UI components (service grouping accordion, achievement rows, edit modal) plus supporting icons/labels.
- Extend shared frontend types/API helpers for achievements and update Navbar + page backgrounds.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| ui/src/routes/profile/[username]/+page.svelte | Adds a white background to the profile page container. |
| ui/src/routes/+page.svelte | Adds a white background to the home page container. |
| ui/src/routes/achievements/+page.svelte | New achievements route that fetches current user and renders achievements display. |
| ui/src/lib/globalFunctions-Types.ts | Adds admin flag to CurrentUser, achievement-related types, and two new fetch helpers. |
| ui/src/lib/components/Navbar.svelte | Adds Achievements nav link and makes username link to profile. |
| ui/src/lib/components/achievements/AchievementDisplay.svelte | Fetches services and renders grouped achievement UI + edit modal hook. |
| ui/src/lib/components/achievements/AchievementServiceGroup.svelte | Accordion group per service; fetches achievements per service and supports edit action. |
| ui/src/lib/components/achievements/Achievement.svelte | Renders a single achievement with goal checklist and progress bar. |
| ui/src/lib/components/achievements/AchievementEditModal.svelte | New modal scaffold for editing/adding services. |
| ui/src/lib/components/CheckedLabel.svelte | New label component that shows a check/cross icon based on a boolean. |
| ui/src/lib/components/icons/AchievementIcon.svelte | New icon for achievements navigation. |
| ui/src/lib/components/icons/ArrowNoBaseIcon.svelte | New arrow icon used in service accordion header. |
| ui/src/lib/components/icons/CheckIcon.svelte | New checkmark icon used in CheckedLabel. |
| ui/src/lib/components/icons/CrossIcon.svelte | New cross icon used in CheckedLabel. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <div class="flex flex-row justify-end w-full mt-10"> | ||
| <button class="bg-orange-200 hover:bg-orange-300 px-4 py-2 rounded-md font-semibold text-orange-900" | ||
| onclick={editModal?.open} | ||
| > | ||
| Add Service | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
The "Add Service" button is rendered for all users, but the modal only exists when admin is true. For non-admins the button becomes a no-op, and for admins it can still be clickable before bind:this assigns editModal. Consider rendering this button only when admin is true (or disabling it until editModal is bound).
| <div class="flex flex-row justify-end w-full mt-10"> | |
| <button class="bg-orange-200 hover:bg-orange-300 px-4 py-2 rounded-md font-semibold text-orange-900" | |
| onclick={editModal?.open} | |
| > | |
| Add Service | |
| </button> | |
| </div> | |
| {#if admin} | |
| <div class="flex flex-row justify-end w-full mt-10"> | |
| <button | |
| class="bg-orange-200 hover:bg-orange-300 px-4 py-2 rounded-md font-semibold text-orange-900" | |
| disabled={!editModal} | |
| onclick={() => { if (editModal) editModal.open(); }} | |
| > | |
| Add Service | |
| </button> | |
| </div> | |
| {/if} |
| <a class="ml-4 text-lg -my-2 px-3 h-12 inline-flex items-center justify-center hover:bg-amber-600" | ||
| href="/achievements"> | ||
| <span class=" hidden md:inline-block"> | ||
| Achievements | ||
| </span> | ||
| <span class="size-7 inline-block md:hidden"> | ||
| <AchievementIcon /> | ||
| </span> | ||
| </a> |
There was a problem hiding this comment.
On small screens the "Achievements" link hides its text (hidden md:inline-block) and only shows an icon. That leaves the link without an accessible name for screen readers unless the icon provides one. Add an aria-label on the <a> or include an sr-only text label so the link is still announced correctly.
| <div role="button" | ||
| class="flex flex-row justify-between items-center w-full py-4 font-bold text-lg md:text-2xl" | ||
| onclick={toggle} | ||
| onkeydown={(ev) => keyClickHandler(ev, toggle)} | ||
| tabindex="0" | ||
| > | ||
| <span> | ||
| {toTitleCase(service.name)} | ||
| {#if editAllowed} | ||
| <button class="cursor-pointer rounded-md text-orange-900 bg-orange-200 hover:bg-orange-300 p-1 mx-2" | ||
| onclick={openEdit} | ||
| onkeydown={(ev) => keyClickHandler(ev, openEdit)}> | ||
| <span class="flex justify-center items-center size-4"> | ||
| <PencilIcon /> | ||
| </span> | ||
| </button> |
There was a problem hiding this comment.
openEdit() both toggles the accordion and is triggered from a <button> nested inside a parent element that also has onclick={toggle}. Because the button click bubbles, toggle() is called twice (once in openEdit, once in the parent), leaving isOpen unchanged and causing flaky behavior. Stop propagation on the edit button click/keydown, or remove the toggle() call from openEdit and manage accordion state explicitly when opening the modal.
|
|
||
| function openEdit() { | ||
| toggle(); | ||
| editModal.open(); |
There was a problem hiding this comment.
editModal.open() is called without guarding against editModal being undefined. Even when editAllowed is true, editModal can be undefined briefly (e.g., before the modal component binds) and this will throw at runtime. Consider using optional chaining (editModal?.open()), or require/validate that editModal is always provided when editAllowed is true.
| editModal.open(); | |
| editModal?.open(); |
| > | ||
| <h3 class="text-lg leading-6 font-medium text-gray-900">Edit Service</h3> | ||
| <div class="mt-2 flex flex-row justify-center"> | ||
| // Middle Content |
There was a problem hiding this comment.
// Middle Content is written in the markup, which Svelte will treat as literal text (not a comment). Replace with a proper HTML comment (<!-- ... -->) or remove it, otherwise it will render in the dialog body.
| // Middle Content | |
| <!-- Middle Content --> |
|
|
||
| let { achievement } = $props(); | ||
|
|
||
| let goalsUnlocked = []; | ||
| for (let i = 0; i < achievement.goals.length; i++) { | ||
| goalsUnlocked.push(Math.random() >= 0.5); | ||
| } | ||
|
|
||
| let imgSrc = 'https://cdn.fastly.steamstatic.com/steamcommunity/public/images/apps/1903340/893a5719f74928a4706ad295b4ab42cf0a2ffacb.jpg'; | ||
|
|
||
| const achievementUnlocked = () => goalsUnlocked.every(g => g); | ||
|
|
||
| const progressBarStyle = `width: ${goalsUnlocked.filter(g => g).length * 100 / goalsUnlocked.length}%`; |
There was a problem hiding this comment.
goalsUnlocked is derived using Math.random() during component initialization. With SvelteKit SSR enabled, this will produce different markup on the server vs client and can cause hydration mismatches, plus it makes the UI nondeterministic. Compute progress from real data, or generate client-only state in onMount (or otherwise ensure deterministic rendering across SSR/CSR).
| let { achievement } = $props(); | |
| let goalsUnlocked = []; | |
| for (let i = 0; i < achievement.goals.length; i++) { | |
| goalsUnlocked.push(Math.random() >= 0.5); | |
| } | |
| let imgSrc = 'https://cdn.fastly.steamstatic.com/steamcommunity/public/images/apps/1903340/893a5719f74928a4706ad295b4ab42cf0a2ffacb.jpg'; | |
| const achievementUnlocked = () => goalsUnlocked.every(g => g); | |
| const progressBarStyle = `width: ${goalsUnlocked.filter(g => g).length * 100 / goalsUnlocked.length}%`; | |
| import { onMount } from 'svelte'; | |
| let { achievement } = $props(); | |
| let goalsUnlocked: boolean[] = Array.from( | |
| { length: achievement.goals.length }, | |
| () => false | |
| ); | |
| onMount(() => { | |
| // Randomize goals on the client only, to avoid SSR/CSR mismatches | |
| goalsUnlocked = goalsUnlocked.map(() => Math.random() >= 0.5); | |
| }); | |
| let imgSrc = 'https://cdn.fastly.steamstatic.com/steamcommunity/public/images/apps/1903340/893a5719f74928a4706ad295b4ab42cf0a2ffacb.jpg'; | |
| const achievementUnlocked = () => goalsUnlocked.every(g => g); | |
| $: progressBarStyle = `width: ${ | |
| goalsUnlocked.length === 0 | |
| ? 0 | |
| : (goalsUnlocked.filter(g => g).length * 100) / goalsUnlocked.length | |
| }%`; |
No description provided.