Skip to content

Commit f1ce283

Browse files
Armaan GuptaArmaan Gupta
authored andcommitted
fix: file upload re-upload issue
1 parent 5f65bc9 commit f1ce283

File tree

2 files changed

+184
-18
lines changed

2 files changed

+184
-18
lines changed

packages/react-vanilla-components/__tests__/components/FileUpload.test.tsx

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -311,4 +311,107 @@ describe("File Upload", () => {
311311
expect(input).toHaveLength(1);
312312
expect(input[0]).toHaveAttribute('aria-describedby', `${f.id}__longdescription ${f.id}__shortdescription`)
313313
})
314-
});
314+
315+
test("should allow re-uploading same file after removal", async () => {
316+
const f = {
317+
...field,
318+
type: "file",
319+
};
320+
const { renderResponse } = await helper(f);
321+
322+
// Get the file input element directly (like other tests do)
323+
const input = renderResponse.container.getElementsByClassName("cmp-adaptiveform-fileinput__widget");
324+
325+
// 1. Upload initial file
326+
const file = new File(["(⌐□_□)"], "test-document.pdf", { type: "application/pdf" });
327+
userEvent.upload(input[0] as HTMLInputElement, file);
328+
329+
// Verify file was uploaded
330+
expect(renderResponse.queryByText("test-document.pdf")).toBeTruthy();
331+
332+
// 2. Remove the file using the remove button
333+
const removeButton = renderResponse.getByLabelText("Remove file");
334+
userEvent.click(removeButton);
335+
336+
// Verify file was removed
337+
expect(renderResponse.queryByText("test-document.pdf")).toBeFalsy();
338+
339+
// 3. Re-upload the same file (this tests the bug fix)
340+
const newFile = new File(["(⌐□_□)"], "test-document.pdf", { type: "application/pdf" });
341+
userEvent.upload(input[0] as HTMLInputElement, newFile);
342+
343+
// 4. Verify the same file appears correctly after re-upload
344+
expect(renderResponse.queryByText("test-document.pdf")).toBeTruthy();
345+
});
346+
347+
test("should handle duplicate filenames correctly without removing wrong file", async () => {
348+
const f = {
349+
...field,
350+
type: "file[]",
351+
};
352+
const { renderResponse } = await helper(f);
353+
354+
const input = renderResponse.container.getElementsByClassName("cmp-adaptiveform-fileinput__widget");
355+
356+
// Upload first document
357+
const largerContent = Array(100).fill('(⌐□_□)').join('');
358+
const document1 = new File([largerContent], "document.pdf", { type: "application/pdf" });
359+
userEvent.upload(input[0] as HTMLInputElement, document1);
360+
361+
// Upload image file to create separation between duplicate names
362+
const imageFile = new File(["image data"], "image.jpg", { type: "image/jpeg" });
363+
userEvent.upload(input[0] as HTMLInputElement, imageFile);
364+
365+
// Upload second document with same filename but different content
366+
const smallerContent = '(⌐□_□)';
367+
const document2 = new File([smallerContent], "document.pdf", { type: "application/pdf" });
368+
userEvent.upload(input[0] as HTMLInputElement, document2);
369+
370+
// Verify all files are present
371+
const fileItems = renderResponse.container.getElementsByClassName("cmp-adaptiveform-fileinput__fileitem");
372+
expect(fileItems).toHaveLength(3);
373+
374+
const fileSizes = renderResponse.container.getElementsByClassName("cmp-adaptiveform-fileinput__filesize");
375+
expect(fileSizes).toHaveLength(3);
376+
const sizeTexts = Array.from(fileSizes).map(el => el.textContent);
377+
expect(sizeTexts.filter(size => size !== sizeTexts[0])).toHaveLength(2);
378+
379+
expect(renderResponse.queryByText("image.jpg")).toBeTruthy();
380+
381+
const removeButtons = renderResponse.container.getElementsByClassName("cmp-adaptiveform-fileinput__filedelete");
382+
expect(removeButtons).toHaveLength(3);
383+
384+
// Remove the first uploaded document
385+
userEvent.click(removeButtons[0] as HTMLButtonElement);
386+
387+
// Verify exactly 2 files remain
388+
const remainingFileItems = renderResponse.container.getElementsByClassName("cmp-adaptiveform-fileinput__fileitem");
389+
expect(remainingFileItems).toHaveLength(2);
390+
391+
const remainingFileSizes = renderResponse.container.getElementsByClassName("cmp-adaptiveform-fileinput__filesize");
392+
const remainingSizeTexts = Array.from(remainingFileSizes).map(el => el.textContent);
393+
expect(remainingSizeTexts).toHaveLength(2);
394+
395+
expect(renderResponse.queryByText("image.jpg")).toBeTruthy();
396+
expect(renderResponse.queryByText("document.pdf")).toBeTruthy();
397+
398+
// Remove the image file to verify the second document remains
399+
const updatedRemoveButtons = renderResponse.container.getElementsByClassName("cmp-adaptiveform-fileinput__filedelete");
400+
expect(updatedRemoveButtons).toHaveLength(2);
401+
402+
const imageRemoveButton = Array.from(updatedRemoveButtons).find((button) => {
403+
const fileItem = button.closest('.cmp-adaptiveform-fileinput__fileitem');
404+
const fileName = fileItem?.querySelector('.cmp-adaptiveform-fileinput__filename')?.textContent;
405+
return fileName === 'image.jpg';
406+
});
407+
userEvent.click(imageRemoveButton as HTMLButtonElement);
408+
409+
// Verify only the second document remains
410+
const finalFileItems = renderResponse.container.getElementsByClassName("cmp-adaptiveform-fileinput__fileitem");
411+
expect(finalFileItems).toHaveLength(1);
412+
413+
const finalFileSizes = renderResponse.container.getElementsByClassName("cmp-adaptiveform-fileinput__filesize");
414+
expect(finalFileSizes).toHaveLength(1);
415+
expect(renderResponse.queryByText("image.jpg")).toBeFalsy();
416+
});
417+
});

