Skip to content

feat: upload input v3 image preview improvement #263

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.

Sign up for GitHub

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

Open
priscila-moneo wants to merge 2 commits into
base: main
Choose a base branch
from
+324 −57
Open

feat: upload input v3 image preview improvement #263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8c59ac0
feat: upload input v3 image preview improvement
priscila-moneo Jun 5, 2026
0061872
fix: feedback from PR
priscila-moneo Jun 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
111 changes: 111 additions & 0 deletions src/components/inputs/upload-input-v3/__tests__/upload-input-v3.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,117 @@ describe('UploadInputV3', () => {
});
});

describe('Image Preview', () => {
beforeEach(() => {
URL.createObjectURL = jest.fn(file => `blob:${file.name}`);
URL.revokeObjectURL = jest.fn();
});

afterEach(() => {
delete URL.createObjectURL;
delete URL.revokeObjectURL;
});

test('shows preview immediately when an image file is added', () => {
render(<UploadInputV3 {...defaultProps} />);
act(() => {
dropzoneCallbacks.onAddedFile({ name: 'photo.jpg', size: 136000, type: 'image/jpeg' });
});
const img = screen.getByRole('img', { name: 'photo.jpg' });
expect(img).toHaveAttribute('src', 'blob:photo.jpg');
});

test('shows no preview for non-image files', () => {
render(<UploadInputV3 {...defaultProps} />);
act(() => {
dropzoneCallbacks.onAddedFile({ name: 'document.pdf', size: 50000, type: 'application/pdf' });
});
expect(screen.queryByRole('img', { name: 'document.pdf' })).not.toBeInTheDocument();
});

test('preserves blob URL preview after value updates with server-renamed filename', () => {
const { rerender } = render(<UploadInputV3 {...defaultProps} value={[]} />);

act(() => {
dropzoneCallbacks.onAddedFile({ name: 'photo.jpg', size: 136000, type: 'image/jpeg' });
dropzoneCallbacks.onFileCompleted({ name: 'photo.jpg', size: 136000 });
dropzoneCallbacks.onUploadComplete({ name: 'server_photo_abc123.jpg', size: 136000 }, 'test-upload', {});
});

rerender(<UploadInputV3 {...defaultProps} value={[{ filename: 'server_photo_abc123.jpg', size: 136000 }]} />);

const img = screen.getByRole('img', { name: 'server_photo_abc123.jpg' });
expect(img).toHaveAttribute('src', 'blob:photo.jpg');
});

test('revokes blob URL on cancel and does not assign it to the next upload', () => {
const { rerender } = render(<UploadInputV3 {...defaultProps} value={[]} maxFiles={2} />);

act(() => {
dropzoneCallbacks.onAddedFile({ name: 'photo-a.jpg', size: 10000, type: 'image/jpeg' });
});
act(() => { fireEvent.click(screen.getByRole('button')); });
expect(URL.revokeObjectURL).toHaveBeenCalledWith('blob:photo-a.jpg');

act(() => {
dropzoneCallbacks.onAddedFile({ name: 'photo-b.jpg', size: 20000, type: 'image/jpeg' });
dropzoneCallbacks.onFileCompleted({ name: 'photo-b.jpg', size: 20000 });
dropzoneCallbacks.onUploadComplete({ name: 'server_photo-b_xyz.jpg', size: 20000 }, 'test-upload', {});
});

rerender(<UploadInputV3 {...defaultProps} value={[{ filename: 'server_photo-b_xyz.jpg', size: 20000 }]} maxFiles={2} />);

const img = screen.getByRole('img', { name: 'server_photo-b_xyz.jpg' });
expect(img).toHaveAttribute('src', 'blob:photo-b.jpg');
expect(img).not.toHaveAttribute('src', 'blob:photo-a.jpg');
});

test('correctly maps previews for parallel uploads using response size', () => {
const { rerender } = render(<UploadInputV3 {...defaultProps} value={[]} maxFiles={2} />);

act(() => {
dropzoneCallbacks.onAddedFile({ name: 'sunset.jpg', size: 10000, type: 'image/jpeg' });
dropzoneCallbacks.onAddedFile({ name: 'portrait.jpg', size: 20000, type: 'image/jpeg' });
dropzoneCallbacks.onFileCompleted({ name: 'sunset.jpg', size: 10000 });
dropzoneCallbacks.onFileCompleted({ name: 'portrait.jpg', size: 20000 });
// server returns files in reverse order
dropzoneCallbacks.onUploadComplete({ name: '246_portrait_abc123.jpg', size: 20000 }, 'test-upload', {});
dropzoneCallbacks.onUploadComplete({ name: '246_sunset_def456.jpg', size: 10000 }, 'test-upload', {});
});

rerender(<UploadInputV3 {...defaultProps} maxFiles={2} value={[
{ filename: '246_portrait_abc123.jpg', size: 20000 },
{ filename: '246_sunset_def456.jpg', size: 10000 },
]} />);

expect(screen.getByRole('img', { name: '246_portrait_abc123.jpg' })).toHaveAttribute('src', 'blob:portrait.jpg');
expect(screen.getByRole('img', { name: '246_sunset_def456.jpg' })).toHaveAttribute('src', 'blob:sunset.jpg');
});

test('revokes blob URL on error and does not assign it to the next upload', () => {
const { rerender } = render(<UploadInputV3 {...defaultProps} value={[]} maxFiles={2} />);

act(() => {
dropzoneCallbacks.onAddedFile({ name: 'photo-a.jpg', size: 10000, type: 'image/jpeg' });
dropzoneCallbacks.onFileError({ name: 'photo-a.jpg', size: 10000 }, 'Upload failed');
});
expect(URL.revokeObjectURL).toHaveBeenCalledWith('blob:photo-a.jpg');

act(() => { fireEvent.click(screen.getByRole('button')); });
act(() => {
dropzoneCallbacks.onAddedFile({ name: 'photo-b.jpg', size: 20000, type: 'image/jpeg' });
dropzoneCallbacks.onFileCompleted({ name: 'photo-b.jpg', size: 20000 });
dropzoneCallbacks.onUploadComplete({ name: 'server_photo-b_xyz.jpg', size: 20000 }, 'test-upload', {});
});

rerender(<UploadInputV3 {...defaultProps} value={[{ filename: 'server_photo-b_xyz.jpg', size: 20000 }]} maxFiles={2} />);

const img = screen.getByRole('img', { name: 'server_photo-b_xyz.jpg' });
expect(img).toHaveAttribute('src', 'blob:photo-b.jpg');
expect(img).not.toHaveAttribute('src', 'blob:photo-a.jpg');
});
});

