From 8d11ac415e3dbd67c4e60dfa603a99994d8e4758 Mon Sep 17 00:00:00 2001 From: tanyar09 Date: Mon, 10 Nov 2025 11:41:45 -0500 Subject: [PATCH] feat: Enhance Modify and Tags components with improved state management and confirmation dialogs This commit refines the Modify and Tags components by implementing better state management for person selection and tag handling. In the Modify component, the logic for checking if a selected person still exists has been optimized to prevent unnecessary state updates. The Tags component has been updated to support immediate saving of tags and includes confirmation dialogs for tag removals, enhancing user experience. Additionally, error handling for tag creation and deletion has been improved, ensuring a more robust interaction with the API. Documentation has been updated to reflect these changes. --- frontend/src/pages/Modify.tsx | 33 +- frontend/src/pages/Tags.tsx | 580 +++++++++++++++++++++++++++------- 2 files changed, 486 insertions(+), 127 deletions(-) diff --git a/frontend/src/pages/Modify.tsx b/frontend/src/pages/Modify.tsx index 7ee3d0e..8f0fece 100644 --- a/frontend/src/pages/Modify.tsx +++ b/frontend/src/pages/Modify.tsx @@ -326,21 +326,28 @@ export default function Modify() { // Reload people list first to update face counts and check if person still exists const peopleRes = await peopleApi.listWithFaces(lastNameFilter || undefined) + + // Check if selected person still exists BEFORE updating state + // This prevents useEffect from triggering unnecessary requests + const currentSelectedId = selectedPersonId + const personStillExists = currentSelectedId + ? peopleRes.items.some(p => p.id === currentSelectedId) + : false + + // Clear selection immediately if person was deleted (before state update) + if (currentSelectedId && !personStillExists) { + setSelectedPersonId(null) + setSelectedPersonName('') + setFaces([]) + setError(null) + } + + // Update people list setPeople(peopleRes.items) - // Check if selected person still exists and handle accordingly - if (selectedPersonId) { - const personStillExists = peopleRes.items.some(p => p.id === selectedPersonId) - if (personStillExists) { - // Person still exists, reload their faces - await loadPersonFaces(selectedPersonId) - } else { - // Person was deleted, clear selection and any errors - setSelectedPersonId(null) - setSelectedPersonName('') - setFaces([]) - setError(null) // Clear any error that might have been set - } + // Reload faces only if person still exists + if (currentSelectedId && personStillExists) { + await loadPersonFaces(currentSelectedId) } setSuccess(`Successfully unlinked ${faceIds.length} face(s)`) diff --git a/frontend/src/pages/Tags.tsx b/frontend/src/pages/Tags.tsx index 3588158..3678162 100644 --- a/frontend/src/pages/Tags.tsx +++ b/frontend/src/pages/Tags.tsx @@ -122,8 +122,8 @@ export default function Tags() { })) } - // Add tag to photo - const addTagToPhoto = async (photoId: number, tagName: string, linkageType: number = 0) => { + // Add tag to photo (with optional immediate save) + const addTagToPhoto = async (photoId: number, tagName: string, linkageType: number = 0, saveImmediately: boolean = false) => { const tag = tags.find(t => t.tag_name.toLowerCase() === tagName.toLowerCase().trim()) let tagId: number @@ -142,46 +142,80 @@ export default function Tags() { } } - // Add to pending changes - setPendingTagChanges(prev => ({ - ...prev, - [photoId]: [...(prev[photoId] || []), tagId], - })) - setPendingLinkageTypes(prev => ({ - ...prev, - [photoId]: { - ...(prev[photoId] || {}), - [tagId]: linkageType, - }, - })) - } - - // Remove tag from photo - const removeTagFromPhoto = (photoId: number, tagId: number, linkageType: number) => { - // If it's a pending addition, just remove it - if (pendingTagChanges[photoId]?.includes(tagId)) { + if (saveImmediately) { + // Save immediately to database + try { + await tagsApi.addToPhotos({ + photo_ids: [photoId], + tag_names: [tagName.trim()], + linkage_type: linkageType, + }) + // Don't reload here - will reload when dialog closes + } catch (error) { + console.error('Failed to save tag:', error) + alert('Failed to save tag') + return + } + } else { + // Add to pending changes setPendingTagChanges(prev => ({ ...prev, - [photoId]: (prev[photoId] || []).filter(id => id !== tagId), + [photoId]: [...(prev[photoId] || []), tagId], + })) + setPendingLinkageTypes(prev => ({ + ...prev, + [photoId]: { + ...(prev[photoId] || {}), + [tagId]: linkageType, + }, })) - setPendingLinkageTypes(prev => { - const newTypes = { ...prev } - if (newTypes[photoId]) { - delete newTypes[photoId][tagId] - if (Object.keys(newTypes[photoId]).length === 0) { - delete newTypes[photoId] - } - } - return newTypes - }) - return } + } - // Otherwise, add to pending removals - setPendingTagRemovals(prev => ({ - ...prev, - [photoId]: [...(prev[photoId] || []), tagId], - })) + // Remove tag from photo (with optional immediate save) + const removeTagFromPhoto = async (photoId: number, tagId: number, linkageType: number, saveImmediately: boolean = false) => { + const tag = tags.find(t => t.id === tagId) + if (!tag) return + + if (saveImmediately) { + // Save immediately to database + try { + await tagsApi.removeFromPhotos({ + photo_ids: [photoId], + tag_names: [tag.tag_name], + }) + // Don't reload here - will reload when dialog closes + } catch (error) { + console.error('Failed to remove tag:', error) + alert('Failed to remove tag') + return + } + } else { + // If it's a pending addition, just remove it + if (pendingTagChanges[photoId]?.includes(tagId)) { + setPendingTagChanges(prev => ({ + ...prev, + [photoId]: (prev[photoId] || []).filter(id => id !== tagId), + })) + setPendingLinkageTypes(prev => { + const newTypes = { ...prev } + if (newTypes[photoId]) { + delete newTypes[photoId][tagId] + if (Object.keys(newTypes[photoId]).length === 0) { + delete newTypes[photoId] + } + } + return newTypes + }) + return + } + + // Otherwise, add to pending removals + setPendingTagRemovals(prev => ({ + ...prev, + [photoId]: [...(prev[photoId] || []), tagId], + })) + } } // Save pending changes @@ -367,7 +401,7 @@ export default function Tags() { onClick={() => toggleFolder(folder.folderPath)} className="px-2 py-1 text-sm" > - {folderStates[folder.folderPath] !== false ? '▼' : '▶'} + {folderStates[folder.folderPath] === true ? '▼' : '▶'} 📁 {folder.folderName} ({folder.photoCount} photos) @@ -381,7 +415,7 @@ export default function Tags() { - {folderStates[folder.folderPath] !== false && folder.photos.map(photo => ( + {folderStates[folder.folderPath] === true && folder.photos.map(photo => ( {photo.id} @@ -449,7 +483,11 @@ export default function Tags() { {showManageTags && ( setShowManageTags(false)} + onClose={async () => { + setShowManageTags(false) + // Refresh photos when exiting the dialog + await loadData() + }} onTagsChange={setTags} /> )} @@ -462,9 +500,13 @@ export default function Tags() { tags={tags} pendingTagChanges={pendingTagChanges[showTagDialog] || []} pendingTagRemovals={pendingTagRemovals[showTagDialog] || []} - onClose={() => setShowTagDialog(null)} - onAddTag={(tagName, linkageType) => addTagToPhoto(showTagDialog, tagName, linkageType)} - onRemoveTag={(tagId, linkageType) => removeTagFromPhoto(showTagDialog, tagId, linkageType)} + onClose={async () => { + setShowTagDialog(null) + // Reload data when dialog is closed + await loadData() + }} + onAddTag={(tagName, linkageType) => addTagToPhoto(showTagDialog, tagName, linkageType, true)} + onRemoveTag={(tagId, linkageType) => removeTagFromPhoto(showTagDialog, tagId, linkageType, true)} getPhotoTags={async (photoId) => { return await tagsApi.getPhotoTags(photoId) }} @@ -477,14 +519,69 @@ export default function Tags() { folderPath={showBulkTagDialog} folder={folderGroups.find(f => f.folderPath === showBulkTagDialog)} tags={tags} - onClose={() => setShowBulkTagDialog(null)} - onAddTag={(tagName) => { + pendingTagChanges={pendingTagChanges} + pendingTagRemovals={pendingTagRemovals} + onClose={async () => { + setShowBulkTagDialog(null) + // Reload data when dialog is closed + await loadData() + }} + onAddTag={async (tagName) => { const folder = folderGroups.find(f => f.folderPath === showBulkTagDialog) - if (folder) { - folder.photos.forEach(photo => { - addTagToPhoto(photo.id, tagName, 1) // linkage_type = 1 (bulk) - }) + if (!folder || folder.photos.length === 0) return + + // Check if tag exists, create if not + let tag = tags.find(t => t.tag_name.toLowerCase() === tagName.toLowerCase().trim()) + if (!tag) { + try { + tag = await tagsApi.create(tagName.trim()) + setTags(prev => [...prev, tag!].sort((a, b) => a.tag_name.localeCompare(b.tag_name))) + } catch (error) { + console.error('Failed to create tag:', error) + alert('Failed to create tag') + return + } } + + // Collect all photo IDs + const photoIds = folder.photos.map(photo => photo.id) + + // Make single batch API call for all photos + try { + await tagsApi.addToPhotos({ + photo_ids: photoIds, + tag_names: [tagName.trim()], + linkage_type: 1, // bulk + }) + } catch (error) { + console.error('Failed to save tags:', error) + alert('Failed to save tags') + } + }} + onRemoveTag={async (tagId) => { + const folder = folderGroups.find(f => f.folderPath === showBulkTagDialog) + if (!folder || folder.photos.length === 0) return + + // Find tag name + const tag = tags.find(t => t.id === tagId) + if (!tag) return + + // Collect all photo IDs + const photoIds = folder.photos.map(photo => photo.id) + + // Make single batch API call for all photos + try { + await tagsApi.removeFromPhotos({ + photo_ids: photoIds, + tag_names: [tag.tag_name], + }) + } catch (error) { + console.error('Failed to remove tags:', error) + alert('Failed to remove tags') + } + }} + getPhotoTags={async (photoId) => { + return await tagsApi.getPhotoTags(photoId) }} /> )} @@ -492,6 +589,40 @@ export default function Tags() { ) } +// Confirmation Dialog Component +function ConfirmDialog({ + message, + onConfirm, + onCancel, +}: { + message: string + onConfirm: () => void + onCancel: () => void +}) { + return ( +
+
+

Confirm Delete

+

{message}

+
+ + +
+
+
+ ) +} + // Manage Tags Dialog Component function ManageTagsDialog({ tags, @@ -506,14 +637,31 @@ function ManageTagsDialog({ const [selectedTagIds, setSelectedTagIds] = useState>(new Set()) const [editingTagId, setEditingTagId] = useState(null) const [editTagName, setEditTagName] = useState('') + const [showConfirmDialog, setShowConfirmDialog] = useState(false) + const [tagError, setTagError] = useState('') + + // Check if tag name already exists (case-insensitive) + const isTagDuplicate = (tagName: string): boolean => { + if (!tagName.trim()) return false + const trimmedName = tagName.trim().toLowerCase() + return tags.some(tag => tag.tag_name.toLowerCase() === trimmedName) + } const handleCreateTag = async () => { if (!newTagName.trim()) return + // Check for duplicate (case-insensitive) + if (isTagDuplicate(newTagName)) { + setTagError('A tag with this name already exists') + return + } + + setTagError('') try { const newTag = await tagsApi.create(newTagName.trim()) onTagsChange([...tags, newTag].sort((a, b) => a.tag_name.localeCompare(b.tag_name))) setNewTagName('') + setTagError('') } catch (error) { console.error('Failed to create tag:', error) alert('Failed to create tag') @@ -534,19 +682,18 @@ function ManageTagsDialog({ } } - const handleDeleteTags = async () => { + const handleDeleteTags = () => { if (selectedTagIds.size === 0) return + setShowConfirmDialog(true) + } - if (!confirm(`Delete ${selectedTagIds.size} selected tag(s)? This will unlink them from photos.`)) { - return - } - + const confirmDeleteTags = async () => { + setShowConfirmDialog(false) + try { await tagsApi.delete(Array.from(selectedTagIds)) onTagsChange(tags.filter(t => !selectedTagIds.has(t.id))) setSelectedTagIds(new Set()) - // Reload photos to reflect tag deletions - window.location.reload() } catch (error) { console.error('Failed to delete tags:', error) alert('Failed to delete tags') @@ -554,27 +701,53 @@ function ManageTagsDialog({ } return ( -
-
+ <> + {showConfirmDialog && ( + setShowConfirmDialog(false)} + /> + )} +
+

