-
Notifications
You must be signed in to change notification settings - Fork 577
feat: Implement comprehensive album management system with TypeScript… #554
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Implement comprehensive album management system with TypeScript… #554
Conversation
… fixes Features Added: - Complete album CRUD operations (Create, Read, Update, Delete) - Album list view with grid layout and hidden album support - Album detail view with image display and selection capabilities - Add photos to existing albums functionality - Create new albums with selected photos - Photo selection mode with multi-select toolbar - Password-protected hidden albums support Components Added: - AlbumList: Grid view of all albums with filtering options - CreateAlbumDialog: Form for creating new albums with validation - AddToAlbumDialog: Interface for adding photos to existing albums - AlbumDetail: Detailed album view with photo grid and management - SelectionToolbar: Multi-select actions toolbar for photo operations Backend Integration: - Albums API endpoints and functions - Redux state management for albums and selection mode - Type-safe API calls with proper error handling - Image selection state management across components TypeScript Improvements: - Resolved 200+ TypeScript errors across the codebase - Custom type declarations for missing modules (react, react-redux, lucide-react, @tauri-apps/api) - Enhanced JSX type safety with proper IntrinsicAttributes - Consistent React namespace imports and hook usage patterns - Improved tsconfig.json with better module resolution UI/UX Enhancements: - Responsive album grid layout - Interactive photo selection with visual feedback - Seamless navigation between album views - Consistent design language with existing components - Loading states and error handling throughout Technical Improvements: - Redux state management for albums and selection - Proper separation of concerns between components - Reusable selection toolbar for multiple contexts - Type-safe event handlers and form validation - Custom hooks integration for API calls and mutations This implementation provides a complete foundation for album management in PictoPy while maintaining high code quality and TypeScript safety.
WalkthroughAdds a full Albums feature: API endpoints and typed client functions, Redux slice and selectors with store wiring, new Album UI components and dialogs (list, detail, create, edit, delete, add-to-album), selection-mode UI, route(s), and multiple TypeScript ambient/JSX declaration files plus tsconfig tweaks. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as AlbumList / AlbumDetail
participant Dialog as Create/Edit/Delete/AddToAlbumDialog
participant API as apiClient
participant EP as albumsEndpoints
participant Store as Redux Store
participant Info as InfoDialog
UI->>API: GET EP.getAllAlbums(showHidden?)
API-->>UI: GetAlbumsResponse
UI->>Store: dispatch setAlbums(albums)
UI->>Dialog: user opens Add/Edit/Create/Delete
Dialog->>API: POST/PUT/DELETE EP.* with payload
API-->>Dialog: APIResponse
alt success
Dialog->>Store: dispatch add/update/remove album
Dialog->>Info: show success
else error
Dialog->>Info: show error
end
sequenceDiagram
autonumber
participant Grid as Image Grid (Home)
participant Card as ImageCard
participant Store as Redux Store
participant Bar as SelectionToolbar
Grid->>Store: enableSelectionMode()
loop user selects images
Card->>Store: toggleImageSelection(imageId)
Store-->>Bar: selected count updates
end
Bar->>Store: selectAllImages() / clearSelectedImages()
Bar->>Store: disableSelectionMode()
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
|
1 similar comment
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 7
♻️ Duplicate comments (1)
frontend/src/api/api-functions/albums.ts (1)
5-10: Unify the Album type in a shared moduleAlbum is also declared in features/albumSlice.ts. Move to src/types/album.ts (or re-export one) to avoid drift.
🧹 Nitpick comments (12)
frontend/src/pages/Album/Album.tsx (3)
20-23: Implement delete confirmation logic.The
handleDeleteAlbumfunction only logs to console and has a TODO comment. Users need a confirmation dialog before deletion to prevent accidental data loss.Do you want me to generate an implementation for the delete confirmation dialog, or open a new issue to track this task?
25-28: Consider implementing album creation callback.The
handleAlbumCreatedcallback currently only logs to console. Consider implementing actual navigation or list refresh logic to improve UX after album creation.Possible implementations:
- Navigate to the newly created album:
navigate(\/albums/${albumId}`)`- Refresh the album list (may happen automatically via React Query cache invalidation)
44-47: Complete the EditAlbumDialog implementation.The edit dialog is currently just a placeholder div. This needs to be replaced with an actual
EditAlbumDialogcomponent similar toCreateAlbumDialog.Do you want me to generate the EditAlbumDialog component implementation, or open a new issue to track this task?
frontend/src/types/global.d.ts (2)
5-16: Reconsider global React object declaration.Declaring a global
Reactobject with hooks and methods may conflict with actual React imports and can cause confusion:
- Shadowing: If a file imports React (
import React from 'react'), the global declaration might shadow or conflict with the import- Type mismatch: The simplified signatures (e.g.,
useState: <T>(initialState: T | (() => T)) => [T, (value: T | ((prev: T) => T)) => void]) don't match React's actual types- Unnecessary with new JSX transform: Modern React with the automatic JSX runtime doesn't require global React
Consider removing the global
Reactobject declaration (lines 6-16) and rely on proper imports. If you need to support legacy code without imports, use module augmentation instead:-declare global { - const React: { - useState: <T>(initialState: T | (() => T)) => [T, (value: T | ((prev: T) => T)) => void]; - useEffect: (effect: () => void | (() => void), deps?: any[]) => void; - useCallback: <T extends (...args: any[]) => any>(callback: T, deps: any[]) => T; - useMemo: <T>(factory: () => T, deps: any[]) => T; - useRef: <T>(initialValue: T) => { current: T }; - Fragment: any; - Component: any; - PureComponent: any; - createElement: any; - };
18-63: Avoid duplicating React types.The
Reactnamespace redeclares many types that already exist in@types/react:
ReactNode,ComponentType,Key(lines 19-21)CSSProperties(lines 23-25) - already imported on line 3- Event interfaces:
FormEvent,ChangeEvent,MouseEvent(lines 27-35)- HTML attribute interfaces (lines 37-62)
This duplication can cause:
- Type conflicts: Different versions of the same types
- Maintenance burden: Updates must be synced with @types/react
- Confusion: Developers won't know which types to use
Remove the duplicate type declarations and rely on
@types/react. If you need to augment React types, use module augmentation:import 'react'; declare module 'react' { // Add any custom augmentations here }Verify that
@types/reactis installed:#!/bin/bash # Check if @types/react is installed cat package.json | jq '.devDependencies["@types/react"] // .dependencies["@types/react"]'frontend/tsconfig.json (1)
17-18: JSX setting with Vite plugin is correct
Using"jsx": "preserve"is appropriate since Vite’s React plugin transforms JSX;jsxImportSourceisn’t applied in this mode (safe but redundant). If you’d rather have TypeScript emit React’s automatic runtime, change to"jsx": "react-jsx"and keepjsxImportSource.frontend/src/features/albumSelectors.ts (1)
12-13: Memoize derived selectors to avoid unnecessary recomputations/re-rendersUse createSelector for count/hasSelected to keep referential stability and O(1) equality checks.
Apply:
+import { createSelector } from '@reduxjs/toolkit'; ... -export const selectSelectedImageCount = (state: RootState) => state.albums.selectedImageIds.length; +export const selectSelectedImageCount = createSelector( + (state: RootState) => state.albums.selectedImageIds, + (ids) => ids.length, +); ... -export const selectHasSelectedImages = (state: RootState) => - state.albums.selectedImageIds.length > 0; +export const selectHasSelectedImages = createSelector( + (state: RootState) => state.albums.selectedImageIds, + (ids) => ids.length > 0, +);Optional: add a parameterized selector to avoid recreating closures on every render:
// outside diff context export const selectIsImageSelectedById = createSelector( (state: RootState) => state.albums.selectedImageIds, (_: RootState, imageId: string) => imageId, (ids, imageId) => ids.includes(imageId), );Also applies to: 18-19
frontend/src/features/albumSlice.ts (3)
3-8: Avoid duplicating the Album type across modulesAlbum is also defined in api/api-functions/albums.ts. Centralize in a shared type (e.g., src/types/album.ts) and import everywhere to prevent drift.
100-104: Guard against duplicate IDs in selectAllImagesEnsure uniqueness to avoid accidental duplicates from callers.
- selectAllImages(state, action: PayloadAction<string[]>) { - state.selectedImageIds = action.payload; - }, + selectAllImages(state, action: PayloadAction<string[]>) { + state.selectedImageIds = Array.from(new Set(action.payload)); + },
32-36: Optional: clear stale selectedAlbum when setAlbums no longer contains itIf setAlbums replaces the list, selectedAlbum may point to a non-existent album.
Consider:
// outside diff context if (state.selectedAlbum && !action.payload.some(a => a.album_id === state.selectedAlbum!.album_id)) { state.selectedAlbum = null; }frontend/src/api/api-functions/albums.ts (2)
52-57: Prefer axios params over manual query stringsImproves encoding and consistency.
-export const fetchAllAlbums = async (showHidden = false): Promise<GetAlbumsResponse> => { - const response = await apiClient.get<GetAlbumsResponse>( - `${albumsEndpoints.getAllAlbums}?show_hidden=${showHidden}`, - ); +export const fetchAllAlbums = async (showHidden = false): Promise<GetAlbumsResponse> => { + const response = await apiClient.get<GetAlbumsResponse>( + albumsEndpoints.getAllAlbums, + { params: { show_hidden: showHidden } }, + ); return response.data; };
35-49: Response shape consistency with APIResponseYou mix top-level fields (albums, image_ids) with APIResponse.data in GetAlbumResponse. Standardize to one shape (e.g., always use data) for easier typing and client handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
frontend/src/api/api-functions/albums.ts(1 hunks)frontend/src/api/api-functions/index.ts(1 hunks)frontend/src/api/apiEndpoints.ts(1 hunks)frontend/src/app/store.ts(2 hunks)frontend/src/components/Album/AddToAlbumDialog.tsx(1 hunks)frontend/src/components/Album/AlbumDetail.tsx(1 hunks)frontend/src/components/Album/AlbumList.tsx(1 hunks)frontend/src/components/Album/CreateAlbumDialog.tsx(1 hunks)frontend/src/components/Album/index.ts(1 hunks)frontend/src/components/Media/ImageCard.tsx(6 hunks)frontend/src/components/Media/SelectionToolbar.tsx(1 hunks)frontend/src/constants/routes.ts(1 hunks)frontend/src/features/albumSelectors.ts(1 hunks)frontend/src/features/albumSlice.ts(1 hunks)frontend/src/pages/Album/Album.tsx(1 hunks)frontend/src/pages/Home/Home.tsx(4 hunks)frontend/src/routes/AppRoutes.tsx(3 hunks)frontend/src/types/declarations.d.ts(1 hunks)frontend/src/types/global.d.ts(1 hunks)frontend/src/types/jsx.d.ts(1 hunks)frontend/src/types/react.d.ts(1 hunks)frontend/tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (15)
frontend/src/routes/AppRoutes.tsx (2)
frontend/src/constants/routes.ts (1)
ROUTES(1-12)frontend/src/components/Album/AlbumDetail.tsx (1)
AlbumDetail(46-293)
frontend/src/pages/Album/Album.tsx (2)
frontend/src/components/Album/AlbumList.tsx (1)
AlbumList(36-197)frontend/src/components/Album/CreateAlbumDialog.tsx (1)
CreateAlbumDialog(29-231)
frontend/src/types/jsx.d.ts (1)
frontend/src/types/declarations.d.ts (5)
HTMLAttributes(25-31)ButtonHTMLAttributes(51-54)InputHTMLAttributes(33-38)FormHTMLAttributes(47-49)TextareaHTMLAttributes(40-45)
frontend/src/api/api-functions/albums.ts (4)
frontend/src/features/albumSlice.ts (2)
Album(3-8)updateAlbum(41-52)frontend/src/types/API.ts (1)
APIResponse(1-8)frontend/src/api/axiosConfig.ts (1)
apiClient(5-12)frontend/src/api/apiEndpoints.ts (1)
albumsEndpoints(26-36)
frontend/src/components/Album/AddToAlbumDialog.tsx (6)
frontend/src/features/albumSelectors.ts (1)
selectAlbums(4-4)frontend/src/hooks/useQueryExtension.ts (2)
usePictoQuery(80-108)usePictoMutation(26-78)frontend/src/api/api-functions/albums.ts (2)
fetchAllAlbums(52-57)addImagesToAlbum(114-124)frontend/src/features/infoDialogSlice.ts (1)
showInfoDialog(16-30)frontend/src/hooks/useMutationFeedback.tsx (1)
useMutationFeedback(60-135)frontend/src/features/albumSlice.ts (1)
setAlbums(32-36)
frontend/src/components/Album/AlbumDetail.tsx (13)
frontend/src/types/Media.ts (1)
Image(1-10)frontend/src/features/albumSelectors.ts (2)
selectIsSelectionMode(10-10)selectSelectedImageIds(11-11)frontend/src/features/imageSelectors.ts (1)
selectIsImageViewOpen(24-27)frontend/src/hooks/useQueryExtension.ts (1)
usePictoQuery(80-108)frontend/src/api/api-functions/albums.ts (2)
fetchAlbum(69-74)fetchAlbumImages(97-111)frontend/src/api/api-functions/images.ts (1)
fetchAllImages(5-10)frontend/src/features/loaderSlice.ts (2)
showLoader(17-20)hideLoader(21-24)frontend/src/features/albumSlice.ts (2)
setSelectedAlbum(67-69)enableSelectionMode(71-73)frontend/src/features/infoDialogSlice.ts (1)
showInfoDialog(16-30)frontend/src/features/imageSlice.ts (1)
setImages(22-26)frontend/src/components/Media/ImageCard.tsx (1)
ImageCard(22-126)frontend/src/components/Media/SelectionToolbar.tsx (1)
SelectionToolbar(37-183)frontend/src/components/Media/MediaView.tsx (1)
MediaView(26-192)
frontend/src/features/albumSlice.ts (1)
frontend/src/api/api-functions/albums.ts (1)
Album(5-10)
frontend/src/components/Album/CreateAlbumDialog.tsx (5)
frontend/src/hooks/useQueryExtension.ts (1)
usePictoMutation(26-78)frontend/src/api/api-functions/albums.ts (2)
CreateAlbumRequest(12-17)createAlbum(60-66)frontend/src/features/albumSlice.ts (1)
addAlbum(37-40)frontend/src/features/infoDialogSlice.ts (1)
showInfoDialog(16-30)frontend/src/hooks/useMutationFeedback.tsx (1)
useMutationFeedback(60-135)
frontend/src/types/global.d.ts (1)
frontend/src/types/declarations.d.ts (7)
FormEvent(12-17)ChangeEvent(19-21)MouseEvent(23-23)HTMLAttributes(25-31)InputHTMLAttributes(33-38)TextareaHTMLAttributes(40-45)FormHTMLAttributes(47-49)
frontend/src/pages/Home/Home.tsx (6)
frontend/src/features/imageSelectors.ts (2)
selectIsImageViewOpen(24-27)selectImages(5-7)frontend/src/features/albumSelectors.ts (2)
selectIsSelectionMode(10-10)selectSelectedImageIds(11-11)frontend/src/features/albumSlice.ts (2)
enableSelectionMode(71-73)disableSelectionMode(74-77)frontend/src/components/Media/SelectionToolbar.tsx (1)
SelectionToolbar(37-183)frontend/src/components/Album/CreateAlbumDialog.tsx (1)
CreateAlbumDialog(29-231)frontend/src/components/Album/AddToAlbumDialog.tsx (1)
AddToAlbumDialog(36-236)
frontend/src/components/Media/ImageCard.tsx (3)
frontend/src/features/albumSelectors.ts (2)
selectIsSelectionMode(10-10)selectIsImageSelected(15-16)frontend/src/features/albumSlice.ts (1)
toggleImageSelection(78-86)frontend/src/features/imageSlice.ts (1)
setCurrentViewIndex(31-40)
frontend/src/types/react.d.ts (1)
frontend/src/types/declarations.d.ts (4)
HTMLAttributes(25-31)FormEvent(12-17)ChangeEvent(19-21)MouseEvent(23-23)
frontend/src/components/Album/AlbumList.tsx (7)
frontend/src/types/Album.ts (1)
AlbumListProps(53-60)frontend/src/features/albumSelectors.ts (1)
selectAlbums(4-4)frontend/src/hooks/useQueryExtension.ts (1)
usePictoQuery(80-108)frontend/src/api/api-functions/albums.ts (1)
fetchAllAlbums(52-57)frontend/src/features/loaderSlice.ts (2)
showLoader(17-20)hideLoader(21-24)frontend/src/features/infoDialogSlice.ts (1)
showInfoDialog(16-30)frontend/src/features/albumSlice.ts (1)
setAlbums(32-36)
frontend/src/components/Media/SelectionToolbar.tsx (3)
frontend/src/features/albumSelectors.ts (2)
selectSelectedImageCount(12-12)selectSelectedImageIds(11-11)frontend/src/features/imageSelectors.ts (1)
selectImages(5-7)frontend/src/features/albumSlice.ts (3)
disableSelectionMode(74-77)clearSelectedImages(103-105)selectAllImages(100-102)
frontend/src/features/albumSelectors.ts (1)
frontend/src/app/store.ts (1)
RootState(24-24)
🪛 Biome (2.1.2)
frontend/src/types/react.d.ts
[error] 7-7: Shouldn't redeclare 'React'. Consider to delete it or rename it.
'React' is defined here:
(lint/suspicious/noRedeclare)
🔇 Additional comments (13)
frontend/tsconfig.json (3)
9-9: LGTM: Synthetic default imports enabled.Enabling
allowSyntheticDefaultImportsimproves interoperability with CommonJS modules and is a good practice for modern TypeScript projects.
25-25: LGTM: Consistent casing enforcement added.Adding
forceConsistentCasingInFileNamesprevents cross-platform issues with case-sensitive file systems.
33-33: LGTM: Type declarations included.Expanding
includeto cover"src/types/**/*"properly incorporates the new ambient type declaration files added in this PR.frontend/src/constants/routes.ts (1)
9-9: LGTM: Album detail route added.The new
ALBUM_DETAILroute follows the existing pattern and correctly includes the dynamic:albumIdparameter for album-specific views.frontend/src/api/api-functions/index.ts (1)
7-7: LGTM: Albums API exported.The new albums API module is properly exported following the existing pattern for other API function modules.
frontend/src/app/store.ts (1)
9-9: LGTM: Album slice integrated.The
albumReduceris properly imported and wired into the Redux store, following the established pattern for other slices. TheRootStatetype will automatically include thealbumsfield through type inference.Also applies to: 20-20
frontend/src/pages/Album/Album.tsx (2)
1-3: LGTM: Clean imports.The imports are well-organized and use the centralized component exports from
@/components/Album.
7-9: LGTM: Appropriate local state.Using local React state for dialog visibility and selection is the correct pattern for transient UI state.
frontend/src/routes/AppRoutes.tsx (2)
14-14: LGTM: Simplified function signature.Removing the
React.FCtype annotation in favor of a plain function is a modern React best practice that provides better type inference for props.
11-12: LGTM: Album routes properly configured.Both the album list route (
ALBUMS) and detail route (ALBUM_DETAIL) are correctly configured:
- Album list renders the
Albumcomponent- Album detail renders
AlbumDetailwith:albumIdparameterThe routing structure supports the complete album navigation flow.
Also applies to: 23-24
frontend/src/components/Album/index.ts (1)
1-4: LGTM: Clean barrel exports.The index file provides a centralized entry point for album-related components, following the barrel export pattern and improving import ergonomics.
frontend/src/features/albumSelectors.ts (1)
4-11: Reducer key ‘albums’ is correctly registered in the root store.frontend/src/api/api-functions/albums.ts (1)
137-148: DELETE with request body is supported
Backend routeDELETE /albums/{album_id}/imagesaccepts anImageIdsRequestbody payload.
| selectedImageIds = [], | ||
| onAlbumCreated, | ||
| }: CreateAlbumDialogProps) { | ||
| const dispatch = useDispatch(); | ||
| const [albumName, setAlbumName] = React.useState(''); | ||
| const [description, setDescription] = React.useState(''); | ||
| const [isHidden, setIsHidden] = React.useState(false); | ||
| const [password, setPassword] = React.useState(''); | ||
| const [errors, setErrors] = React.useState<Record<string, string>>({}); | ||
|
|
||
| const createAlbumMutation = usePictoMutation({ | ||
| mutationFn: (data: CreateAlbumRequest) => createAlbum(data), | ||
| onSuccess: (data: any) => { | ||
| if (data.success) { | ||
| // Add album to Redux store | ||
| dispatch( | ||
| addAlbum({ | ||
| album_id: data.album_id, | ||
| album_name: albumName, | ||
| description: description, | ||
| is_hidden: isHidden, | ||
| }), | ||
| ); | ||
|
|
||
| // Show success message | ||
| dispatch( | ||
| showInfoDialog({ | ||
| title: 'Success', | ||
| message: `Album "${albumName}" created successfully!`, | ||
| variant: 'info', | ||
| }), | ||
| ); | ||
|
|
||
| // Call callback with album ID | ||
| onAlbumCreated?.(data.album_id); | ||
|
|
||
| // Reset form and close dialog | ||
| resetForm(); | ||
| onOpenChange(false); | ||
| } | ||
| }, | ||
| onError: (_error: any) => { | ||
| dispatch( | ||
| showInfoDialog({ | ||
| title: 'Error', | ||
| message: 'Failed to create album. Please try again.', | ||
| variant: 'error', | ||
| }), | ||
| ); | ||
| }, | ||
| }); | ||
|
|
||
| // Use mutation feedback hook | ||
| useMutationFeedback(createAlbumMutation, { | ||
| loadingMessage: 'Creating album...', | ||
| showSuccess: false, // We handle success manually above | ||
| errorTitle: 'Failed to Create Album', | ||
| }); | ||
|
|
||
| const resetForm = () => { | ||
| setAlbumName(''); | ||
| setDescription(''); | ||
| setIsHidden(false); | ||
| setPassword(''); | ||
| setErrors({}); | ||
| }; | ||
|
|
||
| const validateForm = (): boolean => { | ||
| const newErrors: Record<string, string> = {}; | ||
|
|
||
| if (!albumName.trim()) { | ||
| newErrors.albumName = 'Album name is required'; | ||
| } | ||
|
|
||
| if (isHidden && !password.trim()) { | ||
| newErrors.password = 'Password is required for hidden albums'; | ||
| } | ||
|
|
||
| setErrors(newErrors); | ||
| return Object.keys(newErrors).length === 0; | ||
| }; | ||
|
|
||
| const handleSubmit = (e: React.FormEvent) => { | ||
| e.preventDefault(); | ||
|
|
||
| if (!validateForm()) { | ||
| return; | ||
| } | ||
|
|
||
| const albumData: CreateAlbumRequest = { | ||
| name: albumName.trim(), | ||
| description: description.trim(), | ||
| is_hidden: isHidden, | ||
| password: isHidden ? password : undefined, | ||
| }; | ||
|
|
||
| createAlbumMutation.mutate(albumData); | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Selected photos are ignored when creating an album
selectedImageIds is accepted but never sent to the backend. The newly created album is always empty, breaking the “create album from selected photos” flow. Please either extend the create payload or trigger addImagesToAlbum after the album is created so the selected media lands in the new album.
frontend/src/types/declarations.d.ts
Outdated
| // Type declarations for missing modules | ||
|
|
||
| declare module 'react' { | ||
| import { ComponentType, ReactNode, CSSProperties } from 'react'; | ||
|
|
||
| export function useState<T>(initialState: T | (() => T)): [T, (value: T | ((prev: T) => T)) => void]; | ||
| export function useEffect(effect: () => void | (() => void), deps?: any[]): void; | ||
| export function useCallback<T extends (...args: any[]) => any>(callback: T, deps: any[]): T; | ||
| export function useMemo<T>(factory: () => T, deps: any[]): T; | ||
| export function useRef<T>(initialValue: T): { current: T }; | ||
|
|
||
| export interface FormEvent<T = Element> { | ||
| preventDefault(): void; | ||
| stopPropagation(): void; | ||
| currentTarget: T; | ||
| target: EventTarget; | ||
| } | ||
|
|
||
| export interface ChangeEvent<T = Element> extends FormEvent<T> { | ||
| target: EventTarget & T; | ||
| } | ||
|
|
||
| export interface MouseEvent<T = Element> extends FormEvent<T> {} | ||
|
|
||
| export interface HTMLAttributes<T> { | ||
| className?: string; | ||
| id?: string; | ||
| style?: CSSProperties; | ||
| onClick?: (event: MouseEvent<T>) => void; | ||
| children?: ReactNode; | ||
| } | ||
|
|
||
| export interface InputHTMLAttributes<T> extends HTMLAttributes<T> { | ||
| value?: string | number; | ||
| placeholder?: string; | ||
| onChange?: (event: ChangeEvent<T>) => void; | ||
| type?: string; | ||
| } | ||
|
|
||
| export interface TextareaHTMLAttributes<T> extends HTMLAttributes<T> { | ||
| value?: string; | ||
| placeholder?: string; | ||
| onChange?: (event: ChangeEvent<T>) => void; | ||
| rows?: number; | ||
| } | ||
|
|
||
| export interface FormHTMLAttributes<T> extends HTMLAttributes<T> { | ||
| onSubmit?: (event: FormEvent<T>) => void; | ||
| } | ||
|
|
||
| export interface ButtonHTMLAttributes<T> extends HTMLAttributes<T> { | ||
| type?: 'button' | 'submit' | 'reset'; | ||
| disabled?: boolean; | ||
| } | ||
|
|
||
| export default React; | ||
| } | ||
|
|
||
| declare module 'react-dom' { | ||
| import * as ReactDOM from 'react-dom'; | ||
| export = ReactDOM; | ||
| } | ||
|
|
||
| declare module 'react-redux' { | ||
| export function useSelector<T>(selector: (state: any) => T): T; | ||
| export function useDispatch(): any; | ||
| export function connect(mapStateToProps?: any, mapDispatchToProps?: any): any; | ||
| export const Provider: any; | ||
| } | ||
|
|
||
| declare module 'lucide-react' { | ||
| import { ComponentType, SVGProps } from 'react'; | ||
|
|
||
| export interface LucideProps extends Partial<Omit<SVGProps<SVGSVGElement>, "ref">> { | ||
| size?: string | number; | ||
| absoluteStrokeWidth?: boolean; | ||
| } | ||
|
|
||
| export type Icon = ComponentType<LucideProps>; | ||
|
|
||
| export const Check: Icon; | ||
| export const Heart: Icon; | ||
| export const Share2: Icon; | ||
| export const Eye: Icon; | ||
| export const EyeOff: Icon; | ||
| export const Lock: Icon; | ||
| export const Search: Icon; | ||
| export const X: Icon; | ||
| export const FolderPlus: Icon; | ||
| export const Trash2: Icon; | ||
| export const Download: Icon; | ||
| export const SelectAll: Icon; | ||
| export const RotateCcw: Icon; | ||
| export const CheckSquare: Icon; | ||
| export const ArrowLeft: Icon; | ||
| export const Settings: Icon; | ||
| export const Users: Icon; | ||
| export const Calendar: Icon; | ||
| export const Edit3: Icon; | ||
| export const MoreHorizontal: Icon; | ||
| export const Plus: Icon; | ||
| export const FolderOpen: Icon; | ||
| export const ImageIcon: Icon; | ||
| } | ||
|
|
||
| declare module '@tauri-apps/api/core' { | ||
| export function convertFileSrc(filePath: string, protocol?: string): string; | ||
| export function invoke(cmd: string, args?: Record<string, unknown>): Promise<any>; | ||
| } | ||
|
|
||
| declare module 'react/jsx-runtime' { | ||
| export const jsx: any; | ||
| export const jsxs: any; | ||
| export const Fragment: any; | ||
| } | ||
|
|
||
| declare module 'react/jsx-dev-runtime' { | ||
| export const jsx: any; | ||
| export const jsxs: any; | ||
| export const Fragment: any; | ||
| } | ||
|
|
||
| // Global JSX namespace | ||
| declare global { | ||
| namespace JSX { | ||
| interface IntrinsicElements { | ||
| [elemName: string]: any; | ||
| } | ||
|
|
||
| interface Element extends React.ReactElement<any, any> { } | ||
|
|
||
| interface ElementClass extends React.Component<any> { | ||
| render(): React.ReactNode; | ||
| } | ||
|
|
||
| interface ElementAttributesProperty { | ||
| props: {}; | ||
| } | ||
|
|
||
| interface ElementChildrenAttribute { | ||
| children: {}; | ||
| } | ||
|
|
||
| interface IntrinsicAttributes extends React.Attributes { } | ||
| interface IntrinsicClassAttributes<T> extends React.ClassAttributes<T> { } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Critical: Invalid and unsafe ambient typings for React/DOM/Redux; remove and use official types
- declare module 'react' and 'react-dom' import from themselves and export symbols that aren’t declared (e.g., default React), breaking type resolution.
- Global JSX depends on React.* types that aren’t declared here.
- These stubs shadow real package typings, degrading type safety and causing hard-to-debug errors across the app.
Replace with proper dependencies and configs.
Remediation checklist:
- Remove this file entirely.
- Install official types (only where needed):
- React 18/19: npm i -D @types/react @types/react-dom (if not already bundled; react-redux has built-in types).
- lucide-react and @tauri-apps/api ship types; no stubs needed.
- Ensure tsconfig:
- "jsx": "react-jsx"
- "lib": ["ES2022","DOM","DOM.Iterable"]
- Avoid "skipLibCheck": true only as last resort.
Quick verification:
If you must keep ambient declarations, limit them to truly missing modules and do not redeclare 'react', 'react-dom', or 'react/jsx-runtime'.
🏁 Script executed:
#!/bin/bash
# Show any ambient declarations that shadow core libs
rg -n "declare module 'react'|declare module 'react-dom'|declare module 'react/jsx-runtime'|declare module 'react-redux'|declare module 'lucide-react'"
# Inspect package.json for correct deps and types
fd -a package.json | head -n 1 | xargs cat | sed -n '1,200p' | sed -n '1,200p'
# Show tsconfig jsx and libs
fd -a tsconfig*.json
for f in $(fd -a tsconfig*.json); do echo "== $f =="; cat "$f"; echo; doneLength of output: 5071
Remove unsafe ambient declarations for core libraries
- The file
frontend/src/types/declarations.d.tsdeclares modules for ‘react’, ‘react-dom’, ‘react-redux’, ‘react/jsx-runtime’, etc., shadowing official typings and breaking type resolution. Delete this file and rely on the installed@types/react/@types/react-domand built-in types from your dependencies. - Only add ambient stubs for truly missing modules (e.g. Tauri APIs), never for core React/JSX/Redux packages.
🤖 Prompt for AI Agents
In frontend/src/types/declarations.d.ts lines 1-147: this file contains unsafe
ambient declarations shadowing core libraries (react, react-dom, react-redux,
react/jsx-runtime) which break type resolution; remove those React/Redux/JSX
ambient module declarations entirely (delete the file or strip out only the core
library declarations) and keep only true missing-module stubs (e.g.
@tauri-apps/api/core) if needed; after removal ensure your project depends on
proper typings (install or enable @types/react, @types/react-dom or rely on
bundled types) and update tsconfig.json types/include if necessary so TypeScript
picks up the official React/JSX typings.
|
|
3 similar comments
|
|
|
|
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/components/Album/AddToAlbumDialog.tsx (1)
106-109: Guard album names before lowercasingWe already coerce
descriptionto an empty string; do the same foralbum_nameso a missing or null value from the backend can’t crash the dialog during search.- const filteredAlbums = albums.filter((album: any) => - album.album_name.toLowerCase().includes(searchTerm.toLowerCase()) || + const filteredAlbums = albums.filter((album: any) => + (album.album_name ?? '').toLowerCase().includes(searchTerm.toLowerCase()) || (album.description ?? '').toLowerCase().includes(searchTerm.toLowerCase()) );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/components/Album/AddToAlbumDialog.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/src/components/Album/AddToAlbumDialog.tsx (6)
frontend/src/features/albumSelectors.ts (1)
selectAlbums(4-4)frontend/src/hooks/useQueryExtension.ts (2)
usePictoQuery(80-108)usePictoMutation(26-78)frontend/src/api/api-functions/albums.ts (2)
fetchAllAlbums(52-57)addImagesToAlbum(114-124)frontend/src/features/infoDialogSlice.ts (1)
showInfoDialog(16-30)frontend/src/hooks/useMutationFeedback.tsx (1)
useMutationFeedback(60-135)frontend/src/features/albumSlice.ts (1)
setAlbums(32-36)
… - Add EditAlbumDialog for editing album details with password management - Add DeleteAlbumDialog for safe album deletion with confirmation - Update Album.tsx, AlbumList.tsx, and AlbumDetail.tsx for full CRUD - Fix TypeScript duplicate type declaration errors - Fix FormEvent type incompatibility issue - Fix ESLint warnings - All TypeScript and ESLint errors resolved. Fixes #554
|
|
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/package.json (1)
56-57: Upgrade TypeScript to meet React 19 typing requirements.React 19 (and
@types/react@19.x) require at least TypeScript 5.3. With the new"type-check": "tsc --noEmit"script, the current"typescript": "5.1.x"will cause immediate compiler errors. Please bump the compiler to a supported version before merging.- "typescript": "5.1.x", + "typescript": "^5.6.3",Also applies to: 100-100
♻️ Duplicate comments (1)
frontend/src/types/declarations.d.ts (1)
1-124: Remove unsafe React ambient stubs.Re-declaring core packages (
react,react-dom,react-redux,react/jsx-runtime,lucide-react) with these minimal stubs shadows the official typings, so TypeScript can no longer surface realReact.DetailedHTMLProps, hook overloads, or JSX intrinsic element contracts—exactly the regression that was flagged earlier. This breaks type resolution across the new JSX files and collapses safety toany. Rip out the custom React/Redux/icon module declarations and rely on the upstream@types/react,@types/react-dom, and built-in package typings instead; keep only true missing-module shims if absolutely required.-// React type declarations for when @types/react is not available -declare module 'react' { - ... -} - -declare module 'react-dom' { - ... -} - -declare module 'react/jsx-runtime' { - ... -} - -// React Redux types -declare module 'react-redux' { - ... -} - -// Lucide React types -declare module 'lucide-react' { - ... -} - -// JSX namespace for intrinsic elements -declare namespace JSX { - ... -}Then ensure the project depends on the official type packages (or upgrades to React 19 which ships types) so
frontend/src/types/jsx.d.tscan reference the real React definitions without falling back to unsafeany.
🧹 Nitpick comments (3)
frontend/src/components/Media/SelectionToolbar.tsx (1)
59-66: Type the image shape for safety
allImages.map((image: any) => image.id)and the derivedisAllSelectedrely on a loosely typedany. Please type the selector (e.g., returnImage[]) or coerce to a typed helper so we catch missingidproperties at compile time and avoid runtime surprises.frontend/src/components/Album/AddToAlbumDialog.tsx (1)
135-250: Reset dialog state on close eventsWhen the dialog closes via backdrop/escape (onOpenChange(false)),
handleClosedoesn’t run, so we keep stale selection/search values. WraponOpenChangeto callhandleClosewhen closing so the dialog always resets.frontend/src/api/api-functions/albums.ts (1)
52-59: Encode boolean query parameterInterpolating
showHiddendirectly can producetrue/false, but URLSearchParams handles encoding more robustly (?show_hidden=true). Using template strings works yet misses encoding for future multi-value params. Prefernew URLSearchParams({ show_hidden: String(showHidden) }).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (23)
ALBUM_MANAGEMENT_IMPLEMENTATION.md(1 hunks)frontend/package.json(2 hunks)frontend/src/api/api-functions/albums.ts(1 hunks)frontend/src/api/api-functions/index.ts(1 hunks)frontend/src/app/store.ts(2 hunks)frontend/src/components/Album/AddToAlbumDialog.tsx(1 hunks)frontend/src/components/Album/AlbumDetail.tsx(1 hunks)frontend/src/components/Album/AlbumList.tsx(1 hunks)frontend/src/components/Album/CreateAlbumDialog.tsx(1 hunks)frontend/src/components/Album/DeleteAlbumDialog.tsx(1 hunks)frontend/src/components/Album/EditAlbumDialog.tsx(1 hunks)frontend/src/components/Album/index.ts(1 hunks)frontend/src/components/Media/ImageCard.tsx(6 hunks)frontend/src/components/Media/SelectionToolbar.tsx(1 hunks)frontend/src/features/albumSlice.ts(1 hunks)frontend/src/pages/Album/Album.tsx(1 hunks)frontend/src/pages/Home/Home.tsx(4 hunks)frontend/src/routes/AppRoutes.tsx(2 hunks)frontend/src/types/declarations.d.ts(1 hunks)frontend/src/types/global.d.ts(1 hunks)frontend/src/types/jsx.d.ts(1 hunks)frontend/src/types/react.d.ts(1 hunks)frontend/tsconfig.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/src/api/api-functions/index.ts
- frontend/src/features/albumSlice.ts
- frontend/src/types/global.d.ts
🧰 Additional context used
🧬 Code graph analysis (13)
frontend/src/routes/AppRoutes.tsx (3)
frontend/src/constants/routes.ts (1)
ROUTES(1-12)frontend/src/components/Album/AlbumDetail.tsx (1)
AlbumDetail(55-352)frontend/src/components/Album/index.ts (1)
AlbumDetail(2-2)
frontend/src/types/jsx.d.ts (1)
frontend/src/types/declarations.d.ts (4)
ReactElement(40-44)Component(3-5)ReactNode(29-36)Key(38-38)
frontend/src/pages/Album/Album.tsx (4)
frontend/src/components/Album/AlbumList.tsx (1)
AlbumList(43-217)frontend/src/components/Album/CreateAlbumDialog.tsx (1)
CreateAlbumDialog(29-234)frontend/src/components/Album/EditAlbumDialog.tsx (1)
EditAlbumDialog(33-309)frontend/src/components/Album/DeleteAlbumDialog.tsx (1)
DeleteAlbumDialog(28-168)
frontend/src/components/Media/SelectionToolbar.tsx (3)
frontend/src/features/albumSelectors.ts (2)
selectSelectedImageCount(12-12)selectSelectedImageIds(11-11)frontend/src/features/imageSelectors.ts (1)
selectImages(5-7)frontend/src/features/albumSlice.ts (3)
disableSelectionMode(77-80)clearSelectedImages(106-108)selectAllImages(103-105)
frontend/src/components/Album/AlbumDetail.tsx (14)
frontend/src/types/Media.ts (1)
Image(1-10)frontend/src/features/albumSelectors.ts (2)
selectIsSelectionMode(10-10)selectSelectedImageIds(11-11)frontend/src/features/imageSelectors.ts (1)
selectIsImageViewOpen(24-27)frontend/src/hooks/useQueryExtension.ts (1)
usePictoQuery(80-108)frontend/src/api/api-functions/albums.ts (2)
fetchAlbum(73-80)fetchAlbumImages(103-117)frontend/src/api/api-functions/images.ts (1)
fetchAllImages(5-10)frontend/src/features/loaderSlice.ts (2)
showLoader(17-20)hideLoader(21-24)frontend/src/features/albumSlice.ts (3)
setSelectedAlbum(70-72)enableSelectionMode(74-76)setError(109-111)frontend/src/features/imageSlice.ts (1)
setImages(22-26)frontend/src/components/Media/ImageCard.tsx (1)
ImageCard(25-129)frontend/src/components/Media/SelectionToolbar.tsx (1)
SelectionToolbar(36-178)frontend/src/components/Media/MediaView.tsx (1)
MediaView(26-192)frontend/src/components/Album/EditAlbumDialog.tsx (1)
EditAlbumDialog(33-309)frontend/src/components/Album/DeleteAlbumDialog.tsx (1)
DeleteAlbumDialog(28-168)
frontend/src/pages/Home/Home.tsx (10)
frontend/src/features/imageSelectors.ts (2)
selectIsImageViewOpen(24-27)selectImages(5-7)frontend/src/features/albumSelectors.ts (2)
selectIsSelectionMode(10-10)selectSelectedImageIds(11-11)frontend/src/hooks/useQueryExtension.ts (1)
usePictoMutation(26-78)frontend/src/api/api-functions/albums.ts (1)
addImagesToAlbum(120-130)frontend/src/features/albumSlice.ts (2)
disableSelectionMode(77-80)enableSelectionMode(74-76)frontend/src/features/infoDialogSlice.ts (1)
showInfoDialog(16-30)frontend/src/components/Media/ImageCard.tsx (1)
ImageCard(25-129)frontend/src/components/Media/SelectionToolbar.tsx (1)
SelectionToolbar(36-178)frontend/src/components/Album/CreateAlbumDialog.tsx (1)
CreateAlbumDialog(29-234)frontend/src/components/Album/AddToAlbumDialog.tsx (1)
AddToAlbumDialog(32-251)
frontend/src/api/api-functions/albums.ts (3)
frontend/src/types/API.ts (1)
APIResponse(1-8)frontend/src/api/axiosConfig.ts (1)
apiClient(5-12)frontend/src/api/apiEndpoints.ts (1)
albumsEndpoints(26-36)
frontend/src/components/Album/AddToAlbumDialog.tsx (6)
frontend/src/features/albumSelectors.ts (1)
selectAlbums(4-4)frontend/src/hooks/useQueryExtension.ts (2)
usePictoQuery(80-108)usePictoMutation(26-78)frontend/src/api/api-functions/albums.ts (2)
fetchAllAlbums(52-59)addImagesToAlbum(120-130)frontend/src/features/infoDialogSlice.ts (1)
showInfoDialog(16-30)frontend/src/hooks/useMutationFeedback.tsx (1)
useMutationFeedback(60-135)frontend/src/features/albumSlice.ts (1)
setAlbums(32-36)
frontend/src/components/Album/CreateAlbumDialog.tsx (5)
frontend/src/hooks/useQueryExtension.ts (1)
usePictoMutation(26-78)frontend/src/api/api-functions/albums.ts (2)
CreateAlbumRequest(12-17)createAlbum(62-70)frontend/src/features/albumSlice.ts (1)
addAlbum(37-40)frontend/src/features/infoDialogSlice.ts (1)
showInfoDialog(16-30)frontend/src/hooks/useMutationFeedback.tsx (1)
useMutationFeedback(60-135)
frontend/src/components/Album/EditAlbumDialog.tsx (5)
frontend/src/hooks/useQueryExtension.ts (2)
usePictoQuery(80-108)usePictoMutation(26-78)frontend/src/api/api-functions/albums.ts (3)
fetchAlbum(73-80)UpdateAlbumRequest(19-25)updateAlbum(83-92)frontend/src/features/albumSlice.ts (1)
updateAlbum(41-55)frontend/src/features/infoDialogSlice.ts (1)
showInfoDialog(16-30)frontend/src/hooks/useMutationFeedback.tsx (1)
useMutationFeedback(60-135)
frontend/src/components/Album/DeleteAlbumDialog.tsx (5)
frontend/src/features/albumSlice.ts (2)
setError(109-111)removeAlbum(56-69)frontend/src/hooks/useQueryExtension.ts (2)
usePictoQuery(80-108)usePictoMutation(26-78)frontend/src/api/api-functions/albums.ts (2)
fetchAlbum(73-80)deleteAlbum(95-100)frontend/src/features/infoDialogSlice.ts (1)
showInfoDialog(16-30)frontend/src/hooks/useMutationFeedback.tsx (1)
useMutationFeedback(60-135)
frontend/src/components/Album/AlbumList.tsx (6)
frontend/src/features/albumSelectors.ts (1)
selectAlbums(4-4)frontend/src/hooks/useQueryExtension.ts (1)
usePictoQuery(80-108)frontend/src/api/api-functions/albums.ts (1)
fetchAllAlbums(52-59)frontend/src/features/loaderSlice.ts (2)
showLoader(17-20)hideLoader(21-24)frontend/src/features/infoDialogSlice.ts (1)
showInfoDialog(16-30)frontend/src/features/albumSlice.ts (1)
setAlbums(32-36)
frontend/src/components/Media/ImageCard.tsx (3)
frontend/src/features/albumSelectors.ts (2)
selectIsSelectionMode(10-10)selectIsImageSelected(15-16)frontend/src/features/albumSlice.ts (1)
toggleImageSelection(81-89)frontend/src/features/imageSlice.ts (1)
setCurrentViewIndex(31-40)
| ✅ **Add Images to Album** - Already implemented in `AddToAlbumDialog.tsx` | ||
| ✅ **Remove Images from Album** - Already implemented in `AlbumDetail.tsx` | ||
| ✅ **Hidden Album Support** - Password protection for edit/delete |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documented “Remove Images from Album” isn’t implemented
This checklist says the removal flow already exists in AlbumDetail.tsx, but the code still just logs inside handleRemoveFromAlbum. Please either wire up the actual removal mutation or adjust the doc so it doesn’t over-promise functionality that’s missing.
🤖 Prompt for AI Agents
ALBUM_MANAGEMENT_IMPLEMENTATION.md lines 113-115: the doc claims "Remove Images
from Album" is implemented in AlbumDetail.tsx but the handler only logs; either
implement the actual removal or update the doc. To implement: open
AlbumDetail.tsx, find handleRemoveFromAlbum, wire it to the
removeImagesFromAlbum GraphQL/REST mutation (or existing API client), pass the
album ID and image IDs, perform optimistic UI update to remove images from
state, handle success/failure and show user feedback, and ensure
permissions/checks are respected; or if you choose not to implement now, update
ALBUM_MANAGEMENT_IMPLEMENTATION.md to remove the check or mark it as
"planned/not implemented" with a TODO linking to the PR/issue.
| albumId: string, | ||
| imageIds: string[], | ||
| ): Promise<APIResponse> => { | ||
| const requestBody: ImageIdsRequest = { image_ids: imageIds }; | ||
| const response = await apiClient.delete<APIResponse>( | ||
| albumsEndpoints.removeImagesFromAlbum(albumId), | ||
| { data: requestBody }, | ||
| ); | ||
| return response.data; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Confirm backend accepts DELETE with body
axios.delete sends { data: requestBody }, but many servers drop bodies on DELETE. Verify the backend expects JSON here; otherwise the request will silently omit payload and fail to remove images.
🤖 Prompt for AI Agents
In frontend/src/api/api-functions/albums.ts around lines 145–154, the client is
calling axios.delete with a request body, but many servers ignore DELETE bodies;
verify whether the backend route actually expects and reads a JSON body on
DELETE. If the backend does accept a body, ensure the client sends Content-Type:
application/json and keep the current call; if the backend does not, change the
contract: update the client to send image IDs via an accepted mechanism (e.g.,
POST to a remove-images endpoint or include image_ids as query parameters)
and/or update the server route to accept a JSON body for this operation, then
update tests and docs accordingly so the remove-images call reliably transmits
the image IDs.
| onSuccess: (data: any) => { | ||
| if (data.success) { | ||
| const selectedAlbum = albums.find( | ||
| (album: any) => album.album_id === selectedAlbumId, | ||
| ); | ||
| dispatch( | ||
| showInfoDialog({ | ||
| title: 'Success', | ||
| message: `Added ${selectedImageIds.length} photo${ | ||
| selectedImageIds.length > 1 ? 's' : '' | ||
| } to "${selectedAlbum?.album_name}"`, | ||
| variant: 'info', | ||
| }), | ||
| ); | ||
|
|
||
| onImagesAdded?.(); | ||
| handleClose(); | ||
| } | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Guard selected album lookup against stale state
Inside onSuccess, albums.find can return undefined because the slice may refresh between mutation initiation and completion. If that happens we show 'undefined' in the dialog. Capture the album display name before calling mutate (or re-fetch from response) to guarantee a stable label.
🤖 Prompt for AI Agents
In frontend/src/components/Album/AddToAlbumDialog.tsx around lines 64 to 82, the
albums.find lookup inside onSuccess can return undefined if the albums slice has
changed, causing "undefined" to show in the dialog; capture the album display
name before calling mutate (e.g. read selected album name into a local variable
when the dialog opens or immediately before starting the mutation) and use that
stable string in the success message, or alternatively extract a reliable
album_name from the server response and fall back to a sensible default; update
the code to use this captured/fallback name instead of calling albums.find
inside the onSuccess handler.
| onError: () => { | ||
| dispatch( | ||
| showInfoDialog({ | ||
| title: 'Error', | ||
| message: 'Failed to add photos to album. Please try again.', | ||
| variant: 'error', | ||
| }), | ||
| ); | ||
| }, | ||
| }); | ||
|
|
||
| // Use mutation feedback | ||
| useMutationFeedback(addToAlbumMutation, { | ||
| loadingMessage: 'Adding photos to album...', | ||
| showSuccess: false, // We handle success manually | ||
| errorTitle: 'Failed to Add Photos', | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Disable duplicate error dialogs
useMutationFeedback still shows its default error dialog while you dispatch another in onError, so users see two modals. Set showError: false (or drop the manual dispatch) to avoid duplicate messaging. This mirrors the earlier review feedback.
🤖 Prompt for AI Agents
In frontend/src/components/Album/AddToAlbumDialog.tsx around lines 83 to 99, the
mutation error is causing duplicate dialogs because onError manually dispatches
an error dialog while useMutationFeedback also shows its default error dialog;
disable the built-in error dialog by adding showError: false to the
useMutationFeedback call (or alternatively remove the manual dispatch in
onError) so only one error UI is presented.
| const filteredAlbums = albums.filter( | ||
| (album: any) => | ||
| album.album_name.toLowerCase().includes(searchTerm.toLowerCase()) || | ||
| album.description.toLowerCase().includes(searchTerm.toLowerCase()), | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle missing album text fields
album.album_name or album.description may be null/undefined (backend allows it). Calling .toLowerCase() will throw. Default to empty strings before lowercasing, e.g. (album.album_name ?? '').toLowerCase().
🤖 Prompt for AI Agents
In frontend/src/components/Album/AddToAlbumDialog.tsx around lines 109 to 113,
the filter uses album.album_name.toLowerCase() and
album.description.toLowerCase() which can throw if those fields are
null/undefined; update the expressions to default to empty strings before
lowercasing (e.g. use (album.album_name ?? '') and (album.description ?? '') or
equivalent) so the filter safely handles missing text fields and performs the
case-insensitive match.
| // Handle loading states | ||
| React.useEffect(() => { | ||
| if (isLoading) { | ||
| dispatch(showLoader('Loading albums...')); | ||
| } else if (isError) { | ||
| dispatch(hideLoader()); | ||
| dispatch( | ||
| showInfoDialog({ | ||
| title: 'Error', | ||
| message: 'Failed to load albums. Please try again later.', | ||
| variant: 'error', | ||
| }), | ||
| ); | ||
| } else if (isSuccess && data?.success) { | ||
| dispatch(setAlbums(data.albums)); | ||
| dispatch(hideLoader()); | ||
| } | ||
| }, [data, isSuccess, isError, isLoading, dispatch]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle API “success = false” responses so the loader hides
Right now, if the request resolves (react-query marks it successful) but data.success is false, we fall through the effect without hiding the loader or surfacing an error. The global loader stays stuck and the UI never recovers. Please add a branch that always hides the loader when the fetch settles and shows an error dialog when data.success is false.
- React.useEffect(() => {
- if (isLoading) {
- dispatch(showLoader('Loading albums...'));
- } else if (isError) {
- dispatch(hideLoader());
- dispatch(
- showInfoDialog({
- title: 'Error',
- message: 'Failed to load albums. Please try again later.',
- variant: 'error',
- }),
- );
- } else if (isSuccess && data?.success) {
- dispatch(setAlbums(data.albums));
- dispatch(hideLoader());
- }
- }, [data, isSuccess, isError, isLoading, dispatch]);
+ React.useEffect(() => {
+ if (isLoading) {
+ dispatch(showLoader('Loading albums...'));
+ return;
+ }
+
+ dispatch(hideLoader());
+
+ if (isError) {
+ dispatch(
+ showInfoDialog({
+ title: 'Error',
+ message: 'Failed to load albums. Please try again later.',
+ variant: 'error',
+ }),
+ );
+ return;
+ }
+
+ if (isSuccess) {
+ if (data?.success) {
+ dispatch(setAlbums(data.albums));
+ } else {
+ dispatch(
+ showInfoDialog({
+ title: 'Error',
+ message:
+ data?.message ?? 'Failed to load albums. Please try again later.',
+ variant: 'error',
+ }),
+ );
+ }
+ }
+ }, [data, isSuccess, isError, isLoading, dispatch]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Handle loading states | |
| React.useEffect(() => { | |
| if (isLoading) { | |
| dispatch(showLoader('Loading albums...')); | |
| } else if (isError) { | |
| dispatch(hideLoader()); | |
| dispatch( | |
| showInfoDialog({ | |
| title: 'Error', | |
| message: 'Failed to load albums. Please try again later.', | |
| variant: 'error', | |
| }), | |
| ); | |
| } else if (isSuccess && data?.success) { | |
| dispatch(setAlbums(data.albums)); | |
| dispatch(hideLoader()); | |
| } | |
| }, [data, isSuccess, isError, isLoading, dispatch]); | |
| // Handle loading states | |
| React.useEffect(() => { | |
| if (isLoading) { | |
| dispatch(showLoader('Loading albums...')); | |
| return; | |
| } | |
| dispatch(hideLoader()); | |
| if (isError) { | |
| dispatch( | |
| showInfoDialog({ | |
| title: 'Error', | |
| message: 'Failed to load albums. Please try again later.', | |
| variant: 'error', | |
| }), | |
| ); | |
| return; | |
| } | |
| if (isSuccess) { | |
| if (data?.success) { | |
| dispatch(setAlbums(data.albums)); | |
| } else { | |
| dispatch( | |
| showInfoDialog({ | |
| title: 'Error', | |
| message: | |
| data?.message ?? 'Failed to load albums. Please try again later.', | |
| variant: 'error', | |
| }), | |
| ); | |
| } | |
| } | |
| }, [data, isSuccess, isError, isLoading, dispatch]); |
🤖 Prompt for AI Agents
In frontend/src/components/Album/AlbumList.tsx around lines 58 to 75, the effect
handles isLoading/isError/isSuccess but does not handle the case where
react-query marks the request successful yet data.success is false, leaving the
global loader visible and no error shown; update the effect to always call
dispatch(hideLoader()) when the request is no longer loading (in all branches),
and add a branch under isSuccess that checks if data?.success is false to
dispatch showInfoDialog with an error message (and do not call setAlbums),
otherwise proceed as existing when data.success is true.
| }, | ||
| onError: (_error: any) => { | ||
| dispatch( | ||
| showInfoDialog({ | ||
| title: 'Error', | ||
| message: 'Failed to create album. Please try again.', | ||
| variant: 'error', | ||
| }), | ||
| ); | ||
| }, | ||
| }); | ||
|
|
||
| // Use mutation feedback hook | ||
| useMutationFeedback(createAlbumMutation, { | ||
| loadingMessage: 'Creating album...', | ||
| showSuccess: false, // We handle success manually above | ||
| errorTitle: 'Failed to Create Album', | ||
| }); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prevent duplicate error popups
onError already pushes a detailed InfoDialog, but useMutationFeedback still has showError enabled, so failures trigger a second, generic dialog right after. Disable the default error handling (or drop the manual dispatch) to avoid clobbering the informative message.
[suggested fix]
- useMutationFeedback(createAlbumMutation, {
- loadingMessage: 'Creating album...',
- showSuccess: false, // We handle success manually above
- errorTitle: 'Failed to Create Album',
- });
+ useMutationFeedback(createAlbumMutation, {
+ loadingMessage: 'Creating album...',
+ showSuccess: false, // We handle success manually above
+ showError: false,
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| }, | |
| onError: (_error: any) => { | |
| dispatch( | |
| showInfoDialog({ | |
| title: 'Error', | |
| message: 'Failed to create album. Please try again.', | |
| variant: 'error', | |
| }), | |
| ); | |
| }, | |
| }); | |
| // Use mutation feedback hook | |
| useMutationFeedback(createAlbumMutation, { | |
| loadingMessage: 'Creating album...', | |
| showSuccess: false, // We handle success manually above | |
| errorTitle: 'Failed to Create Album', | |
| }); | |
| }, | |
| onError: (_error: any) => { | |
| dispatch( | |
| showInfoDialog({ | |
| title: 'Error', | |
| message: 'Failed to create album. Please try again.', | |
| variant: 'error', | |
| }), | |
| ); | |
| }, | |
| }); | |
| // Use mutation feedback hook | |
| useMutationFeedback(createAlbumMutation, { | |
| loadingMessage: 'Creating album...', | |
| showSuccess: false, // We handle success manually above | |
| showError: false, | |
| }); |
🤖 Prompt for AI Agents
In frontend/src/components/Album/CreateAlbumDialog.tsx around lines 72 to 90,
the component currently dispatches a detailed InfoDialog in onError while
useMutationFeedback still displays its default error dialog, causing duplicate
error popups; fix by disabling the hook's automatic error handling (set
showError: false in the useMutationFeedback options) OR remove the manual
dispatch in onError so only one error dialog is shown, and update related props
(errorTitle) accordingly.
| onError: (_error: any) => { | ||
| dispatch( | ||
| showInfoDialog({ | ||
| title: 'Error', | ||
| message: | ||
| 'Failed to update album. Please check your password and try again.', | ||
| variant: 'error', | ||
| }), | ||
| ); | ||
| }, | ||
| }); | ||
|
|
||
| // Use mutation feedback hook | ||
| useMutationFeedback(updateAlbumMutation, { | ||
| loadingMessage: 'Updating album...', | ||
| showSuccess: false, // We handle success manually above | ||
| errorTitle: 'Failed to Update Album', | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid double-dispatching error dialogs
onError already fires showInfoDialog, but useMutationFeedback still has showError enabled. On failures this dispatches a second dialog with the generic message, overwriting the specific password guidance we just sent. Disable the default error path (or drop the manual dispatch) so users receive a single, accurate message.
[suggested fix]
- useMutationFeedback(updateAlbumMutation, {
- loadingMessage: 'Updating album...',
- showSuccess: false, // We handle success manually above
- errorTitle: 'Failed to Update Album',
- });
+ useMutationFeedback(updateAlbumMutation, {
+ loadingMessage: 'Updating album...',
+ showSuccess: false, // We handle success manually above
+ showError: false,
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| onError: (_error: any) => { | |
| dispatch( | |
| showInfoDialog({ | |
| title: 'Error', | |
| message: | |
| 'Failed to update album. Please check your password and try again.', | |
| variant: 'error', | |
| }), | |
| ); | |
| }, | |
| }); | |
| // Use mutation feedback hook | |
| useMutationFeedback(updateAlbumMutation, { | |
| loadingMessage: 'Updating album...', | |
| showSuccess: false, // We handle success manually above | |
| errorTitle: 'Failed to Update Album', | |
| }); | |
| onError: (_error: any) => { | |
| dispatch( | |
| showInfoDialog({ | |
| title: 'Error', | |
| message: | |
| 'Failed to update album. Please check your password and try again.', | |
| variant: 'error', | |
| }), | |
| ); | |
| }, | |
| }); | |
| // Use mutation feedback hook | |
| useMutationFeedback(updateAlbumMutation, { | |
| loadingMessage: 'Updating album...', | |
| showSuccess: false, // We handle success manually above | |
| showError: false, | |
| }); |
🤖 Prompt for AI Agents
In frontend/src/components/Album/EditAlbumDialog.tsx around lines 105 to 122,
the mutation error is being dispatched twice: a custom onError handler
dispatches showInfoDialog with a password-specific message, while
useMutationFeedback still triggers the default error dialog. Fix by disabling
the built-in error path in useMutationFeedback (e.g., set showError: false or
remove errorTitle) so only the custom onError dialog is shown, or alternatively
remove the manual dispatch and rely solely on useMutationFeedback; choose one
approach and ensure only a single error dialog is dispatched.
| {selectedCount} selected | ||
| </Badge> | ||
|
|
||
| {/* Select all / Clear selection */} | ||
| <Button | ||
| variant="outline" | ||
| size="sm" | ||
| onClick={isAllSelected ? handleClearSelection : handleSelectAll} | ||
| className="gap-2" | ||
| > | ||
| {isAllSelected ? ( | ||
| <> | ||
| <RotateCcw className="h-4 w-4" /> | ||
| Clear All | ||
| </> | ||
| ) : ( | ||
| <> | ||
| <SelectAll className="h-4 w-4" /> | ||
| Select All | ||
| </> | ||
| )} | ||
| </Button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid stale selection count when store updates mid-render
selectedCount and selectedImageIds.length come from different selector calls, so they can momentarily diverge if the store updates between reads (React-Redux warns against this). Compute selectedCount from selectedImageIds.length locally or fetch both via a single selector.
🤖 Prompt for AI Agents
In frontend/src/components/Media/SelectionToolbar.tsx around lines 71 to 92,
avoid reading selectedCount and selectedImageIds via separate selectors which
can diverge mid-render; instead compute selectedCount from
selectedImageIds.length (or return both values from a single selector) so the
count and IDs are always consistent, update the component to derive
selectedCount locally from the selectedImageIds array and remove the separate
selector call.
| "exclude": ["src/types/react.d.ts", "src/types/global.d.ts"], | ||
| "references": [{ "path": "./tsconfig.node.json" }] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do not exclude the new ambient declaration files
TypeScript builds the program from include minus exclude. By excluding src/types/react.d.ts and src/types/global.d.ts, their ambient augmentations never load, so the React/global typing fixes introduced in this PR will not apply. Please keep these files in the compilation (or wire them in via types/typeRoots) so the intended type coverage actually takes effect.
🤖 Prompt for AI Agents
In frontend/tsconfig.json around lines 34 to 35, the ambient declaration files
src/types/react.d.ts and src/types/global.d.ts are currently excluded,
preventing their augmentations from being loaded; remove these two paths from
the "exclude" array (or alternatively add them to "include", or configure
"types"/"typeRoots" to explicitly include that folder) so the ambient
declarations are compiled and the React/global typing fixes take effect.
… fixes
Features Added:
Components Added:
Backend Integration:
TypeScript Improvements:
UI/UX Enhancements:
Technical Improvements:
This implementation provides a complete foundation for album management in PictoPy while maintaining high code quality and TypeScript safety.
Summary by CodeRabbit