🎨 Address review: persist-on-click + radiogroup toggle
- 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.
This commit is contained in:
@@ -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 { 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 { toast } from 'react-toastify';
|
||||||
import { useQueryClient } from '@tanstack/react-query';
|
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.
|
// Layout toggle (Grid / List) persisted per admin via localStorage.
|
||||||
const [viewMode, setViewMode] = useState<PhotoViewMode>(() => getPhotoViewMode());
|
const [viewMode, setViewMode] = useState<PhotoViewMode>(() => getPhotoViewMode());
|
||||||
|
|
||||||
useEffect(() => {
|
// Persist on user action only — writing in an effect would re-save the
|
||||||
setPhotoViewMode(viewMode);
|
// value on every mount (i.e. each time the Photos tab is opened), even
|
||||||
}, [viewMode]);
|
// when the user never touched the toggle.
|
||||||
|
const selectView = (mode: PhotoViewMode) => {
|
||||||
|
setViewMode(mode);
|
||||||
|
setPhotoViewMode(mode);
|
||||||
|
};
|
||||||
|
|
||||||
const handlePhotoSelect = (photoId: number, e?: React.MouseEvent) => {
|
const handlePhotoSelect = (photoId: number, e?: React.MouseEvent) => {
|
||||||
if (e) {
|
if (e) {
|
||||||
@@ -259,12 +263,14 @@ export const AdminPhotoGrid: React.FC<AdminPhotoGridProps> = ({
|
|||||||
<div className="text-sm text-neutral-600 dark:text-neutral-400">
|
<div className="text-sm text-neutral-600 dark:text-neutral-400">
|
||||||
{t('gallery.photosCount', { count: photos.length })}
|
{t('gallery.photosCount', { count: photos.length })}
|
||||||
</div>
|
</div>
|
||||||
{/* Layout toggle: Grid / List */}
|
{/* Layout toggle: Grid / List — radiogroup so a screen reader
|
||||||
<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')}>
|
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
|
<button
|
||||||
type="button"
|
type="button"
|
||||||
onClick={() => setViewMode('grid')}
|
role="radio"
|
||||||
aria-pressed={viewMode === 'grid'}
|
onClick={() => selectView('grid')}
|
||||||
|
aria-checked={viewMode === 'grid'}
|
||||||
title={t('admin.photos.gridView', 'Grid view')}
|
title={t('admin.photos.gridView', 'Grid view')}
|
||||||
className={`p-1.5 transition-colors ${
|
className={`p-1.5 transition-colors ${
|
||||||
viewMode === 'grid'
|
viewMode === 'grid'
|
||||||
@@ -276,8 +282,9 @@ export const AdminPhotoGrid: React.FC<AdminPhotoGridProps> = ({
|
|||||||
</button>
|
</button>
|
||||||
<button
|
<button
|
||||||
type="button"
|
type="button"
|
||||||
onClick={() => setViewMode('list')}
|
role="radio"
|
||||||
aria-pressed={viewMode === 'list'}
|
onClick={() => selectView('list')}
|
||||||
|
aria-checked={viewMode === 'list'}
|
||||||
title={t('admin.photos.listView', 'List view')}
|
title={t('admin.photos.listView', 'List view')}
|
||||||
className={`p-1.5 transition-colors border-l border-neutral-300 dark:border-neutral-600 ${
|
className={`p-1.5 transition-colors border-l border-neutral-300 dark:border-neutral-600 ${
|
||||||
viewMode === 'list'
|
viewMode === 'list'
|
||||||
|
|||||||
@@ -83,11 +83,17 @@ describe('AdminPhotoGrid layout toggle', () => {
|
|||||||
expect(screen.queryByTestId('admin-photo-row-1')).not.toBeInTheDocument();
|
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 () => {
|
it('switches to list rows when the List toggle is clicked', async () => {
|
||||||
const user = userEvent.setup();
|
const user = userEvent.setup();
|
||||||
renderGrid();
|
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-1')).toBeInTheDocument();
|
||||||
expect(screen.getByTestId('admin-photo-row-2')).toBeInTheDocument();
|
expect(screen.getByTestId('admin-photo-row-2')).toBeInTheDocument();
|
||||||
@@ -98,7 +104,7 @@ describe('AdminPhotoGrid layout toggle', () => {
|
|||||||
const user = userEvent.setup();
|
const user = userEvent.setup();
|
||||||
const { unmount } = renderGrid();
|
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');
|
expect(localStorage.getItem('picpeak.adminPhotos.view')).toBe('list');
|
||||||
unmount();
|
unmount();
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user