Manage Tags

-
- setNewTagName(e.target.value)} - onKeyPress={(e) => e.key === 'Enter' && handleCreateTag()} - placeholder="Enter new tag name" - className="flex-1 px-3 py-2 border border-gray-300 rounded" - /> - +
+
+
+ { + setNewTagName(e.target.value) + // Check for duplicate as user types + if (e.target.value.trim() && isTagDuplicate(e.target.value)) { + setTagError('A tag with this name already exists') + } else { + setTagError('') + } + }} + onKeyPress={(e) => e.key === 'Enter' && handleCreateTag()} + placeholder="Enter new tag name" + className={`w-full px-3 py-2 border rounded ${ + tagError ? 'border-red-500' : 'border-gray-300' + }`} + /> + {tagError && ( +

{tagError}

+ )} +
+ +
@@ -660,6 +833,7 @@ function ManageTagsDialog({
+ ) } @@ -681,13 +855,14 @@ function PhotoTagDialog({ pendingTagChanges: number[] pendingTagRemovals: number[] onClose: () => void - onAddTag: (tagName: string, linkageType: number) => void - onRemoveTag: (tagId: number, linkageType: number) => void + onAddTag: (tagName: string, linkageType: number) => Promise + onRemoveTag: (tagId: number, linkageType: number) => Promise getPhotoTags: (photoId: number) => Promise }) { const [selectedTagName, setSelectedTagName] = useState('') const [photoTags, setPhotoTags] = useState([]) const [selectedTagIds, setSelectedTagIds] = useState>(new Set()) + const [showConfirmDialog, setShowConfirmDialog] = useState(false) useEffect(() => { loadPhotoTags() @@ -702,22 +877,28 @@ function PhotoTagDialog({ } } - const handleAddTag = () => { + const handleAddTag = async () => { if (!selectedTagName.trim()) return - onAddTag(selectedTagName.trim(), 0) // linkage_type = 0 (single) + await onAddTag(selectedTagName.trim(), 0) // linkage_type = 0 (single) setSelectedTagName('') - loadPhotoTags() + await loadPhotoTags() } const handleRemoveTags = () => { if (selectedTagIds.size === 0) return - selectedTagIds.forEach(tagId => { + setShowConfirmDialog(true) + } + + const confirmRemoveTags = async () => { + setShowConfirmDialog(false) + if (selectedTagIds.size === 0) return + for (const tagId of selectedTagIds) { const tag = photoTags.find(t => t.tag_id === tagId) const linkageType = tag?.linkage_type || 0 - onRemoveTag(tagId, linkageType) - }) + await onRemoveTag(tagId, linkageType) + } setSelectedTagIds(new Set()) - loadPhotoTags() + await loadPhotoTags() } // Combine saved and pending tags @@ -733,8 +914,27 @@ function PhotoTagDialog({ return [...savedTags, ...pendingTagObjs] }, [photoTags, pendingTagChanges, pendingTagRemovals, tags]) + // Get selected tag names for confirmation message + const selectedTagNames = useMemo(() => { + return Array.from(selectedTagIds) + .map(id => { + const tag = allTags.find(t => t.tag_id === id) + return tag ? tag.tag_name : '' + }) + .filter(Boolean) + .join(', ') + }, [selectedTagIds, allTags]) + return ( -
+ <> + {showConfirmDialog && ( + setShowConfirmDialog(false)} + /> + )} +

Manage Photo Tags

@@ -815,6 +1015,7 @@ function PhotoTagDialog({
+ ) } @@ -823,25 +1024,134 @@ function BulkTagDialog({ folderPath, folder, tags, + pendingTagChanges, + pendingTagRemovals, onClose, onAddTag, + onRemoveTag, + getPhotoTags, }: { folderPath: string folder: FolderGroup | undefined tags: TagResponse[] + pendingTagChanges: Record + pendingTagRemovals: Record onClose: () => void - onAddTag: (tagName: string) => void + onAddTag: (tagName: string) => Promise + onRemoveTag: (tagId: number) => Promise + getPhotoTags: (photoId: number) => Promise }) { const [selectedTagName, setSelectedTagName] = useState('') + const [folderTags, setFolderTags] = useState([]) + const [selectedTagIds, setSelectedTagIds] = useState>(new Set()) + const [showConfirmDialog, setShowConfirmDialog] = useState(false) - const handleAddTag = () => { - if (!selectedTagName.trim()) return - onAddTag(selectedTagName.trim()) - setSelectedTagName('') + useEffect(() => { + loadFolderTags() + }, [folder]) + + const loadFolderTags = async () => { + if (!folder || folder.photos.length === 0) { + setFolderTags([]) + return + } + + try { + // Get tags from the first photo in the folder + // Bulk tags should be the same for all photos in the folder + const response = await getPhotoTags(folder.photos[0].id) + // Filter for bulk tags (linkage_type = 1) + const bulkTags = (response.tags || []).filter((tag: any) => tag.linkage_type === 1) + setFolderTags(bulkTags) + } catch (error) { + console.error('Failed to load folder tags:', error) + setFolderTags([]) + } } + const handleAddTag = async () => { + if (!selectedTagName.trim()) return + const tagName = selectedTagName.trim() + await onAddTag(tagName) + setSelectedTagName('') + + // Update local state immediately to reflect the change + const tag = tags.find(t => t.tag_name.toLowerCase() === tagName.toLowerCase()) + if (tag) { + const newTag = { tag_id: tag.id, tag_name: tag.tag_name, linkage_type: 1 } + setFolderTags(prev => { + // Check if tag already exists + if (prev.some(t => t.tag_id === tag.id)) { + return prev + } + return [...prev, newTag] + }) + } + } + + const handleRemoveTags = () => { + if (selectedTagIds.size === 0) return + setShowConfirmDialog(true) + } + + const confirmRemoveTags = async () => { + setShowConfirmDialog(false) + if (selectedTagIds.size === 0) return + const tagIdsToRemove = Array.from(selectedTagIds) + for (const tagId of tagIdsToRemove) { + await onRemoveTag(tagId) + } + setSelectedTagIds(new Set()) + + // Update local state immediately to reflect the removal + setFolderTags(prev => prev.filter(t => !tagIdsToRemove.includes(t.tag_id))) + } + + // Combine saved and pending tags + const allTags = useMemo(() => { + if (!folder || folder.photos.length === 0) return [] + + // Get pending changes and removals for the first photo (they should be the same for all) + const firstPhotoId = folder.photos[0].id + const pendingTagIds = pendingTagChanges[firstPhotoId] || [] + const pendingRemovalIds = pendingTagRemovals[firstPhotoId] || [] + + // Filter out pending removals from saved tags + const savedTags = folderTags.filter(t => !pendingRemovalIds.includes(t.tag_id)) + + // Add pending tags that are bulk (linkage_type = 1) + const pendingTagObjs = pendingTagIds + .filter(id => !pendingRemovalIds.includes(id)) + .map(id => { + const tag = tags.find(t => t.id === id) + return tag ? { tag_id: id, tag_name: tag.tag_name, linkage_type: 1 } : null + }) + .filter(Boolean) as any[] + + return [...savedTags, ...pendingTagObjs] + }, [folderTags, pendingTagChanges, pendingTagRemovals, tags, folder]) + + // Get selected tag names for confirmation message + const selectedTagNames = useMemo(() => { + return Array.from(selectedTagIds) + .map(id => { + const tag = allTags.find(t => t.tag_id === id) + return tag ? tag.tag_name : '' + }) + .filter(Boolean) + .join(', ') + }, [selectedTagIds, allTags]) + return ( -
+ <> + {showConfirmDialog && ( + setShowConfirmDialog(false)} + /> + )} +

Bulk Link Tags to Folder

@@ -852,33 +1162,74 @@ function BulkTagDialog({ )}
-
- -
- - -
+
+ + +
+ +
+ {allTags.length === 0 ? ( +

No bulk tags linked to this folder

+ ) : ( + allTags.map(tag => { + if (!folder || folder.photos.length === 0) return null + const firstPhotoId = folder.photos[0].id + const isPending = (pendingTagChanges[firstPhotoId] || []).includes(tag.tag_id) + const linkageType = tag.linkage_type || 1 + const canSelect = isPending || linkageType === 1 + + return ( +
+ { + const newSet = new Set(selectedTagIds) + if (e.target.checked) { + newSet.add(tag.tag_id) + } else { + newSet.delete(tag.tag_id) + } + setSelectedTagIds(newSet) + }} + disabled={!canSelect} + className="w-4 h-4" + /> + + {tag.tag_name} + {isPending ? ' (pending)' : ' (saved bulk)'} + +
+ ) + }) + )}
-
+
+
+ ) }