feat: support creating drop box subfolders in upload#514
feat: support creating drop box subfolders in upload#514davidlougheed wants to merge 12 commits into
Conversation
also updates to antd 5.29
gsfk
left a comment
gsfk
left a comment
There was a problem hiding this comment.
A few mostly non-blocking comments:
- shouldn't it block upload if you enter an illegal subfolder name? (currently it will do the upload, but pretend you didn't create a subfolder)
- it seems like there is some stale state going on with the parent folder, because I get a stale parent folder selection this way:
- click a folder in the main Drop Box UI and click "Upload". The parent folder in the modal will be correct.
- close the modal (either by finishing upload or just by closing). If you now click a different folder in the main UI and click "Upload" again the parent folder in the modal will be wrong: it will be your choice from last time. If you choose & click multiple times it will always be one selection behind.
Not changed in this PR but I was confused by the File Selector in the Upload modal, since the button to select a file is called "Upload". Shouldn't it be "Select File"? Presumably the final "OK" button means upload.
|
all good points, should be fixed now. i still need to do some testing with this in S3 mode with this drop box PR: bento-platform/bento_drop_box_service#81 |
SanjeevLakhwani
left a comment
SanjeevLakhwani
left a comment
There was a problem hiding this comment.
First pass.
The UI for creating a new subfolder could use some improvement. Ideally I would prefer a new folder icon in row with the folder in the tree and also an option in the following:
Also, if we are allowing adding subfolders we should also have a way to delete them.
| <Input | ||
| value={newSubfolderName} | ||
| onChange={(e) => { | ||
| setNewSubfolderName(e.target.value.replaceAll(/[\s/*]/g, "_")); |
There was a problem hiding this comment.
the backward slash is also an illegal character on linux. Also should we disable . just in case. I dont see how someone could use this for now, but to be safe.
There was a problem hiding this comment.
i do disable ./.. elsewhere, but we could just prevent . completely it's true
There was a problem hiding this comment.
it does not work "naively" to have a new subfolder button in the main bar, since subfolders do not really exist in S3 and so we'd need to make dummy files. we could take that approach, but otherwise I think it makes sense to just do it on file upload.
There was a problem hiding this comment.
I'll make it a bit clearer, but the new subfolders that show up when uploading are only created if you actually upload a file to that path.
refact: rewrite workflowCommon, ManagerDropBoxContent as tsx
# Conflicts: # package-lock.json
3 participants
also updates to antd 5.29
see newly-fleshed-out ticket https://redmine.c3g-app.sd4h.ca/issues/1949