describe('Edge Cases', () => {
test('handles empty value array', () => {
const { container } = render(<UploadInputV3 {...defaultProps} value={[]} />);
Expand Down
134 changes: 88 additions & 46 deletions src/components/inputs/upload-input-v3/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
* limitations under the License.
**/

import React, { useState, useRef, useMemo, useCallback, useEffect } from 'react';
import React, { useState, useRef, useMemo, useCallback, useLayoutEffect, useEffect } from 'react';
import T from "i18n-react/dist/i18n-react";
import {
Box,
Expand All @@ -30,6 +30,19 @@ import ProgressiveImg from '../../progressive-img';
import file_icon from '../upload-input/file.png';
import './index.less';

const fileRowSx = {
display: 'flex',
alignItems: 'center',
py: 1.5,
mb: 1,
};

const formatFileSize = (bytes) => {
if (!bytes) return '0 KB';
if (bytes >= 1024 * 1024) return `${Math.round(bytes / (1024 * 1024))} MB`;
return `${Math.round(bytes / 1024)} KB`;
};

const UploadInputV3 = ({
value = [],
onRemove,
Expand All @@ -55,26 +68,28 @@ const UploadInputV3 = ({
const dropzoneInstanceRef = useRef(null);
const [uploadingFiles, setUploadingFiles] = useState([]);
const [errorFiles, setErrorFiles] = useState([]);
const [filePreviews, setFilePreviews] = useState({});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const filePreviewsRef = useRef({});
filePreviewsRef.current = filePreviews;
const uploadingFilesRef = useRef([]);
uploadingFilesRef.current = uploadingFiles;

const getDefaultAllowedExtensions = useCallback(() => {
return mediaType && mediaType.type
? mediaType?.type?.allowed_extensions.map((ext) => `.${ext.toLowerCase()}`).join(",")
: '';
}, [mediaType]);

const getDefaultMaxSize = useCallback(() => {
return mediaType ? mediaType?.max_size / (1024 * 1024) : 100;
}, [mediaType]);
useEffect(() => {
return () => {
Object.values(filePreviewsRef.current).forEach(url => { if (url) URL.revokeObjectURL(url); });
uploadingFilesRef.current.forEach(f => { if (f.previewUrl) URL.revokeObjectURL(f.previewUrl); });
};
}, []);

const allowedExt = useMemo(() =>
getAllowedExtensions ? getAllowedExtensions() : getDefaultAllowedExtensions(),
[getAllowedExtensions, getDefaultAllowedExtensions]
);
const allowedExt = useMemo(() => {
if (getAllowedExtensions) return getAllowedExtensions();
return mediaType?.type?.allowed_extensions?.map(ext => `.${ext.toLowerCase()}`).join(',') ?? '';
}, [getAllowedExtensions, mediaType]);

const maxSize = useMemo(() =>
getMaxSize ? getMaxSize() : getDefaultMaxSize(),
[getMaxSize, getDefaultMaxSize]
);
const maxSize = useMemo(() => {
if (getMaxSize) return getMaxSize();
return mediaType ? mediaType.max_size / (1024 * 1024) : 100;
}, [getMaxSize, mediaType]);

const canUpload = useMemo(() =>
!maxFiles || value.length < maxFiles,
Expand Down Expand Up @@ -114,13 +129,7 @@ const UploadInputV3 = ({
media_upload: value,
}), [mediaType, value]);

const formatFileSize = useCallback((bytes) => {
if (!bytes) return '0 KB';
if (bytes >= 1024 * 1024) return `${Math.round(bytes / (1024 * 1024))} MB`;
return `${Math.round(bytes / 1024)} KB`;
}, []);

const formatExtensionsDisplay = useCallback(() => {
const extDisplay = useMemo(() => {
if (!allowedExt) return '';
const exts = allowedExt.split(',')
.map(e => e.trim().replace('.', '').toUpperCase())
Expand All @@ -132,15 +141,21 @@ const UploadInputV3 = ({

const handleRemove = useCallback((file) => (ev) => {
ev.preventDefault();
const blobUrl = filePreviews[file.filename];
if (blobUrl) {
URL.revokeObjectURL(blobUrl);
setFilePreviews(prev => { const next = { ...prev }; delete next[file.filename]; return next; });
}
onRemove(file);
}, [onRemove]);
}, [onRemove, filePreviews]);

const handleDropzoneReady = useCallback((dz) => {
dropzoneInstanceRef.current = dz;
}, []);

const handleAddedFile = useCallback((file) => {
setUploadingFiles(prev => [...prev, { name: file.name, size: file.size, progress: 0, complete: false }]);
const previewUrl = file.type?.startsWith('image/') ? URL.createObjectURL(file) : null;
setUploadingFiles(prev => [...prev, { name: file.name, size: file.size, progress: 0, complete: false, previewUrl }]);
if (onUploadStart) onUploadStart(file);
}, [onUploadStart]);

Expand All @@ -164,14 +179,22 @@ const UploadInputV3 = ({
));
}, []);

// Once the parent updates value, remove all completed files from uploadingFiles
useEffect(() => {
useLayoutEffect(() => {

@santipalenque santipalenque Jun 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see the use case for this change, can you explain ?

@priscila-moneo priscila-moneo Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useLayoutEffect runs synchronously before the browser paints, so when value updates, the completed uploading row is removed before the user sees anything. With useEffect it runs after paint, causing a one-frame flash where the file appears twice — once as "Complete" in the uploading section and once in the committed value section.

@santipalenque santipalenque Jun 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this just hides the symptom but does not address the re-render . which is caused by this

@priscila-moneo priscila-moneo Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done the refactor you mention and prevValueRef, pendingPreviewsRef and the batched matching are all gone. But useLayoutEffect is still needed though: when value updates, both the completed uploading row and the new value row land in the same render, and without it the user sees them both for one frame before the cleanup runs. It's not hiding a symptom, it's preventing a real visual duplicate.

@priscila-moneo priscila-moneo Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm working on the last of your comments and will update this PR with the latest code, will tag you so you can review it.

@smarcet smarcet Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@priscila-moneo any update on this one ?

@priscila-moneo priscila-moneo Jun 24, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code already updated

if (uploadingFiles.length === 0 || value.length === 0) return;
setUploadingFiles(prev => prev.filter(f => !f.complete));
const valueFilenames = new Set(value.map(f => f.filename));
setUploadingFiles(prev => prev.filter(f => {
if (!f.complete) return true;
// Only remove once the parent confirms receipt via value; untracked rows drop immediately.
return f.serverFilename ? !valueFilenames.has(f.serverFilename) : false;
}));
}, [value]);
Comment thread
coderabbitai[bot] marked this conversation as resolved.

const handleFileError = useCallback((file, message) => {
setUploadingFiles(prev => prev.filter(f => !(f.name === file.name && f.size === file.size)));
setUploadingFiles(prev => {
const entry = prev.find(f => f.name === file.name && f.size === file.size);
if (entry?.previewUrl) URL.revokeObjectURL(entry.previewUrl);
return prev.filter(f => !(f.name === file.name && f.size === file.size));
});
setErrorFiles(prev => [...prev, { name: file.name, size: file.size, message }]);
}, []);

Expand All @@ -186,24 +209,39 @@ const UploadInputV3 = ({
}, []);

const handleDeleteUploading = useCallback((file) => {
setUploadingFiles(prev => {
const entry = prev.find(f => f.name === file.name && f.size === file.size);
if (entry?.previewUrl) URL.revokeObjectURL(entry.previewUrl);
return prev.filter(f => !(f.name === file.name && f.size === file.size));
});
if (dropzoneInstanceRef.current) {
const dzFile = dropzoneInstanceRef.current.files?.find(
f => f.name === file.name && f.size === file.size
);
if (dzFile) dropzoneInstanceRef.current.removeFile(dzFile);
}
setUploadingFiles(prev => prev.filter(f => !(f.name === file.name && f.size === file.size)));
}, []);

const wrappedOnUploadComplete = useCallback((response, dzId, dzData) => {
// Mark fully-uploaded rows complete (covers HTTP 202 flow where handleFileCompleted was skipped).
// Guard against flipping rows whose bytes are still in flight when maxFiles > 1.
setUploadingFiles(prev => prev.map(f => (f.progress >= 100 ? { ...f, complete: true } : f)));
// Tag the specific matched row with serverFilename so the layout effect can remove it only
// once the parent confirms receipt via value — avoids pruning sibling rows in parallel uploads.
setUploadingFiles(prev => {
const serverFilename = response?.name;
const matchedEntry = response?.size
? prev.find(f => f.size === response.size && f.previewUrl)
: null;
if (matchedEntry && serverFilename) {
setFilePreviews(p => ({ ...p, [serverFilename]: matchedEntry.previewUrl }));
}
return prev.map(f => {
if (f.progress < 100) return f;
return { ...f, complete: true, ...(f === matchedEntry && serverFilename ? { serverFilename } : {}) };
});
});
if (onUploadComplete) onUploadComplete(response, dzId, dzData);
}, [onUploadComplete]);

const extDisplay = formatExtensionsDisplay();

const renderDropzone = () => {
if (!postUrl) {
return (
Expand Down Expand Up @@ -254,13 +292,6 @@ const UploadInputV3 = ({
);
};

const fileRowSx = {
display: 'flex',
alignItems: 'center',
py: 1.5,
mb: 1,
};

return (
<Box className="upload-input-v3">
{label && (
Expand Down Expand Up @@ -294,8 +325,18 @@ const UploadInputV3 = ({
key={`uploading-${index}`}
sx={fileRowSx}
>
<Box sx={{ color: 'primary.main', display: 'flex', alignItems: 'center', mr: 2, minWidth: 32 }}>
<UploadFileIcon fontSize="medium" />
<Box sx={{ display: 'flex', alignItems: 'center', mr: 2, width: 64, height: 64, flexShrink: 0 }}>
{file.previewUrl ? (
<ProgressiveImg
alt={file.name}
src={file.previewUrl}
placeholderSrc={file_icon}
/>
) : (
<Box sx={{ color: 'primary.main', display: 'flex', alignItems: 'center' }}>
<UploadFileIcon fontSize="medium" />
</Box>
)}
</Box>

<Box sx={{ flex: 1, minWidth: 0 }}>
Expand Down Expand Up @@ -379,7 +420,8 @@ const UploadInputV3 = ({
let src = file?.private_url || file?.public_url || file?.file_url;
if (src === '#') src = file?.public_url;
// custom replace for dropbox case ( download vs raw)
const previewSrc = src ? src.replace("?dl=0", "?raw=1") : filename;
const serverPreviewSrc = src ? src.replace("?dl=0", "?raw=1") : filename;
const previewSrc = filePreviews[filename] || serverPreviewSrc;

return (
<Box
Expand Down
Loading
Loading