forgejo_admin/westside-landing
1
0
Fork 0
Code Issues 13 Pull requests 2 Projects Releases Packages Wiki Activity Actions

fix: DRY Keycloak URLs, add coach Account link, a11y #126

Merged
forgejo_admin merged 1 commit from 69-cleanup-keycloak-console-link-nits into main 2026-03-28 00:07:03 +00:00
Conversation 1 Commits 1 Files changed 3 +13 -6
forgejo_admin commented 2026-03-27 22:11:21 +00:00
Owner
Copy link

Summary

  • QA nits from PR #68: exports Keycloak URL constants so consumers import instead of hardcoding
  • Adds Account nav link for coaches (previously only player/parent had it)
  • Replaces inline style with utility class and adds aria-labels on external links

Changes

  • src/lib/keycloak.js — export KEYCLOAK_URL and REALM constants (were module-private)
  • src/routes/(app)/+layout.svelte — import constants, build Account URL from them for both player/parent and coach navs, add aria-label on external links
  • src/routes/(app)/admin/+page.svelte — import constants, replace hardcoded Keycloak admin console URL, replace style="margin-top: 0.5rem;" with mt-sm utility class, add aria-label on external link

Test Plan

  • npm run build passes
  • Coach bottom nav shows Account link (gear icon) between Team and Sign Out
  • Player/parent Account link unchanged functionally, now uses imported constants
  • Admin dashboard Keycloak link unchanged functionally, now uses imported constants
  • All external links have aria-label indicating new tab

Review Checklist

  • Passed automated review-fix loop
  • No secrets committed
  • No unnecessary file changes
  • Commit messages are descriptive

Related Notes

  • Closes #69
  • westside-app #68 — parent PR that introduced these nits
## Summary - QA nits from PR #68: exports Keycloak URL constants so consumers import instead of hardcoding - Adds Account nav link for coaches (previously only player/parent had it) - Replaces inline style with utility class and adds aria-labels on external links ## Changes - `src/lib/keycloak.js` — export `KEYCLOAK_URL` and `REALM` constants (were module-private) - `src/routes/(app)/+layout.svelte` — import constants, build Account URL from them for both player/parent and coach navs, add `aria-label` on external links - `src/routes/(app)/admin/+page.svelte` — import constants, replace hardcoded Keycloak admin console URL, replace `style="margin-top: 0.5rem;"` with `mt-sm` utility class, add `aria-label` on external link ## Test Plan - [x] `npm run build` passes - [ ] Coach bottom nav shows Account link (gear icon) between Team and Sign Out - [ ] Player/parent Account link unchanged functionally, now uses imported constants - [ ] Admin dashboard Keycloak link unchanged functionally, now uses imported constants - [ ] All external links have `aria-label` indicating new tab ## Review Checklist - [x] Passed automated review-fix loop - [x] No secrets committed - [x] No unnecessary file changes - [x] Commit messages are descriptive ## Related Notes - Closes #69 - `westside-app #68` — parent PR that introduced these nits
fix: DRY Keycloak URLs, add coach Account link, replace inline style
All checks were successful
ci/woodpecker/pr/woodpecker Pipeline was successful
Details
d79da20d77 
Export KEYCLOAK_URL and REALM from $lib/keycloak.js and import them
in the app layout and admin page, replacing hardcoded URLs. Add the
Account nav link for the coach role (previously only player/parent).
Replace inline margin-top style with the mt-sm utility class and add
aria-labels on external links for accessibility.

Closes #69

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
forgejo_admin commented 2026-03-27 22:12:12 +00:00
Author
Owner
Copy link

QA Review

Acceptance Criteria Check

  • $lib/keycloak.js exports KEYCLOAK_URL and REALM constants
  • Keycloak URLs imported from constants in layout and admin page (DRY)
  • Account link visible for coach role (added between Team and Sign Out)
  • Inline style style="margin-top: 0.5rem;" replaced with mt-sm utility class
  • aria-label on all external links indicating new tab

Code Review

  • keycloak.js: Minimal change -- only added export keyword to two existing constants. CLIENT_ID stays private (correct, not needed externally).
  • +layout.svelte: Svelte template interpolation {KEYCLOAK_URL}/realms/{REALM}/account/ is correct syntax. Coach nav now matches player/parent nav with Account link. Consistent aria-label text across both coach and player/parent Account links.
  • admin/+page.svelte: Import added, hardcoded URL replaced with constants, mt-sm utility class confirmed to exist in app.css (line 153: .mt-sm { margin-top: 0.5rem; }). aria-label added.

Build

  • npm run build passes. No new warnings introduced (all warnings are pre-existing from other files).

Scope

  • 3 files changed, +13/-6 lines. No unrelated changes. Tightly scoped to issue #69.

VERDICT: APPROVE -- All acceptance criteria met, no issues found.

## QA Review ### Acceptance Criteria Check - [x] `$lib/keycloak.js` exports `KEYCLOAK_URL` and `REALM` constants - [x] Keycloak URLs imported from constants in layout and admin page (DRY) - [x] Account link visible for coach role (added between Team and Sign Out) - [x] Inline style `style="margin-top: 0.5rem;"` replaced with `mt-sm` utility class - [x] `aria-label` on all external links indicating new tab ### Code Review - **keycloak.js**: Minimal change -- only added `export` keyword to two existing constants. `CLIENT_ID` stays private (correct, not needed externally). - **+layout.svelte**: Svelte template interpolation `{KEYCLOAK_URL}/realms/{REALM}/account/` is correct syntax. Coach nav now matches player/parent nav with Account link. Consistent aria-label text across both coach and player/parent Account links. - **admin/+page.svelte**: Import added, hardcoded URL replaced with constants, `mt-sm` utility class confirmed to exist in `app.css` (line 153: `.mt-sm { margin-top: 0.5rem; }`). aria-label added. ### Build - `npm run build` passes. No new warnings introduced (all warnings are pre-existing from other files). ### Scope - 3 files changed, +13/-6 lines. No unrelated changes. Tightly scoped to issue #69. **VERDICT: APPROVE** -- All acceptance criteria met, no issues found.
forgejo_admin deleted branch 69-cleanup-keycloak-console-link-nits 2026-03-28 00:07:03 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
domain:backend

   
domain:devops

   
domain:frontend
No labels
domain:backend
domain:devops
domain:frontend
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
forgejo_admin/westside-landing!126
No description provided.