packages/react-vanilla-components/src/components/FileUpload.tsx

Lines changed: 80 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
// * LINK- https://github.com/adobe/aem-core-forms-components/blob/master/ui.af.apps/src/main/content/jcr_root/apps/core/fd/components/form/fileinput/v1/fileinput/fileinput.html
1818
// ******************************************************************************
1919

20-
import React, { useCallback, useRef, useState } from 'react';
20+
import React, { useCallback, useRef, useState, useEffect } from 'react';
2121
import { FileObject } from '@aemforms/af-core';
2222
import { getFileSizeInBytes } from '@aemforms/af-core';
2323
import { withRuleEngine } from '../utils/withRuleEngine';
@@ -41,19 +41,52 @@ const FileUpload = (props: PROPS) => {
4141
properties,
4242
valid
4343
} = props;
44+
type LocalFile = { uid: string, file: File | FileObject };
45+
46+
const generateUid = (seed?: string) => `${Date.now()}-${Math.random().toString(36).slice(2)}${seed ? `-${seed}` : ''}`;
47+
48+
const getIdentity = (f: any) => `${f?.name || ''}|${f?.size || ''}|${f?.lastModified || ''}|${f?.type || ''}`;
49+
50+
const uidMapRef = React.useRef<Map<string, string>>(new Map());
51+
52+
const wrapWithUid = (items: Array<File | FileObject> | null | undefined): LocalFile[] => {
53+
const list = items && (items instanceof Array ? items : [items]);
54+
return (list || []).map((f) => {
55+
const identity = getIdentity(f as any);
56+
let uid = uidMapRef.current.get(identity);
57+
if (!uid) {
58+
uid = generateUid(identity);
59+
uidMapRef.current.set(identity, uid);
60+
}
61+
return { uid, file: f };
62+
});
63+
};
64+
4465
let val = value && (value instanceof Array ? value : [value]);
45-
const [files, setFiles] = useState<FileObject[]>(val || []);
66+
const [files, setFiles] = useState<LocalFile[]>(wrapWithUid(val as Array<File | FileObject>) || []);
4667
const [ dragOver, setDragOver ] = useState(false);
4768

69+
// Sync internal state with external value prop only once (initial mount)
70+
const didInitFromPropsRef = useRef(false);
71+
useEffect(() => {
72+
if (!didInitFromPropsRef.current) {
73+
const newVal = value && (value instanceof Array ? value : [value]);
74+
setFiles(wrapWithUid(newVal as Array<File | FileObject>));
75+
didInitFromPropsRef.current = true;
76+
}
77+
}, [value]);
78+
4879
const maxFileSizeInBytes = getFileSizeInBytes(maxFileSize);
4980
let multiple = props.type?.endsWith('[]') ? { multiple: true } : {};
5081

