From 46f904fe59fd8ae6354536a1069ba2499a11f249 Mon Sep 17 00:00:00 2001 From: Noooste <83548733+Noooste@users.noreply.github.com> Date: Fri, 17 Apr 2026 16:46:55 +0200 Subject: [PATCH] refactor: optimize state management and filtering in AccessControl and ObjectsTable components Signed-off-by: Noooste <83548733+Noooste@users.noreply.github.com> --- .../src/components/buckets/ObjectsTable.tsx | 123 ++++++++---------- frontend/src/hooks/useBucketObjects.ts | 98 ++++++-------- frontend/src/pages/AccessControl.tsx | 21 ++- 3 files changed, 104 insertions(+), 138 deletions(-) diff --git a/frontend/src/components/buckets/ObjectsTable.tsx b/frontend/src/components/buckets/ObjectsTable.tsx index 457f654..0e1c9c3 100644 --- a/frontend/src/components/buckets/ObjectsTable.tsx +++ b/frontend/src/components/buckets/ObjectsTable.tsx @@ -1,4 +1,4 @@ -import {useEffect, useState} from 'react'; +import {useEffect, useMemo, useState} from 'react'; import {useNavigate} from 'react-router-dom'; import {Badge} from '@/components/ui/badge'; import {Button} from '@/components/ui/button'; @@ -64,7 +64,6 @@ export function ObjectsTable({ const navigate = useNavigate(); const [sortColumn, setSortColumn] = useState('name'); const [sortDirection, setSortDirection] = useState('asc'); - const [filteredObjects, setFilteredObjects] = useState([]); // Store tokens for each page: [undefined (page 1), token1 (page 2), token2 (page 3), ...] const [pageTokens, setPageTokens] = useState<(string | undefined)[]>([undefined]); const [currentPageIndex, setCurrentPageIndex] = useState(0); @@ -86,14 +85,13 @@ export function ObjectsTable({ } }, [initialized, initialPageToken, initialItemsPerPage, itemsPerPage, nextContinuationToken, onPageChange, onItemsPerPageChange]); - const sortObjects = (objList: S3Object[]): S3Object[] => { - const sorted = [...objList].sort((a, b) => { - // Always put folders before files + const filteredObjects = useMemo(() => { + const query = searchQuery.toLowerCase(); + const filtered = objects.filter((obj) => obj.key.toLowerCase().includes(query)); + return [...filtered].sort((a, b) => { const aIsFolder = a.isFolder ? 1 : 0; const bIsFolder = b.isFolder ? 1 : 0; - if (aIsFolder !== bIsFolder) { - return bIsFolder - aIsFolder; - } + if (aIsFolder !== bIsFolder) return bIsFolder - aIsFolder; let compareValue = 0; switch (sortColumn) { @@ -116,20 +114,7 @@ export function ObjectsTable({ return sortDirection === 'asc' ? compareValue : -compareValue; }); - - return sorted; - }; - - // Effect 1: Apply client-side filtering and sorting (NO pagination reset) - useEffect(() => { - const filtered = objects.filter((obj) => - obj.key.toLowerCase().includes(searchQuery.toLowerCase()) - ); - const sorted = sortObjects(filtered); - setFilteredObjects(sorted); - - // Do NOT reset pagination - search/sort are client-side operations - }, [searchQuery, objects, sortColumn, sortDirection]); + }, [objects, searchQuery, sortColumn, sortDirection, currentPath]); // Effect 2: Reset pagination ONLY on path navigation useEffect(() => { @@ -192,6 +177,7 @@ export function ObjectsTable({ return ( <>
+ @@ -304,55 +290,53 @@ export function ObjectsTable({ {obj.lastModified ? (() => { const d = new Date(obj.lastModified); return ( - - - -
- {d.toLocaleDateString('en-GB', { - day: '2-digit', - month: 'short', - year: 'numeric', - })} {d.toLocaleTimeString('en-GB', { - hour: '2-digit', - minute: '2-digit', - second: '2-digit', - hour12: false, - })} CET + + +
+ {d.toLocaleDateString('en-GB', { + day: '2-digit', + month: 'short', + year: 'numeric', + })} {d.toLocaleTimeString('en-GB', { + hour: '2-digit', + minute: '2-digit', + second: '2-digit', + hour12: false, + })} CET +
+
+ +
+
+ UTC + + {d.toLocaleString('en-GB', { + day: '2-digit', + month: 'short', + year: 'numeric', + hour: '2-digit', + minute: '2-digit', + second: '2-digit', + hour12: false, + timeZone: 'UTC', + })} UTC +
- - -
-
- UTC - - {d.toLocaleString('en-GB', { - day: '2-digit', - month: 'short', - year: 'numeric', - hour: '2-digit', - minute: '2-digit', - second: '2-digit', - hour12: false, - timeZone: 'UTC', - })} UTC - -
-
- Relative - - {formatRelativeTime(d)} - -
-
- Timestamp - - {d.toISOString()} - -
+
+ Relative + + {formatRelativeTime(d)} +
- - - +
+ Timestamp + + {d.toISOString()} + +
+
+
+ ); })() : null} @@ -390,6 +374,7 @@ export function ObjectsTable({ )}
+
{/* Pagination Controls */} diff --git a/frontend/src/hooks/useBucketObjects.ts b/frontend/src/hooks/useBucketObjects.ts index 4fbdcf8..5862698 100644 --- a/frontend/src/hooks/useBucketObjects.ts +++ b/frontend/src/hooks/useBucketObjects.ts @@ -1,4 +1,4 @@ -import { useState, useEffect, useCallback } from 'react'; +import { useState, useEffect, useCallback, useRef } from 'react'; import { objectsApi } from '@/lib/api'; import type { S3Object, UploadTask } from '@/types'; import { toast } from 'sonner'; @@ -13,8 +13,9 @@ export function useBucketObjects(bucketName: string | null, currentPath: string const [nextContinuationToken, setNextContinuationToken] = useState(undefined); const [itemsPerPage, setItemsPerPage] = useState(25); const [currentContinuationToken, setCurrentContinuationToken] = useState(undefined); - const [previousPath, setPreviousPath] = useState(currentPath); + const previousPathRef = useRef(currentPath); const [uploadTasks, setUploadTasks] = useState([]); + const clearTasksTimerRef = useRef | null>(null); const fetchObjects = useCallback(async (continuationToken?: string, isRefresh = false, isNav = false) => { if (!bucketName) return; @@ -46,24 +47,26 @@ export function useBucketObjects(bucketName: string | null, currentPath: string useEffect(() => { if (!bucketName) return; - // Detect if this is a path change (navigation) or initial load - const isPathChange = previousPath !== currentPath && objects.length > 0; - setPreviousPath(currentPath); + const isPathChange = previousPathRef.current !== currentPath && objects.length > 0; + previousPathRef.current = currentPath; - // Use navigation mode if it's a path change, otherwise use normal loading fetchObjects(undefined, false, isPathChange); // eslint-disable-next-line react-hooks/exhaustive-deps }, [bucketName, currentPath, itemsPerPage]); + useEffect(() => { + return () => { + if (clearTasksTimerRef.current) clearTimeout(clearTasksTimerRef.current); + }; + }, []); + const uploadFiles = useCallback(async (files: File[]) => { if (!bucketName) return false; - // Check if files are from a folder upload - const hasRelativePaths = files.some((file: any) => file.webkitRelativePath); + const hasRelativePaths = files.some((file) => !!file.webkitRelativePath); - // Get unique folders from the files const folders = new Set(); - files.forEach((file: any) => { + files.forEach((file) => { if (file.webkitRelativePath) { const parts = file.webkitRelativePath.split('/'); if (parts.length > 1) { @@ -72,9 +75,8 @@ export function useBucketObjects(bucketName: string | null, currentPath: string } }); - // Initialize upload tasks const tasks: UploadTask[] = files.map((file, index) => { - const relativePath = (file as any).webkitRelativePath || file.name; + const relativePath = file.webkitRelativePath || file.name; const key = currentPath ? `${currentPath}${relativePath}` : relativePath; return { id: `${Date.now()}-${index}`, @@ -88,53 +90,36 @@ export function useBucketObjects(bucketName: string | null, currentPath: string setUploadTasks(tasks); - // Upload files with progress tracking and error handling let successCount = 0; let errorCount = 0; - // Upload files one by one - const concurrency = 1; - const uploadPromises: Promise[] = []; + for (const task of tasks) { + try { + setUploadTasks(prev => prev.map(t => + t.id === task.id ? { ...t, status: 'uploading' as const } : t + )); - for (let i = 0; i < tasks.length; i += concurrency) { - const batch = tasks.slice(i, Math.min(i + concurrency, tasks.length)); + await objectsApi.upload(bucketName, task.key, task.file, (progress) => { + setUploadTasks(prev => prev.map(t => { + if (t.id !== task.id || t.progress === progress) return t; + return { ...t, progress }; + })); + }); - const batchPromises = batch.map(async (task) => { - try { - // Update task status to uploading - setUploadTasks(prev => prev.map(t => - t.id === task.id ? { ...t, status: 'uploading' as const } : t - )); - - await objectsApi.upload(bucketName, task.key, task.file, (progress) => { - setUploadTasks(prev => prev.map(t => - t.id === task.id ? { ...t, progress } : t - )); - }); - - // Update task status to completed - setUploadTasks(prev => prev.map(t => - t.id === task.id ? { ...t, status: 'completed' as const, progress: 100 } : t - )); - successCount++; - } catch (error) { - // Update task status to error but continue with other uploads - const errorMessage = error instanceof Error ? error.message : 'Upload failed'; - setUploadTasks(prev => prev.map(t => - t.id === task.id ? { ...t, status: 'error' as const, error: errorMessage } : t - )); - errorCount++; - console.error(`Failed to upload ${task.key}:`, error); - } - }); - - uploadPromises.push(...batchPromises); - await Promise.all(batchPromises); + setUploadTasks(prev => prev.map(t => + t.id === task.id ? { ...t, status: 'completed' as const, progress: 100 } : t + )); + successCount++; + } catch (error) { + const errorMessage = error instanceof Error ? error.message : 'Upload failed'; + setUploadTasks(prev => prev.map(t => + t.id === task.id ? { ...t, status: 'error' as const, error: errorMessage } : t + )); + errorCount++; + console.error(`Failed to upload ${task.key}:`, error); + } } - await Promise.all(uploadPromises); - - // Show summary toast if (errorCount === 0) { if (hasRelativePaths && folders.size > 0) { const folderNames = Array.from(folders).join(', '); @@ -148,9 +133,10 @@ export function useBucketObjects(bucketName: string | null, currentPath: string toast.error(`Failed to upload ${errorCount} file${errorCount > 1 ? 's' : ''}`); } - // Clear upload tasks after a delay - setTimeout(() => { + if (clearTasksTimerRef.current) clearTimeout(clearTasksTimerRef.current); + clearTasksTimerRef.current = setTimeout(() => { setUploadTasks([]); + clearTasksTimerRef.current = null; }, 3000); await fetchObjects(currentContinuationToken, true); @@ -161,7 +147,6 @@ export function useBucketObjects(bucketName: string | null, currentPath: string if (!bucketName) return false; try { - // Optimistically remove the object from the UI setObjects(prev => prev.filter(obj => obj.key !== key)); await objectsApi.delete(bucketName, key); @@ -170,7 +155,6 @@ export function useBucketObjects(bucketName: string | null, currentPath: string return true; } catch (error) { console.error('Delete object error:', error); - // Revert the optimistic update by refetching await fetchObjects(currentContinuationToken, true); return false; } @@ -180,7 +164,6 @@ export function useBucketObjects(bucketName: string | null, currentPath: string if (!bucketName || keys.length === 0) return false; try { - // Optimistically remove the objects from the UI setObjects(prev => prev.filter(obj => !keys.includes(obj.key))); await objectsApi.deleteMultiple(bucketName, keys, currentPath || undefined); @@ -189,7 +172,6 @@ export function useBucketObjects(bucketName: string | null, currentPath: string return true; } catch (error) { console.error('Bulk delete error:', error); - // Revert the optimistic update by refetching await fetchObjects(currentContinuationToken, true); return false; } diff --git a/frontend/src/pages/AccessControl.tsx b/frontend/src/pages/AccessControl.tsx index 69e216c..916ae37 100644 --- a/frontend/src/pages/AccessControl.tsx +++ b/frontend/src/pages/AccessControl.tsx @@ -1,4 +1,4 @@ -import {useEffect, useState} from 'react'; +import {useEffect, useMemo, useState} from 'react'; import {Header} from '@/components/layout/header'; import {Button} from '@/components/ui/button'; import {Input} from '@/components/ui/input'; @@ -31,7 +31,6 @@ import {toast} from 'sonner'; export function AccessControl() { const [keys, setKeys] = useState([]); - const [filteredKeys, setFilteredKeys] = useState([]); const [searchQuery, setSearchQuery] = useState(''); const [isLoading, setIsLoading] = useState(true); const [createDialogOpen, setCreateDialogOpen] = useState(false); @@ -83,7 +82,6 @@ export function AccessControl() { setIsLoading(true); const data = await accessApi.listKeys(); setKeys(data); - setFilteredKeys(data); } catch (error) { console.error('Failed to fetch keys:', error); } finally { @@ -94,14 +92,15 @@ export function AccessControl() { fetchKeys(); }, []); - useEffect(() => { - const filtered = keys.filter( - (key) => - key.name.toLowerCase().includes(searchQuery.toLowerCase()) || - key.accessKeyId.toLowerCase().includes(searchQuery.toLowerCase()) - ); - setFilteredKeys(filtered); - }, [searchQuery, keys]); + const filteredKeys = useMemo( + () => + keys.filter( + (key) => + key.name.toLowerCase().includes(searchQuery.toLowerCase()) || + key.accessKeyId.toLowerCase().includes(searchQuery.toLowerCase()) + ), + [keys, searchQuery] + ); const handleCreateKey = async () => { if (!newKeyName) {