From 2a81d992f15291c4637402dc185f8c417aaa1d74 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Deuerling?= <andre+github@dlng.de>
Date: Wed, 1 Jul 2026 07:55:49 +0200
Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=A8=20Address=20review:=20persist-on-c?=
 =?UTF-8?q?lick=20+=20radiogroup=20toggle?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Persist the layout choice in the toggle click handlers instead of a
  useEffect, so simply opening the Photos tab no longer re-writes the
  value it just read from localStorage (review concern 1).
- Give the Grid/List toggle radiogroup/radio + aria-checked semantics
  so a screen reader announces them as one mutually-exclusive set
  (review concern 2).
- Add a test that mount performs no localStorage write.
---
 .../src/components/admin/AdminPhotoGrid.tsx   | 27 ++++++++++++-------
 .../AdminPhotoGrid.viewToggle.test.tsx        | 10 +++++--
 2 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/frontend/src/components/admin/AdminPhotoGrid.tsx b/frontend/src/components/admin/AdminPhotoGrid.tsx
index 64485699..d482a25a 100644
--- a/frontend/src/components/admin/AdminPhotoGrid.tsx
+++ b/frontend/src/components/admin/AdminPhotoGrid.tsx
@@ -1,4 +1,4 @@
-import React, { useEffect, useState } from 'react';
+import React, { useState } from 'react';
 import { Check, Download, Trash2, Eye, EyeOff, Heart, Package, MessageSquare, Star, Video, FolderOpen, Cog, AlertTriangle, RefreshCw, LayoutGrid, List } from 'lucide-react';
 import { toast } from 'react-toastify';
 import { useQueryClient } from '@tanstack/react-query';
@@ -47,9 +47,13 @@ export const AdminPhotoGrid: React.FC<AdminPhotoGridProps> = ({
   // Layout toggle (Grid / List) persisted per admin via localStorage.
   const [viewMode, setViewMode] = useState<PhotoViewMode>(() => getPhotoViewMode());
 
-  useEffect(() => {
-    setPhotoViewMode(viewMode);
-  }, [viewMode]);
+  // Persist on user action only — writing in an effect would re-save the
+  // value on every mount (i.e. each time the Photos tab is opened), even
+  // when the user never touched the toggle.
+  const selectView = (mode: PhotoViewMode) => {
+    setViewMode(mode);
+    setPhotoViewMode(mode);
+  };
 
   const handlePhotoSelect = (photoId: number, e?: React.MouseEvent) => {
     if (e) {
@@ -259,12 +263,14 @@ export const AdminPhotoGrid: React.FC<AdminPhotoGridProps> = ({
           <div className="text-sm text-neutral-600 dark:text-neutral-400">
             {t('gallery.photosCount', { count: photos.length })}
           </div>
-          {/* Layout toggle: Grid / List */}
-          <div className="inline-flex rounded-lg border border-neutral-300 dark:border-neutral-600 overflow-hidden" role="group" aria-label={t('admin.photos.viewMode', 'View mode')}>
+          {/* Layout toggle: Grid / List — radiogroup so a screen reader
+              announces the two options as one mutually-exclusive set. */}
+          <div className="inline-flex rounded-lg border border-neutral-300 dark:border-neutral-600 overflow-hidden" role="radiogroup" aria-label={t('admin.photos.viewMode', 'View mode')}>
             <button
               type="button"
-              onClick={() => setViewMode('grid')}
-              aria-pressed={viewMode === 'grid'}
+              role="radio"
+              onClick={() => selectView('grid')}
+              aria-checked={viewMode === 'grid'}
               title={t('admin.photos.gridView', 'Grid view')}
               className={`p-1.5 transition-colors ${
                 viewMode === 'grid'
@@ -276,8 +282,9 @@ export const AdminPhotoGrid: React.FC<AdminPhotoGridProps> = ({
             </button>
             <button
               type="button"
-              onClick={() => setViewMode('list')}
-              aria-pressed={viewMode === 'list'}
+              role="radio"
+              onClick={() => selectView('list')}
+              aria-checked={viewMode === 'list'}
               title={t('admin.photos.listView', 'List view')}
               className={`p-1.5 transition-colors border-l border-neutral-300 dark:border-neutral-600 ${
                 viewMode === 'list'
diff --git a/frontend/src/components/admin/__tests__/AdminPhotoGrid.viewToggle.test.tsx b/frontend/src/components/admin/__tests__/AdminPhotoGrid.viewToggle.test.tsx
index 91e1360c..a9dee398 100644
--- a/frontend/src/components/admin/__tests__/AdminPhotoGrid.viewToggle.test.tsx
+++ b/frontend/src/components/admin/__tests__/AdminPhotoGrid.viewToggle.test.tsx
@@ -83,11 +83,17 @@ describe('AdminPhotoGrid layout toggle', () => {
     expect(screen.queryByTestId('admin-photo-row-1')).not.toBeInTheDocument();
   });
 
+  it('does not write to localStorage on mount (only on user toggle)', () => {
+    renderGrid();
+    // Opening the tab must not persist the value it just read.
+    expect(localStorage.getItem('picpeak.adminPhotos.view')).toBeNull();
+  });
+
   it('switches to list rows when the List toggle is clicked', async () => {
     const user = userEvent.setup();
     renderGrid();
 
-    await user.click(screen.getByRole('button', { name: /list view/i }));
+    await user.click(screen.getByRole('radio', { name: /list view/i }));
 
     expect(screen.getByTestId('admin-photo-row-1')).toBeInTheDocument();
     expect(screen.getByTestId('admin-photo-row-2')).toBeInTheDocument();
@@ -98,7 +104,7 @@ describe('AdminPhotoGrid layout toggle', () => {
     const user = userEvent.setup();
     const { unmount } = renderGrid();
 
-    await user.click(screen.getByRole('button', { name: /list view/i }));
+    await user.click(screen.getByRole('radio', { name: /list view/i }));
     expect(localStorage.getItem('picpeak.adminPhotos.view')).toBe('list');
     unmount();