File List: Dragger mode update#4444
Conversation
…s. Updated styles for file name wrapper and upload button alignment based on listType. Removed hidden condition for thumbnail in attachments editor settings.
…gic, enhance item rendering, and improve styles for drag-and-drop functionality. Adjusted upload button text and ensured consistent file handling across different states.
…dling with improved event propagation, update styles for drag-and-drop upload, and modify hidden conditions in attachments editor settings for better clarity.
…ttonText, buttonTextReadOnly, and title for improved customization.
…itional rendering, streamline event handling in item click, and update styles for drag-and-drop upload based on file presence.
…ing logic by removing unnecessary condition and ensure consistent behavior for drag-and-drop upload functionality.
…l rendering logic by removing unnecessary condition and ensure consistent behavior for drag-and-drop upload functionality." This reverts commit deb8d5c.
…agger mode, enhance button text for clarity, and ensure consistent file dialog opening behavior in Dragger component.
WalkthroughRefactors storedFilesRendererBase to use a dedicated internal itemRenderFunction, makes item/icon render returns React.ReactElement, adds listType and hasFiles to the render model, integrates per-file extra content hooks and thumbnail URL lifecycle management, and updates related styling and some designer component props. (47 words) Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant StoredFilesRendererBase
participant AntUploadComponent
participant StylingHook
participant Server
User->>StoredFilesRendererBase: Add/preview/download/replace/delete file actions
StoredFilesRendererBase->>StylingHook: get styles(model: { listType, hasFiles, ... })
StoredFilesRendererBase->>AntUploadComponent: trigger upload (customRequest / beforeUpload)
AntUploadComponent->>StoredFilesRendererBase: upload callbacks (progress, success, error)
StoredFilesRendererBase->>Server: send file (customRequest)
Server-->>StoredFilesRendererBase: response
StoredFilesRendererBase->>StoredFilesRendererBase: createObjectURL for thumbnails & attach revoke cleanup
User->>StoredFilesRendererBase: click item actions (stopPropagation handled)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx (1)
394-401: Download usesuidinstead of persistedfileId.
Ifuiddiffers from the stored id, downloads may fail or hit the wrong record.🐛 Proposed fix
- downloadFile({ fileId: file.uid, fileName: file.name }); + downloadFile({ fileId, fileName: file.name });- downloadFile({ fileId: file.uid, fileName: file.name }); + downloadFile({ fileId, fileName: file.name });Also applies to: 428-436
🤖 Fix all issues with AI agents
In `@shesha-reactjs/src/components/storedFilesRendererBase/index.tsx`:
- Around line 348-352: The code repeatedly casts UploadFile to IStoredFile
inside itemRenderFunction (used to compute isDownloaded, fileId,
persistedFileId); add a type guard function (e.g., isStoredFile(file:
UploadFile): file is IStoredFile) and use it once to narrow the type, then
compute isDownloaded, fileId and persistedFileId from the narrowed variable
instead of repeated (file as IStoredFile) casts so TypeScript knows the shape
safely.
- Around line 615-623: The code currently calls itemRenderFunction(<></>, file)
inside the Dragger fileList map which passes an empty originNode and causes
blank renders for non-text list types; change the originNode to the visual icon
by calling iconRender(file) (i.e., replace the empty fragment with
iconRender(file)) when rendering each file in the map so
Dragger/itemRenderFunction receives the proper visual content for thumbnails and
other non-text types.
- Around line 513-523: Change the customRequest signature to use the proper
rc-upload type instead of any: import UploadRequestOption (aliasing as
RcCustomRequestOptions if desired) from 'rc-upload/lib/interface' and update
customRequest(options: RcCustomRequestOptions): void { ... } so the parameter is
strongly typed; also remove or update the inline comment that claims the type
"doesn't seem to be found anymore" and ensure related usages (e.g., creating
normalizedFile and calling uploadFile) still compile with the typed options.
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx (1)
178-184: Same type casting pattern here - apply the type guard refactor.The
onReplaceClickfunction also uses(file as IStoredFile).id. Once you introduce theisStoredFiletype guard, apply it here as well for consistency.♻️ Suggested refactor
const onReplaceClick = (file: UploadFile): void => { - const fileId = (file as IStoredFile).id || file.uid; + const storedFile = isStoredFile(file) ? file : undefined; + const fileId = storedFile?.id ?? file.uid; setFileToReplace({ uid: file.uid, id: fileId });
🤖 Fix all issues with AI agents
In `@shesha-reactjs/src/components/storedFilesRendererBase/index.tsx`:
- Around line 428-437: In handleItemClick, remove the stray semicolon after the
closing brace of the else block (the `};` at the end of the function) so the
function body ends with `}` only; locate the handleItemClick function that uses
isImageType(file.type) and calls handlePreview(file) or downloadFile({ fileId:
file.uid, fileName: file.name }) and delete the extra semicolon to keep
consistent TypeScript style.
- Around line 558-566: The onClick handler on the Button currently calls
hiddenUploadInputRef.current.click() without verifying the ref exists; update
the Button's onClick (in the component containing hiddenUploadInputRef and
isDragger) to safely check the ref before invoking click (e.g., use an existence
check or optional chaining on hiddenUploadInputRef.current) so a
missing/unassigned ref does not throw at runtime, and preserve the current
conditional behavior that only calls the ref when isDragger is false.
♻️ Duplicate comments (2)
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx (2)
348-352: Replace repeated type casts with a type guard forIStoredFile.The code repeatedly casts
file as IStoredFileto accessidanduserHasDownloaded. Per coding guidelines, prefer type guards over type casting.♻️ Suggested refactor
+const isStoredFile = (file: UploadFile): file is UploadFile & IStoredFile => + typeof file === 'object' && file !== null && ('id' in file || 'userHasDownloaded' in file); const itemRenderFunction = (originNode: React.ReactElement, file: UploadFile): React.ReactElement => { - const isDownloaded = (file as IStoredFile).userHasDownloaded === true; - const fileId = (file as IStoredFile).id || file.uid; - const persistedFileId = (file as IStoredFile).id; + const storedFile = isStoredFile(file) ? file : undefined; + const isDownloaded = storedFile?.userHasDownloaded === true; + const fileId = storedFile?.id ?? file.uid; + const persistedFileId = storedFile?.id;
513-522: Replaceanywith properUploadRequestOptiontype.The
anytype on line 513 violates coding guidelines. TheUploadRequestOptiontype is available fromrc-upload/lib/interfaceand is used elsewhere in the codebase. The comment claiming the type "doesn't seem to be found anymore" is incorrect.🔧 Proposed fix
Add import at the top of the file:
import { UploadRequestOption as RcCustomRequestOptions } from 'rc-upload/lib/interface';Then update the method signature:
-customRequest(options: any) { - // It used to be RcCustomRequestOptions, but it doesn't seem to be found anymore +customRequest(options: RcCustomRequestOptions) {
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx (1)
394-401: Use the computedfileIdvariable for downloads instead offile.uid.The code incorrectly passes
file.uid(a client-side identifier) todownloadFile()at lines 400 and 435, bypassing the computedfileIdvariable which properly handles persisted files. ThefileIdis defined as(file as IStoredFile).id || file.uid, using the persistedfile.idwhen available. Downloads will fail for persisted files becausefile.uidis only valid during the upload session. TheFileVersionsButtonalready correctly usesfileId(line 41), demonstrating the intended pattern.Proposed fixes
<Button size="small" icon={<DownloadOutlined />} title="Download file" onClick={(e) => { e.stopPropagation(); - downloadFile({ fileId: file.uid, fileName: file.name }); + downloadFile({ fileId, fileName: file.name }); }} />} else { - downloadFile({ fileId: file.uid, fileName: file.name }); + downloadFile({ fileId, fileName: file.name }); }
♻️ Duplicate comments (4)
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx (4)
348-352: Replace repeated casts with a type guard.
This keeps to the TS guideline (avoid casts) and makes the narrowing explicit.♻️ Suggested refactor
+const isStoredFile = (file: UploadFile): file is UploadFile & IStoredFile => + typeof file === 'object' && file !== null && ('id' in file || 'userHasDownloaded' in file); - const isDownloaded = (file as IStoredFile).userHasDownloaded === true; - const fileId = (file as IStoredFile).id || file.uid; - const persistedFileId = (file as IStoredFile).id; // Only persisted files have .id + const storedFile = isStoredFile(file) ? file : undefined; + const isDownloaded = storedFile?.userHasDownloaded === true; + const fileId = storedFile?.id ?? file.uid; + const persistedFileId = storedFile?.id; // Only persisted files have .id
436-437: Remove the stray semicolon after the else block.
It’s unnecessary and inconsistent style.🔧 Proposed fix
- }; + }
513-522: ReplaceanyincustomRequestwith the rc-upload type.
This violates the TS guideline and loses type safety.♻️ Suggested fix
+import { UploadRequestOption as RcCustomRequestOptions } from 'rc-upload/lib/interface';- customRequest(options: any) { - // It used to be RcCustomRequestOptions, but it doesn't seem to be found anymore + customRequest(options: RcCustomRequestOptions) {
558-564: Guard the hidden upload input ref before callingclick().
Avoids a null dereference when the ref hasn’t mounted yet.🔧 Proposed fix
- onClick={isDragger ? undefined : () => hiddenUploadInputRef.current.click()} + onClick={isDragger ? undefined : () => hiddenUploadInputRef.current?.click()}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx (1)
305-314: Avoid duplicate filename rendering for thumbnails.
iconRenderalready outputs the filename for thumbnails anditemRenderFunctionadds it again, which can duplicate the name in the UI. Consider rendering the name in one place.🔧 Possible fix (render filename only once)
return ( <> <Image src={imageUrls[uid]} alt={file.name} preview={false} /> - <p className="ant-upload-list-item-name">{file.name}</p> </> );Also applies to: 484-487
♻️ Duplicate comments (3)
shesha-reactjs/src/components/storedFilesRendererBase/index.tsx (3)
348-351: Replace repeatedIStoredFilecasts with a type guard.
This avoids unsafe casting and keeps the type narrowing explicit. As per coding guidelines, ...♻️ Suggested refactor
+ const isStoredFile = (file: UploadFile): file is UploadFile & IStoredFile => + typeof file === 'object' && file !== null && ('id' in file || 'userHasDownloaded' in file); + const itemRenderFunction = (originNode: React.ReactElement, file: UploadFile): React.ReactElement => { - const isDownloaded = (file as IStoredFile).userHasDownloaded === true; - const fileId = (file as IStoredFile).id || file.uid; - const persistedFileId = (file as IStoredFile).id; // Only persisted files have .id + const storedFile = isStoredFile(file) ? file : undefined; + const isDownloaded = storedFile?.userHasDownloaded === true; + const fileId = storedFile?.id ?? file.uid; + const persistedFileId = storedFile?.id; // Only persisted files have .id
513-522: TypecustomRequestoptions instead ofany.
rc-uploadprovidesUploadRequestOption; using it removesanyand clarifies the contract. As per coding guidelines, ...🔧 Suggested fix
+import { UploadRequestOption as RcCustomRequestOptions } from 'rc-upload/lib/interface';- customRequest(options: any) { - // It used to be RcCustomRequestOptions, but it doesn't seem to be found anymore + customRequest(options: RcCustomRequestOptions) {
563-564: GuardhiddenUploadInputRefbefore calling.click().
The ref can be null on initial renders; optional chaining prevents a runtime error.🔧 Suggested fix
- onClick={isDragger ? undefined : () => hiddenUploadInputRef.current.click()} + onClick={isDragger ? undefined : () => hiddenUploadInputRef.current?.click()}
2 participants
#4239
#4098
Summary by CodeRabbit
New Features
Bug Fixes
Style
✏️ Tip: You can customize this high-level summary in your review settings.