82+
// Dispatch value to the model. When field supports multiple values, send array; otherwise a single item
5183
const fileChangeHandler = useCallback(
52-
(files: Array<File | FileObject>) => {
84+
(localFiles: Array<LocalFile>) => {
85+
const plainFiles = localFiles.map(({ file }) => file);
5386
if (multiple) {
54-
props.dispatchChange(files);
87+
props.dispatchChange(plainFiles);
5588
} else {
56-
props.dispatchChange(files.length > 0 ? files[0] : null);
89+
props.dispatchChange(plainFiles.length > 0 ? plainFiles[0] : null);
5790
}
5891
},
5992
[multiple, props.dispatchChange]
@@ -69,30 +102,54 @@ const FileUpload = (props: PROPS) => {
69102
setDragOver(false);
70103
};
71104

105+
// Handles file selection via input, drag/drop, or paste
72106
const fileUploadHandler = useCallback((e) => {
73107
e.preventDefault();
74108
const newFiles = Array.from<File>(e.dataTransfer?.files || e?.target?.files || e.clipboardData?.files || []);
109+
110+
// Clear the input value to allow re-uploading the same file again
111+
if (e.target && e.target.type === 'file') {
112+
e.target.value = '';
113+
}
114+
75115
if (newFiles?.length) {
76116
const validFiles = newFiles.filter((file: File) => file.size <= maxFileSizeInBytes);
77117
if (validFiles.length < newFiles.length) {
78118
// Show constraint message for files with size exceeding the limit
79119
alert(`${props.constraintMessages?.maxFileSize}`);
80120
}
81-
const updatedFiles = [...files, ...validFiles];
82-
setFiles(updatedFiles as FileObject[]);
121+
122+
// Avoid collapsing same-named files: append new entries without deduping
123+
const wrappedNew = validFiles.map((f) => ({ uid: generateUid(`${f.name}-${f.size}-${f.lastModified}`), file: f }));
124+
const updatedFiles: LocalFile[] = [...files, ...wrappedNew];
125+
setFiles(updatedFiles);
83126
fileChangeHandler(updatedFiles);
84127
}
128+
85129
setDragOver(false);
86130
},
87131
[files, fileChangeHandler, maxFileSizeInBytes, props?.constraintMessages]
88132
);
89133

134+
// Removes one file by its unique id and clears the input to allow re-uploading the same file
90135
const removeFile = useCallback(
91-
(index: number) => {
136+
(uid: string) => {
137+
const index = files.findIndex((f) => f.uid === uid);
138+
if (index === -1) {return;}
139+
// remove identity mapping as well to avoid leaks
140+
const toRemove = files[index];
141+
const identity = getIdentity((toRemove?.file as any));
142+
if (identity) {
143+
uidMapRef.current.delete(identity);
144+
}
92145
const fileList = [...files];
93-
fileList.splice(index,1);
146+
fileList.splice(index, 1);
94147
setFiles(fileList);
95148
fileChangeHandler(fileList);
149+
// Clear the input value so the same file can be selected again
150+
if (fileInputField.current) {
151+
(fileInputField.current as HTMLInputElement).value = '';
152+
}
96153
},
97154
[files, fileChangeHandler]
98155
);
@@ -156,25 +213,31 @@ const FileUpload = (props: PROPS) => {
156213
</div>
157214
<ul className="cmp-adaptiveform-fileinput__filelist">
158215
{files &&
159-
files?.map((item: FileObject, index) => (
216+
files?.map(({ file, uid }) => (
160217
<li
161218
className="cmp-adaptiveform-fileinput__fileitem"
162-
key={item?.name}
219+
key={uid}
163220
>
164221
<span
165222
className="cmp-adaptiveform-fileinput__filename"
166-
aria-label={item?.name}
223+
aria-label={(file as any)?.name}
167224
>
168-
{item?.name}
225+
{(file as any)?.name}
169226
</span>
170227
<span className="cmp-adaptiveform-fileinput__fileendcontainer">
171228
<span className="cmp-adaptiveform-fileinput__filesize">
172-
{formatBytes(item?.size)}
229+
{formatBytes((file as any)?.size)}
173230
</span>
174231
<button
175-
onClick={() => removeFile(index)}
232+
type="button"
233+
onClick={(e) => {
234+
// Prevent form submit bubbling when used inside a <form>
235+
e.preventDefault();
236+
e.stopPropagation();
237+
removeFile(uid);
238+
}}
176239
className="cmp-adaptiveform-fileinput__filedelete"
177-
role="button"
240+
aria-label="Remove file"
178241
>
179242
x
180243
</button>
@@ -188,4 +251,4 @@ const FileUpload = (props: PROPS) => {
188251
);
189252
};
190253

191-
export default withRuleEngine(FileUpload);
254+
export default withRuleEngine(FileUpload);

0 commit comments

Comments
 (0)