feat: optimistic file upload #717
Conversation
|
Mind rebasing with the current version of main then I'll review? To merge it with the database migration changes |
akuokojnr
force-pushed
the
@akuokojnr/optimistic-uploading
branch
from
10accef to
eeec7b5
Compare
|
This looks fantastic! One thing I would change: can you set the carousel to not show any actions for files that are still uploading rather than just making them not clickable? This would be for the privacy, add to slate, delete, download options etc. And also probably also not allow them to be multi-selected with the checkbox or with shift + click + drag? We don't want people trying to do actions that way on files that are still uploading either |
|
Also, I noticed that if you upload only duplicate files and it skips them (aka no files successfully upload), they just stay there and don't get removed. This is probably because it never reaches the websocket library update that happens at the end of successfully uploading a file. There's two ways to go about fixing this: one is for every file that fails during upload, you manually remove it from the library. The other is you make some kind of a call that does |
|
I also noticed that when uploading to a slate, the files flash on screen then disappear? This could be because of some logic I have in SceneSlate, so let me know if you have trouble debugging this and I can help out Actually I don't think it's uploading properly to slates at all. Since when I drop something into a slate it doesn't show up there even after it shows up in library. And when I upload to a slate, the uploading notification just stays there and doesn't ever go away even though it's done uploading. This may be due to something that got deleted while rebasing with the new code? |
| let update = succeeded.map((item) => { | ||
| let data = item.json?.data; | ||
|
|
||
| let itemToUpdateIndex = library[0].children.findIndex((item) => item.id === data.id); |
There was a problem hiding this comment.
This is something that changed in the migration. Pleas make sure you take a look at the doc https://www.notion.so/slatesystem/Files-Database-Migration-84ccad66574d487fbad4a4a2a5857276 for changes and edit your code to fit the new data structure
It's no longer library[0].children, it's now just library
There was a problem hiding this comment.
This might be the source of some of the other errors I mentioned (like uploads not resolving properly etc)
| @@ -0,0 +1,758 @@ | |||
| import * as React from "react"; | |||
There was a problem hiding this comment.
CarouselSidebarData.js and CarouselSidebarSlate.js have been removed and replaced with CarouselSidebar.js
If there's any changes you made to this, please add them to CarouselSidebar.js instead and delete this
| type: files[i].type, | ||
| size: files[i].size, | ||
| }, | ||
| decorator: "OPTIMISTIC-FILE", |
There was a problem hiding this comment.
Instaed of using decorator = "OPTIMISTIC-FILE" maybe we can do something like file.optimistic = true
Decorator might be used for other things so we'd like to reserve it for that ideally. Plus it's easier to check for (if (file.optimistic)) and b/c with decorator.startsWith("OPTIMISTIC") you have to worry about keeping capitalization consistent
This PR lands optimistic upload in Slate. Files (images, video, audio, pdf, html, fonts) are immediately shown and can be previewed whilst the upload completes. When the upload fails, the file is removed from the data screen.
Corresponding PR at: https://github.com/slate-engineering/shovel
TODO