From 393cb52178d03718abc1c937a3e209b9018c0af1 Mon Sep 17 00:00:00 2001 From: Marcel Date: Mon, 30 Mar 2026 11:23:27 +0200 Subject: [PATCH] fix(admin): address PR review feedback from all personas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Blockers resolved: - localStorage key collision: UsersListPanel/GroupsListPanel/TagsListPanel now each use their own key (admin_*_list_collapsed) - $effect autocollapse replaced with $derived(autocollapse || manualCollapse) across all three list panels (Felix — Svelte 5 rule violation) - groups/new: add READ_ALL and ANNOTATE_ALL to available standard permissions - Mobile back-to-list links added to all five detail panel headers (md:hidden) so users landing directly on a detail URL on mobile can navigate back - onDestroy(() => stopPolling()) added to system/+page.svelte (Tobias) High priority resolved: - Permission labels in groups/[id] and groups/new now use Paraglide i18n keys (admin_perm_read_all, admin_perm_annotate_all, etc.) across de/en/es - $derived used for permission arrays (reactive i18n) — Felix Svelte 5 rule - UserGroup type in +layout.server.ts now uses generated API type (Markus/Felix) - discardTarget annotation changed to variable-level type annotation Accessibility (Leonie): - EntityNav tablet icon strip buttons: min-h-[44px] for WCAG 2.5.8 compliance - Flyout focus management: openFlyout() focuses first link, closeFlyout() returns focus to the trigger button that opened it - Flyout animation replaced: broken inline style -> transition:fly={{ x: -160 }} Tests (Sara/Felix): - localStorage key assertion tests added per panel - localStorage.removeItem calls updated to use the panel-specific keys - page.server.spec.ts added for groups/[id] and tags/[id] delete actions - Polling lifecycle tests added to system/page.svelte.spec.ts Note: Paraglide types for new admin_perm_* keys regenerate automatically on next npm run dev (Vite plugin). No manual compilation step needed. Co-Authored-By: Claude Sonnet 4.6 --- frontend/messages/de.json | 7 ++ frontend/messages/en.json | 7 ++ frontend/messages/es.json | 7 ++ frontend/src/app.d.ts | 1 + frontend/src/routes/admin/+layout.server.ts | 3 +- frontend/src/routes/admin/EntityNav.svelte | 46 ++++++---- .../admin/groups/GroupsListPanel.svelte | 16 ++-- .../src/routes/admin/groups/[id]/+page.svelte | 39 ++++++--- .../admin/groups/[id]/page.server.spec.ts | 87 +++++++++++++++++++ .../routes/admin/groups/layout.svelte.spec.ts | 10 ++- .../src/routes/admin/groups/new/+page.svelte | 35 ++++++-- frontend/src/routes/admin/system/+page.svelte | 4 +- .../routes/admin/system/page.svelte.spec.ts | 55 +++++++++++- .../routes/admin/tags/TagsListPanel.svelte | 16 ++-- .../src/routes/admin/tags/[id]/+page.svelte | 17 +++- .../admin/tags/[id]/page.server.spec.ts | 85 ++++++++++++++++++ .../routes/admin/tags/layout.svelte.spec.ts | 10 ++- .../routes/admin/users/UsersListPanel.svelte | 16 ++-- .../src/routes/admin/users/[id]/+page.svelte | 17 +++- .../routes/admin/users/layout.svelte.spec.ts | 10 ++- .../src/routes/admin/users/new/+page.svelte | 17 +++- 21 files changed, 434 insertions(+), 71 deletions(-) create mode 100644 frontend/src/routes/admin/groups/[id]/page.server.spec.ts create mode 100644 frontend/src/routes/admin/tags/[id]/page.server.spec.ts diff --git a/frontend/messages/de.json b/frontend/messages/de.json index c344e8e3..de8e73e6 100644 --- a/frontend/messages/de.json +++ b/frontend/messages/de.json @@ -190,6 +190,13 @@ "admin_group_created": "Gruppe erstellt.", "admin_groups_section_standard": "Standard", "admin_groups_section_administrative": "Administrativ", + "admin_perm_read_all": "Nur lesen", + "admin_perm_annotate_all": "Lesen & Annotieren", + "admin_perm_write_all": "Lesen & Schreiben", + "admin_perm_admin": "Vollzugriff (Admin)", + "admin_perm_admin_user": "Benutzer verwalten", + "admin_perm_admin_tag": "Schlagworte verwalten", + "admin_perm_admin_permission": "Berechtigungen verwalten", "admin_user_new_heading": "Neuen Benutzer anlegen", "admin_user_edit_heading": "Benutzer bearbeiten: {username}", "admin_user_created": "Benutzer wurde erstellt.", diff --git a/frontend/messages/en.json b/frontend/messages/en.json index 7bb2a116..cc25945f 100644 --- a/frontend/messages/en.json +++ b/frontend/messages/en.json @@ -190,6 +190,13 @@ "admin_group_created": "Group created.", "admin_groups_section_standard": "Standard", "admin_groups_section_administrative": "Administrative", + "admin_perm_read_all": "Read only", + "admin_perm_annotate_all": "Read & Annotate", + "admin_perm_write_all": "Read & Write", + "admin_perm_admin": "Full access (Admin)", + "admin_perm_admin_user": "Manage users", + "admin_perm_admin_tag": "Manage tags", + "admin_perm_admin_permission": "Manage permissions", "admin_user_new_heading": "Create new user", "admin_user_edit_heading": "Edit user: {username}", "admin_user_created": "User has been created.", diff --git a/frontend/messages/es.json b/frontend/messages/es.json index 0f9d93d4..3e2cab2b 100644 --- a/frontend/messages/es.json +++ b/frontend/messages/es.json @@ -190,6 +190,13 @@ "admin_group_created": "Grupo creado.", "admin_groups_section_standard": "Est\u00e1ndar", "admin_groups_section_administrative": "Administrativo", + "admin_perm_read_all": "Solo lectura", + "admin_perm_annotate_all": "Leer y anotar", + "admin_perm_write_all": "Leer y escribir", + "admin_perm_admin": "Acceso completo (Admin)", + "admin_perm_admin_user": "Gestionar usuarios", + "admin_perm_admin_tag": "Gestionar etiquetas", + "admin_perm_admin_permission": "Gestionar permisos", "admin_user_new_heading": "Crear nuevo usuario", "admin_user_edit_heading": "Editar usuario: {username}", "admin_user_created": "Usuario creado.", diff --git a/frontend/src/app.d.ts b/frontend/src/app.d.ts index 0663671f..ca517012 100644 --- a/frontend/src/app.d.ts +++ b/frontend/src/app.d.ts @@ -12,6 +12,7 @@ declare global { email?: string; contact?: string; groups: { + id: string; name: string; permissions: string[]; }[]; diff --git a/frontend/src/routes/admin/+layout.server.ts b/frontend/src/routes/admin/+layout.server.ts index 19405345..f21bbff4 100644 --- a/frontend/src/routes/admin/+layout.server.ts +++ b/frontend/src/routes/admin/+layout.server.ts @@ -1,8 +1,9 @@ import { error } from '@sveltejs/kit'; import { createApiClient } from '$lib/api.server'; import { getErrorMessage } from '$lib/errors'; +import type { components } from '$lib/generated/api'; -type UserGroup = { permissions: string[] }; +type UserGroup = components['schemas']['UserGroup']; function hasPerm(user: { groups?: UserGroup[] } | undefined, perm: string): boolean { return user?.groups?.some((g) => g.permissions.includes(perm)) ?? false; diff --git a/frontend/src/routes/admin/EntityNav.svelte b/frontend/src/routes/admin/EntityNav.svelte index ae57556b..0de54985 100644 --- a/frontend/src/routes/admin/EntityNav.svelte +++ b/frontend/src/routes/admin/EntityNav.svelte @@ -1,4 +1,6 @@ @@ -55,8 +71,8 @@ function handleKeydown(event: KeyboardEvent) { data-flyout-trigger type="button" aria-label={m.admin_tab_users()} - onclick={() => (flyoutOpen = true)} - class="flex w-full flex-col items-center justify-center gap-0.5 border-l-[3px] py-3 transition-colors lg:hidden + onclick={openFlyout} + class="flex min-h-[44px] w-full flex-col items-center justify-center gap-0.5 border-l-[3px] py-3 transition-colors lg:hidden {isActive('users') ? 'border-brand-mint bg-white/10' : 'border-transparent hover:bg-white/5'}" @@ -121,8 +137,8 @@ function handleKeydown(event: KeyboardEvent) { data-flyout-trigger type="button" aria-label={m.admin_tab_groups()} - onclick={() => (flyoutOpen = true)} - class="flex w-full flex-col items-center justify-center gap-0.5 border-l-[3px] py-3 transition-colors lg:hidden + onclick={openFlyout} + class="flex min-h-[44px] w-full flex-col items-center justify-center gap-0.5 border-l-[3px] py-3 transition-colors lg:hidden {isActive('groups') ? 'border-brand-mint bg-white/10' : 'border-transparent hover:bg-white/5'}" @@ -187,8 +203,8 @@ function handleKeydown(event: KeyboardEvent) { data-flyout-trigger type="button" aria-label={m.admin_tab_tags()} - onclick={() => (flyoutOpen = true)} - class="flex w-full flex-col items-center justify-center gap-0.5 border-l-[3px] py-3 transition-colors lg:hidden + onclick={openFlyout} + class="flex min-h-[44px] w-full flex-col items-center justify-center gap-0.5 border-l-[3px] py-3 transition-colors lg:hidden {isActive('tags') ? 'border-brand-mint bg-white/10' : 'border-transparent hover:bg-white/5'}" @@ -257,8 +273,8 @@ function handleKeydown(event: KeyboardEvent) { data-flyout-trigger type="button" aria-label={m.admin_tab_system()} - onclick={() => (flyoutOpen = true)} - class="flex w-full flex-col items-center justify-center gap-0.5 border-t border-l-[3px] border-white/10 py-3 transition-colors lg:hidden + onclick={openFlyout} + class="flex min-h-[44px] w-full flex-col items-center justify-center gap-0.5 border-t border-l-[3px] border-white/10 py-3 transition-colors lg:hidden {isActive('system') ? 'border-brand-mint bg-white/10' : 'border-l-transparent hover:bg-white/5'}" @@ -320,7 +336,7 @@ function handleKeydown(event: KeyboardEvent) { data-flyout-backdrop role="none" class="fixed inset-0 z-40 bg-black/40" - onclick={() => (flyoutOpen = false)} + onclick={closeFlyout} > @@ -329,7 +345,7 @@ function handleKeydown(event: KeyboardEvent) { aria-modal="true" aria-label={m.admin_heading()} class="fixed top-0 left-12 z-50 flex h-full w-40 flex-col bg-brand-navy shadow-xl" - style="transform: translateX(0); transition: transform 180ms ease-out;" + transition:fly={{ x: -160, duration: 180 }} >
@@ -339,7 +355,7 @@ function handleKeydown(event: KeyboardEvent) { {#if canManageUsers} (flyoutOpen = false)} + onclick={closeFlyout} class="flex flex-col items-start justify-center gap-0.5 border-l-[3px] px-3.5 py-2.5 transition-colors {isActive('users') ? 'border-brand-mint bg-white/10' @@ -377,7 +393,7 @@ function handleKeydown(event: KeyboardEvent) { {#if canManageGroups} (flyoutOpen = false)} + onclick={closeFlyout} class="flex flex-col items-start justify-center gap-0.5 border-l-[3px] px-3.5 py-2.5 transition-colors {isActive('groups') ? 'border-brand-mint bg-white/10' @@ -415,7 +431,7 @@ function handleKeydown(event: KeyboardEvent) { {#if canManageTags} (flyoutOpen = false)} + onclick={closeFlyout} class="flex flex-col items-start justify-center gap-0.5 border-l-[3px] px-3.5 py-2.5 transition-colors {isActive('tags') ? 'border-brand-mint bg-white/10' @@ -454,7 +470,7 @@ function handleKeydown(event: KeyboardEvent) { {#if canRunMaintenance} (flyoutOpen = false)} + onclick={closeFlyout} class="flex flex-col items-start justify-center gap-0.5 border-t border-l-[3px] border-white/10 px-3.5 py-2.5 transition-colors {isActive('system') ? 'border-brand-mint bg-white/10' diff --git a/frontend/src/routes/admin/groups/GroupsListPanel.svelte b/frontend/src/routes/admin/groups/GroupsListPanel.svelte index fe967f82..233a6943 100644 --- a/frontend/src/routes/admin/groups/GroupsListPanel.svelte +++ b/frontend/src/routes/admin/groups/GroupsListPanel.svelte @@ -16,17 +16,15 @@ let { autocollapse?: boolean; } = $props(); -let isCollapsed = $state( - typeof localStorage !== 'undefined' && localStorage.getItem('admin_list_collapsed') === 'true' +let manualCollapse = $state( + typeof localStorage !== 'undefined' && + localStorage.getItem('admin_groups_list_collapsed') === 'true' ); - -$effect(() => { - if (autocollapse) isCollapsed = true; -}); +const isCollapsed = $derived(autocollapse || manualCollapse); $effect(() => { if (typeof localStorage !== 'undefined') { - localStorage.setItem('admin_list_collapsed', String(isCollapsed)); + localStorage.setItem('admin_groups_list_collapsed', String(manualCollapse)); } }); @@ -34,7 +32,7 @@ $effect(() => { {#if isCollapsed}